[PATCH] D129951: adds `__disable_adl` attribute

2023-05-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In 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 WG

[PATCH] D129951: adds `__disable_adl` attribute

2023-05-01 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. Ping (moving my pings from Thursday afternoon to Monday mornings) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129951/new/ https://reviews.llvm.org/D129951 ___ cfe-commits mailing

[PATCH] D129951: adds `__disable_adl` attribute

2023-04-27 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. Gentle ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129951/new/ https://reviews.llvm.org/D129951 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D129951: adds `__disable_adl` attribute

2023-04-20 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added inline comments. 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) cor3ntin wrote: > a

[PATCH] D129951: adds `__disable_adl` attribute

2023-04-20 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 515497. cjdb marked 6 inline comments as done. cjdb added a comment. Herald added subscribers: aheejin, dschuff. - undoes whitespace changes as requested - documents feature I think the only thing left is to address the global new/delete and static member funct

[PATCH] D129951: adds `__disable_adl` attribute

2023-03-09 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a subscriber: lewissbaker. cor3ntin added a comment. In D129951#4179596 , @cjdb wrote: > In D129951#4179481 , @ldionne wrote: > >> In D129951#4179467 , @cjd

[PATCH] D129951: adds `__disable_adl` attribute

2023-03-08 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In D129951#4179481 , @ldionne wrote: > In D129951#4179467 , @cjdb wrote: > >> I'm deeply disappointed that libc++ moved away from using `__function_like`: >> that was an important part of pr

[PATCH] D129951: adds `__disable_adl` attribute

2023-03-08 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. In D129951#4179467 , @cjdb wrote: > I'm deeply disappointed that libc++ moved away from using `__function_like`: > that was an important part of preventing niebloid misuse. It isn't conforming > to treat niebloids as function ob

[PATCH] D129951: adds `__disable_adl` attribute

2023-03-08 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. This is a really neat attribute! FWIW, I think we would most likely have used it in `` if it were available back then. Assuming that GCC implemented it, I think we could consider changing to use this attribute. However: 1. This would technically be an ABI break, but thi

[PATCH] D129951: adds `__disable_adl` attribute

2023-03-08 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In D129951#4178949 , @philnik wrote: > In D129951#4178844 , @cjdb wrote: > >> In D129951#4178154 , @philnik >> wrote: >> >>> I don't think libc++ ca

[PATCH] D129951: adds `__disable_adl` attribute

2023-03-08 Thread Nikolas Klauser via Phabricator via cfe-commits
philnik added a comment. In D129951#4178844 , @cjdb wrote: > In D129951#4178154 , @philnik wrote: > >> I don't think libc++ can adopt this without having to essentially duplicate >> our code, since GCC doesn't su

[PATCH] D129951: adds `__disable_adl` attribute

2023-03-08 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In 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 t

[PATCH] D129951: adds `__disable_adl` attribute

2023-03-08 Thread 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? Bo

[PATCH] D129951: adds `__disable_adl` attribute

2023-03-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. 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) aaron.ballman w

[PATCH] D129951: adds `__disable_adl` attribute

2023-03-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. 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) cor3ntin w

[PATCH] D129951: adds `__disable_adl` attribute

2023-03-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. 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) aaron.ballman w

[PATCH] D129951: adds `__disable_adl` attribute

2023-03-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: erichkeane, clang-language-wg. aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:4132 +def DisableADL : InheritableAttr { + let Spellings = [Keyword<"__disable_adl">]; + let Subjects = SubjectList<[Function]>;

[PATCH] D129951: adds `__disable_adl` attribute

2023-02-06 Thread 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

[PATCH] D129951: adds `__disable_adl` attribute

2023-02-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/include/clang/Basic/Attr.td:4132 +def DisableADL : InheritableAttr { + let Spellings = [Keyword<"__disable_adl">]; + let Subjects = SubjectList<[Function]>; Has this syntax been discussed already? If not, why did

[PATCH] D129951: adds `__disable_adl` attribute

2023-02-06 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. Ping @aaron.ballman Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129951/new/ https://reviews.llvm.org/D129951 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.

[PATCH] D129951: adds `__disable_adl` attribute

2023-01-30 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. Ping @aaron.ballman @rsmith Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129951/new/ https://reviews.llvm.org/D129951 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D129951: adds `__disable_adl` attribute

2023-01-25 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:542 If a statement is marked ``nomerge`` and contains call expressions, those call -expressions inside the statement will not be merged during optimization. This +expressions inside the statement will

[PATCH] D129951: adds `__disable_adl` attribute

2023-01-25 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 492296. cjdb marked 6 inline comments as done. cjdb added a comment. responds to feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129951/new/ https://reviews.llvm.org/D129951 Files: clang/include/clang/AS

[PATCH] D129951: adds `__disable_adl` attribute

2023-01-25 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:542 If a statement is marked ``nomerge`` and contains call expressions, those call -expressions inside the statement will not be merged during optimization. This +expressions inside the statement wi

[PATCH] D129951: adds `__disable_adl` attribute

2023-01-25 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:12934-12936 +// else if (!CandidateSet.empty() && CandidateSet.begin()->FoundDecl->hasAttr()) { +// return; +// } This and below need to be deleted. Repository: rG LLVM Git

[PATCH] D129951: adds `__disable_adl` attribute

2023-01-25 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 492234. cjdb retitled this revision from "[clang] teaches Clang the special ADL rules for functions in std::ranges" to "adds `__disable_adl` attribute". cjdb edited the summary of this revision. cjdb added a comment. - updates patch so that it meets what Aaron a