Adds a check to clang-tidy to warn when streaming objects of type int8_t or uint8_t, when the likely intended behavior is to promote to int first.
So admittedly, I don't have any experience in clang-tidy. This check is intended to catch what is to us a really annoying source of error (and I'm probably not the only one?). I suspect there are far better ways of writing this check than the one that I pretty much guess-and-checked my way into.
I don't know clang-tidy style either, but might it be more appropriate to say something like "value of type int8_t will be printed as character, not number"? I had to go all the way down to the test cases in this patch before it occurred to me what the actual problem being diagnosed here was.
And speaking of "printed": do you care about streaming *in* values of these types with ">>"?
Please add real description, use space after it and shorten to 80 symbols.
May be match should be configurable, so other streams could be analyzed too? How about LLVM's raw_ostream?
Please don't use auto in cases where return type could not be easily deduced. See modernize-use-auto. Same for auto Underlying.
Please add real description and use space after it.
Please write real description here.
Please move to new checks section in alphabetical order and add one statement description (usually first statement from documentation).
Please use meaningful name or main.
You can use bind for the expr and get Offender from it.
The duplicated diag can be removed. You can use placeholders in diag and make the if include both types.
I think having a list of possible typedefnames would be nice. Some projects might not use the standard ones (e.g. Qt?).
Since the branch occured it might be better to rebase first. Then the Release Notes are empty.
Updates based on review comments - and rebased off of latest so as to get the ReleaseNotes right. Added options so that the user can provide the list of stream types and int typedef types, as desired, defaulting to just basic_ostream and int8_t/uint8_t.
Yes that is possible. Templates ignore the typedefnames. I think that needs to be considered. It might be relevant for a templated operator<<.
It would be nice to have these cases in tests to document the issue. A note in the docs wouldn't hurt too.
I don't think this is a good name for the check, especially because the check can be configured for types other than integers. How about "bugprone-suspicious-stream-value-type"? If you rename the check name, you should also update the name of the files and check accordingly.
This should be ::std::basic_ostream just in case someone does something awful like put namespace std inside another namespace.
It'd be nice to have a qualifier-agnostic way of expressing this (there are other qualifiers, like volatile, and we don't want to have to list them all out unless it's important for the check functionality).
You can use llvm::find(AliasTypes, Name) here instead.
It might also make sense to use an llvm::StringSet instead of a vector of strings that you have to do a linear search over.
const auto *?
Should not use auto here.
This diagnostic doesn't really explain what the issue is. It should probably be reworded to something more along the lines of "%0 will not undergo integral promotion prior to streaming; cast to 'int' to prevent streaming a character".
If you pass in Name, then it won't be properly quoted. You should pass Typedef->getDecl() (but not repeat the expression from above).
You can hoist this into the for loop.
types -> type
Extra space between the comma and "if".
I'd reword this a bit to:
Checks that an object of type int8_t or uint8_t is not streamed. Those types are usually a typedef to a character type that are printed as a character rather than an integer. The user likely intends to stream such a value as an int instead.
(Or something like this.)
Same wording suggestions as above.