[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

@Quuxplusone thank you for the input! I think it is a good idea if the library-implementors take a look at it. I myself don't have enough knowledge of C++ and the libraries to judge all this stuff.
So who to ping or to add as reviewer?



================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-unintended-adl.cpp:54
+  move(a);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expression calls 'ns::move' through ADL [bugprone-unintended-adl]
+  forward(a);
----------------
logan-5 wrote:
> Quuxplusone wrote:
> > This is awesome. :)  Please also consider ADL where there's more than one associated namespace, something like this: https://godbolt.org/z/S73Gzy
> > ```
> > template<class T> struct Templated { };
> > Templated<std::byte> testX() {
> >     Templated<std::byte> x;
> >     using std::move;
> >     return move(x);  // "correctly" resolves to std::move today, but still does unintended ADL
> > }
> > ```
> > Please add a test isomorphic to the above, unless you think it's already covered by one of the existing tests.
> It's interesting, that code only triggers the check (i.e. my AST matchers only think it's doing ADL) without the `using std::move`. I admit I'm a bit confused as to why.
The matcher itself only ask the `CallExpr->usesADL()` (not sure about the method name right now, but basically that). So the reason is probably hidden somewhere in `Sema`, if the matcher is the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72282