aaron.ballman added a comment.
In https://reviews.llvm.org/D40854#950640, @JonasToth wrote:
> > The "enforcement" listed on the C++ Core Guidelines is very unhelpful, so
> > it might be worth bringing it up as an issue on their bug tracker. ES.100
> > basically says "you know what we mean, wink wink" as enforcement and
> > doesn't give any guidance as to what is safe or unsafe. It gives no
> > exceptions, which I think is unlikely to be useful to most developers. For
> > instance: `void f(unsigned i) { if (i > 0) {} }`, this is a mixed signed
> > and unsigned comparison (literals are signed by default) -- should this
> > check diagnose?
>
> I think the guidelines mostly aim for actual calculations here. For ES.102
> there is an exception to "we want actual modulo arithmetic". They seem
> mostly concerned with the mixing of the signedness of integer types that
> comes from explicitly expressing non-negative values with `unsigned` types.
> Because this is a common pattern (e.g. STL containers), the mixing from
> literals and co is quickly overseen.
> See `ES.106: Don’t try to avoid negative values by using unsigned`
>
> > How about `unsigned int i = 12; i += 1;`? ES.102 is equally as strange with
> > "Flag unsigned literals (e.g. -2) used as container subscripts." That's a
> > signed literal (2) with a negation operator -- do they mean "Flag container
> > subscripts whose value is known to be negative", or something else?
>
> I think here ES.106 is again the rationale but your example shows a hole.
> `unsigned int i = -1` is explicitly forbidden and the example shows a chain
> of implicit conversion of integer types as bad code.
>
> My gut feeling is that the guidelines want programers to write `auto i = 12u`
> or `unsigned int = 12u` but they are not clear about that. Other rules that
> could relate to this are:
> `ES.11: Use auto to avoid redundant repetition of type names`, `ES.23:
> Prefer the {} initializer syntax` (forbidding implicit conversions in
> initialization), `ES.48: Avoid casts` (maybe, but implicit conversion are not
> covered there).
>
> I will ask them about this issue and hopefully we have some clarification.
> But I didn't have much success before :D
Thanks! As it stands, I'm not able to determine whether your implementation
actually does or does not enforce those rules. That's why I was wondering what
the extent of the coverage is according to the rule authors, because I am not
certain whether we're actually implementing the intent of the rules or not.
> @aaron.ballman You are a maintainer for the `cert` rules, are you? How do you
> handle these common issues with integer types? Maybe we could already propose
> some guidance based on `cert`. It is mentioned as well written standard in
> the guidelines :)
Yes, I used to maintain the CERT rules when I worked for the SEI (and I
authored a considerable number of them). CERT and the C++ Core Guidelines have
a somewhat fundamental difference in opinion on how we write the rules. CERT
flags things that are demonstrably going to lead to incorrectness
(specifically, vulnerabilities). The C++ Core Guidelines flag things that could
possibly lead to correctness issues but may also be perfectly correct code.
Because of this difference, CERT rules are a bit harder to implement in
clang-tidy because they often will have some component of data or flow analysis
to them, while C++ Core Guidelines are often blanket bans.
That said, CERT's guidance on integers mostly falls back to the C rules:
https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152052
I would recommend looking at the C++ Core Guideline wording and try to come up
with example code that would run afoul of the rule to see if *you* think a
diagnostic would be useful with that code. If you can't convince yourself that
diagnosing the code is a good idea, it's worth bringing up to the authors to
see if they agree. Similarly, see if you can find example code that would not
run afoul of the rules but you think *should* to see if they agree. (You can
use the CERT rules on integers to help give ideas.) Once you have a corpus of
examples you want to see diagnosed and not diagnosed, try to devise (possibly
new) wording for the enforcement section of the rule and see if the rule
authors agree with the enforcement. From there, writing the check becomes a
pretty simple exercise in translating the enforcement into a clang-tidy check.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40854
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
