Sent from my iPhone
> On Apr 28, 2017, at 1:00 PM, JF Bastien <[email protected]> wrote: > > >> 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 (hook > into clang for extra diagnostics, and you also get rewriting tools for Free™). I listed that reason last intentionally. I don't think it's the most important. That said, I like the idea of a smarter style checker based on clang. > > >> 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]> 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]> >>>> 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] >>>>> 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 >> >> _______________________________________________ >> 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
