[PATCH] D129951: adds `__disable_adl` attribute

Tue May 2 12:22:50 PDT 2023

aaron.ballman added a comment.

In D129951#4310046 <https://reviews.llvm.org/D129951#4310046>, @cjdb wrote:

> Ping (moving my pings from Thursday afternoon to Monday mornings)

Apologies for the delay in responding, but I was actually silent in the hopes that the discussion around proposing to WG21 and how much general use this feature sees would materialize a bit more. https://clang.llvm.org/get_involved.html#criteria has two criteria that I think apply here: "Evidence of a significant user community" and "Representation within the appropriate governing organization". The fact that this could potentially be used by an STL or Abseil and drastically reduce compile times with a more direct approach than function objects suggests that there might be a significant user community that would benefit from this, but if those projects would struggle to adopt this feature because they need to handle older Clang versions or non-Clang compilers, that community may not materialize. Further, because this is being proposed as a keyword (which I think makes sense over an attribute because a correct program with this construct will not necessarily remain correct in its absence due to different lookup results) and it seems to be functionality that would be appropriate to standardize, there should be some collaboration with WG21 if only to hear back "we don't see a reason such an extension would be a problem for us or conflict with existing efforts".

Specific to this review, however, this is missing documentation in LanguageExtensions.rst for the feature, how to use it, etc. as well as release notes. How does this marking fit into the language? How does it impact redeclaration merging (do all declarations have to have the marking, or is it additive like attributes often are?), how does it impact ODR (do declarations have to match across TU boundaries?), is this a declaration specifier or something else (can  you do `void __disable_adl func()` or does it have to be `__disable_adl void func();`?), that sort of stuff.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129951