> On Apr 20, 2014, at 1:19 PM, Mark Rowe <[email protected]> wrote: > > >> On Apr 20, 2014, at 13:08, Filip Pizlo <[email protected]> wrote: >> >> >> >>> On Apr 20, 2014, at 12:56 PM, Mark Rowe <[email protected]> wrote: >>> >>> >>>> On Apr 19, 2014, at 13:09, Filip Pizlo <[email protected]> wrote: >>>> >>>> Hey everyone, >>>> >>>> When guarding code with macros that are always defined, such as >>>> ASSERT_DISABLED (it's always either 0 or 1), we have a choice between: >>>> >>>> if (!ASSERT_DISABLED) { >>>> // do things >>>> } >>>> >>>> and: >>>> >>>> #if !ASSERT_DISABLED >>>> // do things >>>> #endif >>>> >>>> I'd like to propose that anytime the normal if would be semantically >>>> equivalent to the preprocessor #if, the normal if should be used. >>>> >>>> We don't lose any compiler optimization, since even at -O0, the compiler >>>> will constant fold the normal if. We do gain a lot of clarity, since the >>>> control flow of normal if statements is subject to proper indentation. >>>> >>>> The "semantically equivalent" requirement still allows for #if to be used >>>> for thinngs like: >>>> >>>> - Guarding the placement of fields in a class. >>>> - Guarding the definitions of other macros (in the case of >>>> ASSERT_DISABLED, we define ASSERT in different ways guarded by #if's) >>>> - Guarding the definition/declaration/inclusion of entire functions, >>>> classes, and other top-level constructs. >>>> >>>> Thoughts? >>> >>> It’d be one thing if we could adopt this approach everywhere, but as you >>> note there are numerous situations in which it won’t compile. What’s worse >>> is that there are situations in which it’ll silently give unintended >>> behavior. >> >> Can you give an example of this? > > Won’t compile: > > if (!MAYBE_DEFINED) { > … > } > > if (SOMETHING) { > void doSomething() { } > } > > if (SOMETHING) { > #include “Something.h" > } > > > Will result in unintended behavior: > > if (!FOO) { > … > #define BAR > … > }
If it won't even compile then I wouldn't worry about it. As for conditionalized #define's and such, it's already rare and discouraged to have those inside function definition bodies. > > >>> It’s not clear to me what benefit you see this proposal bringing, and why >>> it’d be worth the complexity that it adds. Given the lack of a compelling >>> argument in favor of this change I’m firmly against it. >> >> I would be firmly against changing any of the existing "if >> (!ASSERT_DISABLED)" into #if's as this would make already-gnarly JIT code >> even harder to follow. > > So you’re only interested in the wider community’s opinion on this style > proposal if they agree with you, and if not you’ll just ignore them and keep > doing your own thing? No. The style currently allows the use of #if's inside function bodies even if doing so makes the code ugly. It also allows the use of regular conditionals when the predicate being tested happens to be a macro (as in "if (!ASSERT_DISABLED)"). > > - Mark >
_______________________________________________ webkit-dev mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-dev
