[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/clang-tidy/bugprone/UnintendedADLCheck.cpp:51
+  // Remove the '::' at the end.
+  assert(NNS.size() >= 2);
+  NNS.erase(NNS.end() - 2, NNS.end());
----------------
As long as you're asserting anyway, I think you should assert that the last two characters //are// `::`.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.cpp:159
+    if (auto *Ty = dyn_cast<DependentSizedArrayType>(E->getType()))
+      return Ty->getElementType()->isFundamentalType();
+    return false;
----------------
Never mind me if this becomes too ugly, but, I would have expected `bool OnlyDependsOnFundamentalArraySizes(const Expr *)` to be recursive, and I would have expected the `llvm::all_of`-over-a-parameter-list to happen in the caller. To properly handle corner cases like
```
template<class T> void test() {
    char a[sizeof(T)][sizeof(T)]; foo(a);
}
```
Experimentally, it appears that Clang thinks that the type of `char b[sizeof(T)]; ... (b+1) ...` is dependent. (It's definitely `char*`, but it comes from applying `+` to an expression with dependent type.) I have no idea what the paper standard says about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72282