aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!



================
Comment at: clang/docs/ThreadSafetyAnalysis.rst:471
 
-  + ``-Wthread-safety-attributes``: Sanity checks on attribute syntax.
+  + ``-Wthread-safety-attributes``: Validation checks on attribute syntax.
   + ``-Wthread-safety-analysis``: The core analysis.
----------------
aaronpuchert wrote:
> Quuxplusone wrote:
> > I'd just say "Rejects invalid attribute syntax." But also
> > - surely this is totally obvious, that invalid syntax causes an error? why 
> > do we even need to write this?
> > - line 469 says "three" but then there are four bullet points
> It's more about semantics than syntax, i.e. something like “this attribute 
> doesn't make sense on such a declaration.” The full (current) list of warning 
> messages is on 
> https://clang.llvm.org/docs/DiagnosticsReference.html#wthread-safety-attributes.
>  (Even “ignoring //A// attribute because its argument is invalid” is actually 
> just saying that we don't understand the AST.)
> 
> "Validation checks" feels like a hendiadys and doesn't really add anything. 
> Indeed something like “Semantic (checks|validation) for thread safety 
> attributes” would be most accurate. Though maybe @aaron.ballman has a better 
> idea.
TIL what `hendiadys` means.

I think the currently proposed wording is reasonable (it's not just about 
syntax, as @aaronpuchert says, it's also about semantic checks). Really, this 
groups is about diagnostics that relate to incorrect use of the attribute 
itself, not the analysis supported by the attribute and "semantic checks" works 
for that).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114562/new/

https://reviews.llvm.org/D114562

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to