Post B4ke75zG23dFdbfZ0C by acsawdey@fosstodon.org
 (DIR) More posts by acsawdey@fosstodon.org
 (DIR) Post #B4kXULKIT3PqC2eGae by azonenberg@ioc.exchange
       0 likes, 0 repeats
       
       Do you indent code inside a #ifdef block the same way as if it were a normal runtime if/else statement?
       
 (DIR) Post #B4kXoyejwyjBxp39Wa by azonenberg@ioc.exchange
       0 likes, 0 repeats
       
       Related: on at least recent clang this triggers a Wmisleading-indentation warning on the meow() line, but IMO should notif(foo)    bar();else if(baz)    foobar();#ifdef ASDF      meow();#endif
       
 (DIR) Post #B4kZGcvERyO0cTVnsG by funkylab@mastodon.social
       0 likes, 0 repeats
       
       @azonenberg should absolutely warn; this *is* misleading!
       
 (DIR) Post #B4kZNwhnILOc0GiTxY by azonenberg@ioc.exchange
       0 likes, 0 repeats
       
       @funkylab see to me, the meow() is indented because it's controlled by the ifdef ASDF, which is itself at the same level of indentation as the top level if/else.i.e. it's exactly the same as if I hadif(foo)    bar();else if(baz)    foobar();if(ASDF)      meow();
       
 (DIR) Post #B4kZUrTR8zodnyIZdo by funkylab@mastodon.social
       0 likes, 0 repeats
       
       @azonenberg no, it's **not** the same; that's exactly the interpretational difference that people who would not indent code within `#if` have with you.
       
 (DIR) Post #B4kZVYRTOAnL4Ki73A by azonenberg@ioc.exchange
       0 likes, 0 repeats
       
       @funkylab the fact that the conditional evaluates at compile time vs run time is immaterial to the fact that it is a control statement with code inside it
       
 (DIR) Post #B4kZbxRtKM1IMaxYJs by firefly@frogs.lgbt
       0 likes, 0 repeats
       
       @azonenberg it looks ilke your `meow()` line has more spaces of indentation than the `bar()` and `foobar()`?(looks like 6 vs 4 if I copy-paste and check in vim)I guess that's unintentional(?)
       
 (DIR) Post #B4kZiADKmO8Tgw6ra4 by azonenberg@ioc.exchange
       0 likes, 0 repeats
       
       @firefly yeah thats just mastodon not having proper monospace editing capability so its easy to get indentation wrong
       
 (DIR) Post #B4kaJcy4ptDXaJpyoS by funkylab@mastodon.social
       0 likes, 0 repeats
       
       @azonenberg the warning is not that your code is wrong, it's that your code is misleading. And me, it would have misled to double-check whether `meow()` is part of the `else` branch there. I'm thankful you're getting warned about these things.
       
 (DIR) Post #B4kaJdBC37i2EzySG0 by synx508@bsd.network
       0 likes, 0 repeats
       
       @funkylab @azonenberg You're right, I was flummoxed by the code, the block really did end on the blank line and I assumed incorrectly that the indent was there to match the indent above. But then I also subscribe to mandatory curly braces even when they're not needed as it helps a little to stop my brain melting because of whitespace, indent, next-line-only rules.
       
 (DIR) Post #B4kaJdLTQtvsksmfHU by azonenberg@ioc.exchange
       0 likes, 0 repeats
       
       @synx508 @funkylab The actual code as written was a bit more clear than this reduced example:https://github.com/ngscopeclient/scopehal/blob/839692f5e1f999f5c818a8b20ce9440ca79f2758/scopehal/AcceleratorBuffer.h#L346Which I didn't think seemed ambiguous.I rewrote the code with an else() around the ifdef to avoid the warning but the code doesn't seem any more readable in that version than what's shown here
       
 (DIR) Post #B4kaJdSZ0XbV6s6KKe by azonenberg@ioc.exchange
       0 likes, 0 repeats
       
       @synx508 @funkylab to me it seems like a big if/else cascade with a few runtime paths and the last two evaluated at compile time
       
 (DIR) Post #B4kaotfgmbd2Mn9Aqu by jpm@aus.social
       0 likes, 0 repeats
       
       @azonenberg IMHO this warning should only fire before the pre-processor is run, because macro-mangling can produce some interesting results…
       
 (DIR) Post #B4kbSRtHUTR7n0MeSe by funkylab@mastodon.social
       0 likes, 0 repeats
       
       @azonenberg @synx508 to me that code is confusing in more than one way! Yeah, by the uniformity of that large block up there this looks immediate, but you know what, having an 8-branch if/elseif:  the exception not the rule! And uniformity beats local beauty (which reasonable can disagree on. Also, I can disagree.) .Generally, you have a type-generic class. Use type generics to your type-to-string conversion! your code isn't shorter than had you…
       
 (DIR) Post #B4kbiwcJ1EhribI2Wu by azonenberg@ioc.exchange
       0 likes, 0 repeats
       
       @funkylab @synx508 The whole reason for the if/else cascade here is that I do *not* always want the raw native type.For example an AcceleratorBuffer<int64_t> will have T evaluate to "long" at compile time, while I explicitly want the stdint type for display.
       
 (DIR) Post #B4kc11PzuDwJp39XTk by poleguy@mastodon.social
       0 likes, 0 repeats
       
       @azonenberg @synx508 @funkylab Adding an else seems like a good disambiguation.I'm thinking of the various ways this could look similar but have very different behavior. The warning seems fairly unimportant in your case, but maybe it's doing heavy lifting for other easy to misinterpret cases?
       
 (DIR) Post #B4ke75zG23dFdbfZ0C by acsawdey@fosstodon.org
       0 likes, 0 repeats
       
       @azonenberg @funkylab To me, ifdef and other preprocessor directives are out of band and their indentation is separate from the C code. So IMHO this compiler warning is correct.