NoQ added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11792
"change type of '%0' to '%select{std::span|std::array|std::span::iterator}1'
to preserve bounds information">;
+def note_safe_buffer_usage_suggestions_disabled : Note<
+ "pass -fsafe-buffer-usage-suggestions to receive code hardening
suggestions">;
----------------
jkorous wrote:
> I like this idea!
>
> Thinking about the questions you raised:
> > Do we really need that? Maybe we should somehow throttle it to reduce noise?
>
> The only case I can see value in not emitting this note is when the user
> passes `-fno-safe-buffer-usage-suggestions`. In that case it can be argued
> that the user is well aware of the flag since they explicitly turned it off.
>
> That being said I am fine with the current behavior and in either case expect
> we will tweak this once we get feedback from the users.
> The only case I can see value in not emitting this note is when the user
> passes -fno-safe-buffer-usage-suggestions. In that case it can be argued that
> the user is well aware of the flag since they explicitly turned it off.
Yeah fair enough. This will need some rework though, because right now
`-fno-...` is purely a Driver flag, so `-cc1` invocations aren't even aware of
it being passed this way. So it turns from a normal flag to a somewhat quirky
flag.
================
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2159
Sema &S;
+ bool SuggestSuggestions;
----------------
t-rasmud wrote:
> Was there a reason for naming this variable SuggestSuggestions? Can this be
> called EmitSuggestions? I think that would make it uniform and easy to follow
> the code.
It means "Should I emit a note reminding the user to enable
`-fsafe-buffer-usage-suggestions` if they want suggestions?", so it's quite
different from `EmitSuggestions` (in fact it's the opposite). Added a comment.
================
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2203
+ if (IsRelatedToDecl) {
+ assert(!SuggestSuggestions &&
+ "Variables blamed for unsafe buffer usage without suggestions!");
----------------
ziqingluo-90 wrote:
> nitpick:
> I was a bit confused by the name of the variable. My understand of
> `!SuggestSuggestions ` here means "we are suggesting suggestions now".
> Then what does `SuggestSuggestions ` mean?
Yeah it means "Should I emit a note reminding the user to enable
`-fsafe-buffer-usage-suggestions` if they want suggestions?". Added a comment
to the variable.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146669/new/
https://reviews.llvm.org/D146669
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
