- User Since
- Jul 1 2019, 8:51 AM (29 w, 1 d)
Fri, Jan 17
Correct, it's not supported for MSVC yet. I just tried to enable it to see if MSVC 2019 has support and I get errors:C:\llvm-project\llvm\include\llvm\ADT\Optional.h(290): error C2560: 'llvm::Optional<unknown-type> llvm::Optional<T>::map(const Function &) &&': cannot overload a member function with ref-qualifier with a member function without ref-qualifier
Perhaps the map overload just above that simply needs LLVM_LVALUE_FUNCTION on it? Like how the other getValueOr() and operator*s do.
That's a bummer. I wrote *std::move(Fixup) intentionally that way, since llvm::Optional has a &&-qualified operator* overload that returns std::move of the contained value. I guess the Windows configuration doesn't have LLVM_HAS_RVALUE_REFERENCE_THIS?
Thu, Jan 16
Should be good now.
Fixed some wrong author information in the patch.
Great! In that case, I'll need help committing this, as well as the thing it depends on, https://reviews.llvm.org/D72284 (which has also been LGTM'd).
Wed, Jan 15
Added TODO comment.
Tue, Jan 14
Renamed check option from "Whitelist" to "AllowedIdentifiers". Added note about missing checks in documentation. Changed to use a %select for diagnostic text. Some nits.
Rebased with trunk.
Mon, Jan 13
Fri, Jan 10
Added a TODO comment for catching more reserved names. Added links in documentation to CERT guidelines covered by the check. Pulled strings into named constants and made that logic easier to read.
Thu, Jan 9
Addressed nits. Added CERT aliases. Adjusted the check to work for both C and C++, including where they differ.
Wed, Jan 8
Added tests for template parameters.
Hi @Quuxplusone, glad you found your way here. I thought of adding you as a reviewer out the gate but then I didn't.
Beefed up tests and split into separate files. Added tests for instantiations of template functions that do definitely result in ADL, and made the warning messages more descriptive in those cases (listing out concrete types of arguments in a note).
Tue, Jan 7
Addressed some nits.
(Just in case this got lost in the shuffle)
If this looks good, (I think) I'll need someone with commit access to help wrap it up.
@JonasToth Thanks for the feedback. Will be updating the diff in the next day or so.
Clean up some false positives in macro expansions and in expressions with nested potential ADL usages (e.g. in lambdas passed as function parameters).
Mon, Jan 6
Consistent-ified single-statement-body control flow statements to not use braces.
Addressed some nits. If the rest looks good, I'll need someone with commit access to help me wrap this up.
Addressed some formatting stuff.
I'm taking @JonasToth's suggestion and splitting this into two patches, one for the refactor, followed by one for the new check. The refactor patch is here: https://reviews.llvm.org/D72284. Thanks everyone for your feedback so far.
Sat, Jan 4
Change double-underscore fix-it finding algorithm to correctly collapse any number of >=2 underscores in a row, not just exactly 2 (or multiples of 2).
Addressed formatting issues, and tweaked handling of the names __ and ::_: the check now flags these but doesn't suggest a fix.
@Eugene.Zelenko never mind about pushing back on the nits; I had misread some of them. They all sound good, will address.
Mon, Dec 30
Thanks Marshall. As for www/cxx1z_status.html, my best guess is that it doesn't need updating, since the row for P0433R2 lists it as In progress, which I believe is still the case after this patch. Please correct me if I'm wrong. Otherwise, I'd be grateful if someone with commit access could help me tie a bow around this.
Sun, Dec 29
Hello! Pinging this again.
Nov 18 2019
Nov 1 2019
Beefed up the tests in response to some feedback -- thanks.
Oct 30 2019
Add a newline to synopsis
Oct 29 2019
Aug 29 2019
Aug 27 2019
Thanks. I'll need someone with commit access to help me out with this.
Aug 25 2019
Addressed some feedback.
Aug 24 2019
Aug 23 2019
Add a missing null check.
Aug 22 2019
@rsmith I agree having a final destructor in a non-final class smells weird. I'd be interested in working on adding a warning like that, if it sounds like a useful thing.
Tweaked comment wording for accuracy.
That appears sort of tangential to me. To clarify, this PR is not (necessarily) about devirtualizing destructors themselves, but rather devirtualizing OTHER member function calls for classes whose destructor is marked final, since that is sort of morally equivalent to marking the entire class final.
Jul 8 2019
I believe I need someone to commit it for me... brand new to all this and unsure, frankly.
I had the exact same thought about factoring out this and the function pointer conversion condition. I didn't bother, but I agree it could be a useful function to have.