- User Since
- Mar 14 2013, 3:16 PM (331 w, 3 d)
Sat, Jul 20
Fri, Jul 19
LGTM aside from a nit. You should give other reviewers a chance to sign off in case they have additional comments.
Thu, Jul 18
There seems to be some separable concerns in this patch. I'd rather real with warn_unused_result, const, and pure attributes separately only because those are under the gnu attribute namespace and I'd prefer to match GNU's semantics for those attributes (or at least understand better where the inconsistencies are).
Wed, Jul 17
Tue, Jul 16
LGTM aside from a tiny nit.
I think this looks reasonable to me, though I am still not certain if the relative path in the python script will work with both the svn in-tree directory layout as well as the git monorepo layout (which I'm far less familiar with).
Fri, Jul 12
Thu, Jul 11
LGTM! Do you need someone to commit on your behalf?
Wed, Jul 10
I'm wary of adding this as an on-by-default warning, despite its presence in MSVC (and despite submitting a similar patch years ago to implement the same functionality). Machine-generated code runs into this one frequently (we've been bitten by it numerous times with our tablegen code, especially in the backends), but it does not seem likely to occur in hand-written code very often.
I'm worried that this will continue to drift out of sync with the static analyzer checks unless we find some way to automate it. I wonder if we could write a script to generate these .rst files and somehow fit it into the workflow for adding new static analyzer checks to re-run the script to produce new files (or modified files) as needed? @NoQ and @Szelethus may have ideas.
Tue, Jul 9
I've commit on your behalf in r365498. Thank you for the patch!
Mon, Jul 8
LGTM, but you should wait for others to have a chance to discuss before committing. You can always do the Clang docs in a separate follow-up patch (I know the changes are trivial, I just wanted to make sure they were all taken care of).
I think this patch should also update our user-facing documentation at the same time, such as https://clang.llvm.org/get_started.html, https://llvm.org/docs/GettingStarted.html, and https://llvm.org/docs/GettingStartedVS.html.
Sat, Jul 6
Fri, Jul 5
Thu, Jul 4
Wed, Jul 3
Mon, Jul 1
Thu, Jun 27
In terms of the code, I think this is ready to go. However, I'm still not happy with '*' in boolean context as a diagnostic. Perhaps appending something about the resulting value always being true|false would help most of these diagnostics be more obvious. blah in boolean context will always evaluate to true|false optionally followed by ; did you mean yada? would make it more obvious what is wrong with the code.
Tue, Jun 25
Mon, Jun 24
Jun 21 2019
Jun 20 2019
Jun 19 2019
LGTM with a few nits.
LGTM aside from a documentation nit.
The attribute parts LGTM! You can change the TargetItaniumCXXABI part in a follow-up commit if you'd prefer.
Committed in r363820; we'll look into integrating the script into lit as you suggested for a possible follow-up.