[PATCH] D129951: adds `__disable_adl` attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D129951#4178154 <https://reviews.llvm.org/D129951#4178154>, @philnik wrote:

> 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).

I haven't had a lot of time to drive this in Clang, let alone GCC. Even if libc++ can't ultimately use it (which would be sad), there are other libraries that can. For example, Abseil has a similar attitude towards functions as Niebloids, and could wrap it behind a macro.

> 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.

A quick change to the original benchmark <https://godbolt.org/z/13z65EY88> shows the AST for `static operator()` being substantially larger than a function template with ADL being disabled. I haven't properly benchmarked build time impact, but here's a quick one <https://gist.github.com/cjdb/6ade504f010dc550890a82f3a5c0ea6a>. The averages are below:

**`__disable_adl`**

  real  0.1164
  user  0.0706
  sys   0.0488

**`static operator()`**

  real  0.1272
  user  0.081
  sys   0.0488

It is worth acknowledging that the assembly output is now much closer with optimised flags (1.63x larger as opposed to 7.56x larger), but 1.26x larger with `-g` (this is down from 1.66x as non-static).

> 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?

CPOs and Niebloids are different things (and `__disable_adl` is for Niebloids, not CPOs), so any such attribute would need a different name. Having said that, a struct that hasn't has no base and is final only slightly improves the AST size <https://godbolt.org/z/ncq1qx5Ys> with respect to the improvement by using an actual overload set. Finally, there would still be a portability issue because even if `[[clang::niebloid]]` works on Clang, there would still need to be coordination for it to work on GCC; otherwise GCC w/ libc++ mode would have copyable Niebloids; something that the original libc++ design worked hard to ensure wasn't possible so that a feature like this could exist.

It is again worth acknowledging that the assembly output in an optimised build would have parity, but a build using `-O0 -g` will still be ~1.26x larger.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129951