> On Apr 28, 2017, at 12:12, Maciej Stachowiak <[email protected]> wrote: > > > Here's some comments in the other direction: > > - If there are times we recommend x != 0 instead of !x, it should maybe be > based on whether the condition is better expressed as "not zero" or "false". > In the numTestsForEqualityComparison, that's clearly a "not zero" check given > the naming of the variable. This could be addressed by removing zero/non-zero > from the list with true/false and null/non-null instead of making a carve-out > based on type. > > - Allowing both forms for zero/non-zero comparisons would be unfortunate. We > have style guidelines to avoid tiny inconsistencies like this. So if the > guideline changes, it should be to *require* == 0 or != 0 for numeric > comparisons to 0, not merely allow it. Proliferating both styles would be sad. > > - If we adopted the new rule, it would be slightly sad that a bunch of old > code doesn't follow it. Changing it all at once would be needless churn, but > we'll end up with a lot of code in both styles, partly defeating the > consistency benefits of having a style guideline at all. This is sort of a > general issue with any change to the coding style guidelines. If we change > the guideline for a frequently used construct, the benefit has to be really > high to account for the fact that we'll have many years of inconsistency. > Note that the guidelines are mainly for the benefit of people reading the > code, not writing, and inconsistent style may be worse than consistently > using a slightly worse form. > > - The style checker wouldn't be able to check the rule since it's not smart > enough to tell if you are doing a null check, a false check or a zero check. > (I am not sure if it enforces the current rule.)
That’s kind of a sad reason though. If we think it’s really worth it, we can move to a clang-based approach. It’ll definitely be way more powerful than regular expressions. I really liked how That Other Browser did this <https://chromium.googlesource.com/chromium/src/tools/clang/+/master> (hook into clang for extra diagnostics, and you also get rewriting tools for Free™). > I don't actually have a strong opinion on the substance of the rule, either > version seems fine to me if we were starting from a blank slate. I'm not > entirely sure why the rule ended up that way in the first place. But I wanted > to note these as things to think about. > > Regards, > Maciej > >> On Apr 28, 2017, at 1:00 AM, Keith Miller <[email protected] >> <mailto:[email protected]>> wrote: >> >> Is there any opposition to relaxing this rule? Speak now or forever hold >> your piece! (not really but I would be curious to hear opposition). >> >> Cheers, >> Keith >> >>> On Apr 27, 2017, at 10:32 PM, Carlos Garcia Campos <[email protected] >>> <mailto:[email protected]>> wrote: >>> >>> El jue, 27-04-2017 a las 16:06 -0700, JF Bastien escribió: >>>> Hello C++ fans! >>>> >>>> The C++ style check currently say: >>>> Tests for true/false, null/non-null, and zero/non-zero should all be >>>> done without equality comparisons >>>> >>>> I totally agree for booleans and pointers… but not for integers. I >>>> know it’s pretty much the same thing, but I it takes me slightly >>>> longer to process code like this: >>>> >>>> int numTestsForEqualityComparison = 0: >>>> // Count ‘em! >>>> // … >>>> if (!numTestsForEqualityComparison) >>>> printf(“Good job!”); >>>> >>>> I read it as “if not number of tests for equality comparison”. That's >>>> weird. It takes me every slightly longer to think about, and I’ve >>>> gotten it wrong a bunch of times already. I’m not trying to check for >>>> “notness", I’m trying to say “if there were zero tests for equality >>>> comparison”, a.k.a.: >>>> >>>> if (numTestsForEqualityComparison == 0) >>>> printf(“Good job!”); >>>> >>>> So how about the C++ style let me just say that? I’m not suggesting >>>> we advise using that style for integers everywhere, I’m just saying >>>> it should be acceptable to check zero/non-zero using equality >>>> comparison. >>> >>> I agree, it's also quite confusing when using strcmp, because !strcmp >>> means the strings are equal. It's not only more difficult to read, I've >>> seen patches with wrong strcmp checks because of that. >> >> I also think this could be solved by #defining a success a C call positive >> result that is 0 (e.g. CCallSuccess), regardless of the choice we make here. >> >>> >>>> >>>> !!Thanks (i.e. many thanks), >>>> >>>> JF >>>> >>>> p.s.: With you I am, fans of Yoda comparison, but for another day >>>> this will be. >>>> _______________________________________________ >>>> webkit-dev mailing list >>>> [email protected] <mailto:[email protected]> >>>> https://lists.webkit.org/mailman/listinfo/webkit-dev >>>> <https://lists.webkit.org/mailman/listinfo/webkit-dev> >>> _______________________________________________ >>> webkit-dev mailing list >>> [email protected] <mailto:[email protected]> >>> https://lists.webkit.org/mailman/listinfo/webkit-dev >>> <https://lists.webkit.org/mailman/listinfo/webkit-dev> >> _______________________________________________ >> webkit-dev mailing list >> [email protected] <mailto:[email protected]> >> https://lists.webkit.org/mailman/listinfo/webkit-dev >> <https://lists.webkit.org/mailman/listinfo/webkit-dev> > _______________________________________________ > webkit-dev mailing list > [email protected] > https://lists.webkit.org/mailman/listinfo/webkit-dev
_______________________________________________ webkit-dev mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-dev
