gamesh411 added a comment.
I like this, especially, that the functionality of UBOR can be merged and
extended in a much cleaner way (architecturally).
================
Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:301-302
+ pluralSuffix(MaximalAllowedShift));
+ R->addNote(LeftNote, PathDiagnosticLocation{LHS, Ctx.getSourceManager(),
+ Ctx.getLocationContext()});
+ Ctx.emitReport(std::move(R));
----------------
donat.nagy wrote:
> NoQ wrote:
> > Can we just append this to the warning? The `addNote()` is useful for notes
> > that need to be placed in code outside of the execution path, but if it's
> > always next to the warning, it probably doesn't make sense to display it
> > separately.
> I didn't append this to the warning because I felt that the warning text is
> already too long. (By the way, an earlier internal variant of this checker
> produced two separate notes next to the warning message.)
>
> I tweaked the messages of this checker before initiating this review, but I
> feel that there's still some room for improvements. (E.g. in this particular
> case perhaps we could omit some of the details that are not immediately
> useful and could be manually calculated from other parts of the message...)
Just a thought on simplifying the diagnostics a bit:
Warning: "Right operand is negative in left shift"
Note: "The result of left shift is undefined because the right operand is
negative"
Shortened: "Undefined left shift due to negative right operand"
Warning: "Left shift by '34' overflows the capacity of 'int'"
Note: "The result of left shift is undefined because the right operand '34' is
not smaller than 32, the capacity of 'int'"
Shortened: "Undefined left shift: '34' exceeds 'int' capacity (32 bits)"
The more complex notes are a bit sketchy, as relevant information can be lost,
and the following solution is probably a bit too much, but could prove to be an
inspiration:
Warning: "Left shift of '1024' overflows the capacity of 'int'"
CXX Note: "Left shift of '1024' is undefined because 'int' can hold only 32
bits (including the sign bit), so some bits overflow"
CXX Note: "The value '1024' is represented by 11 bits, allowing at most 21 bits
for bitshift"
C Note: "Left shift of '1024' is undefined because 'int' can hold only 31 bits
(excluding the sign bit), so some bits overflow"
C Note: "The value '1024' is represented by 11 bits, allowing at most 20 bits
for bitshift"
Shortened:
CXX Warning: "Undefined left shift: '1024' (11 bits) exceeds 'int' capacity (32
bits, including sign)"
C Warning: "Undefined left shift: '1024' (11 bits) exceeds 'int' capacity (31
bits, excluding sign)"
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156312/new/
https://reviews.llvm.org/D156312
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
