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

[Logan Smith via Phabricator via cfe-commits]

logan-5 marked an inline comment as done.
logan-5 added inline comments.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-unintended-adl.cpp:202
+void macro_test(T t) {
+#define MACRO(x) find_if(x, x, x)
+
----------------
Quuxplusone wrote:
> logan-5 wrote:
> > EricWF wrote:
> > > Arguably this is *exactly* the kind of code we want to diagnose.
> > > 
> > > The call in the macro either, 
> > >   * Is a "customization point" and should be whitelisted. Or,
> > >   * It resolves the same in expansion (and can be qualified), Or,
> > >   * It is a bug. 
> > > 
> > > You mentioned false positives in things like `assert`. Can you provide examples?
> > Fair enough. Disabling the check for macros does seem short sighted on closer thought.
> > 
> > When I run the check over LLVM in debug, `assert` expands to `(__builtin_expect(!(e), 0) ? __assert_rtn(__func__, __FILE__, __LINE__, #e) : (void)0)`. If the assert() is inside a function template, the check claims that `unqualified call to '__assert_rtn' may be resolved through ADL`. Inspecting the AST, this seems to be due to the fact that `__func__` has dependent type. I suppose `__func__` could be special cased to be ignored, or all uglified names, or something?
> Ouch, is that because `__func__` is an array of char with dependent length?
> ```
> template<class T>
> void foo() {
>     char buf[sizeof(T)];
>     memset(buf, '\0', sizeof buf);
> }
> ```
> Unqualified call to `memset`, where one of the arguments has dependent type `char[sizeof(T)]` — does that trigger the diagnostic?
Yep. So, I suppose the check should not trigger for expressions of `DependentSizedArrayType` of `char`? Or of any built-in type, really?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72282