[PATCH] D129951: adds `__disable_adl` attribute

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

I don't think libc++ can adopt this without having to essentially duplicate our code, since GCC doesn't support `__disable_adl` (and AFAICT there is no coordination between GCC and Clang to add it to both). Have you tested what impact making the members `static` has? Both clang and GCC already support this as an extension back to C++11: https://godbolt.org/z/drE5v8nYo. Maybe it would make more sense to add an attribute `[[clang::cpo]]` instead to tell clang that the class should just be treated as an overload set? Make it requirements that the class is empty, there are no non-static member functions and the class is declared `final` and you should get any benefits without the major drawback of basically no portability. It's of course possible that I'm missing something major, but I think that general way would be a lot more acceptable. Any thoughts?



================
Comment at: clang/lib/Sema/SemaOverload.cpp:9455-9459
+    // Functions in 'std::ranges' are hidden from ADL per [range.iter.ops]/2 and
+    // [algorithms.requirements]/2.
+    if ((*I)->isInStdRangesNamespace() &&
+        Name.getNameKind() == DeclarationName::NameKind::Identifier)
+      continue;
----------------
cjdb wrote:
> rsmith wrote:
> > I worry that this might break some stdlib implementation that finds helper functions via ADL in `std::ranges` somehow. Also, it seems desirable to make this opt-in for the stdlib implementation and indeed for end-user functions not in `std::ranges`.
> > 
> > Have you considered enabling this behavior with an attribute instead of by detecting whether the function is in `std::ranges`?
> You're the second person to have suggested this, and I daresay Aaron will too, based on our prior discussion about this. We'd chatted about `__disable_adl` specifically, so that anyone who uses it won't be affected by a silent change from Clang to GCC: they'd instead get a break. I would prefer an attribute too.
> 
> My main concern is that other stdlib implementers would object to adding yet another annotation to their function calls (based on my failure to get libc++ to be as aggressive as Microsoft/STL is with `[[nodiscard]]`).
FWIW we are getting more aggressive with adding `[[nodiscard]]`. We have extensions enabled by default and are (slowly) adding it to more places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129951