Quuxplusone added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.h:27
+class UnintendedADLCheck : public ClangTidyCheck {
+ const bool IgnoreOverloadedOperators;
+ const std::vector<std::string> AllowedIdentifiers;
----------------
EricWF wrote:
> I think we should always ignore operators. I don't see value in having a mode
> where every comparison triggers this warning.
>
I think there's value in that mode, for library writers (not libc++) who really
care about finding every unintentional ADL in their whole library. The standard
library is designed not-to-work with types like
[`Holder<Incomplete>`](https://quuxplusone.github.io/blog/2019/09/26/uglification-doesnt-stop-adl/),
but someone else's library might be designed to work even in that case, and
then they'd want to hear about ADL lookups for things like `operator,`.
Besides, it's just 1 extra line of code in the patch, isn't it?
However, I now think I may not understand how this check works. I thought it
looked for unqualified calls (even in templates) that "may" use ADL, but now
that I look again at the tests, it seems to trigger only on concrete calls (in
concrete template instantiations) that "do" use ADL, which sounds still useful
but much less comprehensive than I had thought.
I think it would catch
```
template<class T> void foo(T t) { t, 0; }
struct S { friend void operator,(S, int); };
template void foo(S);
```
but not
```
template<class T> void foo(T t) { t, 0; }
struct S { friend void operator,(S, int); };
template void foo(int);
```
or
```
template<class T> void foo(T t) { t, 0; }
```
is that right?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72282/new/
https://reviews.llvm.org/D72282
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
