[PATCH] D129951: adds `__disable_adl` attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.


================
Comment at: clang/include/clang/Basic/Attr.td:4132
+def DisableADL : InheritableAttr {
+  let Spellings = [Keyword<"__disable_adl">];
+  let Subjects = SubjectList<[Function]>;
----------------
rsmith wrote:
> Has this syntax been discussed already? If not, why did you choose keyword syntax instead of a normal attribute?
Aaron and I discussed this offline. I'd originally intended to do `[[clang::disable_adl]]`, but Aaron pointed out that this would be problematic for all involved if someone uses a library in GCC, since the feature is implicitly turned off. If we use this syntax, then it forces libraries to acknowledge (and possibly document) that ADL is restricted on Clang, but potentially not GCC.

I will add a note to the documentation that I'm promising.


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5756
+  if (FunctionDecl *F = D->getAsFunction();
+      F->isOverloadedOperator() || F->isCXXClassMember()) {
+    S.Diag(AL.getLoc(), diag::err_disable_adl_no_operators)
----------------
rsmith wrote:
> Maybe also global `operator new` / `operator delete`?
How does ADL even work in with the global namespace? Should it be invalid to apply here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129951