================
@@ -30,7 +30,7 @@ extern time_t mktime(struct tm *timeptr);
void uninit_mbrlen(const char *mbs) {
mbstate_t state;
- mbrlen(mbs, 1, &state); // expected-warning{{3rd function call argument
points to an uninitialized value}}
+ mbrlen(mbs, 1, &state); // expected-warning{{3rd function call argument
points to an uninitialized value; Function 'mbrlen' expects memory pointed to
by this argument to be initialized}}
----------------
NagyDonat wrote:
> Did we have a precedent for having a semicolon in the diagnostic message?
There are many precedents, e.g.
- Property of mutable type 'NSMutableString' has 'copy' attribute; an immutable
object will be stored instead
- Opened file is never closed; potential resource leak
- Return is prohibited after a successful vfork; call _exit() instead
However, the word after the semicolon should start with a lowercase letter --
it is not a separate sentence.
> I wonder if regular punctuation signs would fit better here, like a full-stop.
There are also precedents for using a full-stop, but I think it is really ugly
to end a sentence with a full stop, and have a second sentence after it where
the full stop (which would be at the end of the message) is omitted.
> Alternatively, we can always emit a note alongside the warning location.
That would be also acceptable for me, but I prefer including this in the
warning message to ensure that the warning message is understandable on its own
> @NagyDonat Isn't that the rule of thumb is that the warning messages should
> be plain-old strings - not patterns.
I don't think that there is such rule of thumb, and in fact I'm frequently
writing checkers where the warning message varies dynamically.
> Because having parametric warning messages break uniqueing? Warnings with
> different messages are treated as separate warnings - regardless if they have
> the same BugType and everything else.
The clarification added by this PR cannot break uniqueing, because the added
string will always be the same for two bug reports at the same location. (The
source location is already counted as a distinguishing feature in all bug
reports except those that explicitly set a different uniqueing location, e.g.
the allocation point of the leaked resource.)
--------
Also note that there is the `ShortDescription` / `Description` distinction
which is IMO not directly relevant here, but is important to to know for
similar situations:
A `BugReport` has __two__ data members that store diagnostic messages:
`ShortDescription` (the `warning` diagnostic) and `Description` (the `note`
level diagnostic at the end of the bug path). By default these are the same
[1], but `BugReport` (and `PathSensitiveBugReport`) has constructors that allow
specifying them separately.
The uniqueing process includes the `ShortDescription` in the identity of the
bug report, but ignores the `Description`. (Reports with different
`ShortDescription` are treated as separate reports, while reports that only
differ in the `Description` are treated as identical -- i.e. the user will only
see one of them, while the rest are discarded.)
[1] Implementation detail: technically the default behavior is that
`ShortDescription` is the empty string and in this case `getShortDescription()`
will return the `Description`.
https://github.com/llvm/llvm-project/pull/211038
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
