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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone 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)
+
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72282