JonasToth added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.cpp:89
+
+ const auto *Lookup =
+ Result.Nodes.getNodeAs<UnresolvedLookupExpr>("templateADLexpr");
----------------
logan-5 wrote:
> JonasToth wrote:
> > Can't you just bind directly to the `unresolvedExpr`?
> I need the `"templateADLCall"` node for the diagnostic caret, and the
> `"templateADLexpr"` for the name/spelling of the call. I might totally be
> misunderstanding what you're suggesting here.
Ah, i overlooked that bit. Then The `Lookup` should be `assert`ed as well, to
ensure the matchers works in all circumstances. (future refactorings included)
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/bugprone-unintended-adl.cpp:1
+// RUN: %check_clang_tidy -std=c++14-or-later %s bugprone-unintended-adl %t
+
----------------
logan-5 wrote:
> JonasToth wrote:
> > why 14 or later? `ADL` exists in the prior standards, too.
> >
> > Additionally we need a test for where overloaded operators are not ignored
> > and create the warnings.
> 14 or later just because of the generic lambdas in some of the tests. Is it
> worth separating those tests out into their own files so that we don't have
> to pass this flag here?
Yes. Usually we have the "base"-version of c++ for most of the test, that are
common and extra test-files for newer features or requirements.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72282/new/
https://reviews.llvm.org/D72282
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
