[PATCH] D134507: [Clang] add missing ClangABICompat check

tahonermann added a comment. > Agreed. It turns out the ambiguous case is already in place. > (https://github.com/llvm/llvm-project/blob/d8af31ecede0c54ec667ab687784149e806c9e4c/clang/test/CXX/drs/dr6xx.cpp#L1104) Ah, perfect! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D134507: [Clang] add missing ClangABICompat check

ychen added a comment. In D134507#3856956 , @tahonermann wrote: >> Per further discussions in D128745 , this >> is not needed. > > I agree that the code change is not needed. I think it is worth keeping the > test

[PATCH] D134507: [Clang] add missing ClangABICompat check

tahonermann added a comment. > Per further discussions in D128745 , this > is not needed. I agree that the code change is not needed. I think it is worth keeping the test though (modified to verify the expected ambiguity). Repository: rG LLVM Github

[PATCH] D134507: [Clang] add missing ClangABICompat check

ychen abandoned this revision. ychen added a comment. Per further discussions in D128745 , this is not needed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134507/new/ https://reviews.llvm.org/D134507

[PATCH] D134507: [Clang] add missing ClangABICompat check

rjmccall added a comment. I left a comment on the old review thread; probably best to center the conversation there. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134507/new/ https://reviews.llvm.org/D134507

[PATCH] D134507: [Clang] add missing ClangABICompat check

ychen added a comment. In D134507#3831951 , @rjmccall wrote: > DRs do not modify existing language versions. We can decide as a vendor > whether to apply DRs to existing language versions. The fact that this is a > language semantics change makes me

[PATCH] D134507: [Clang] add missing ClangABICompat check

rjmccall added a comment. DRs do not modify existing language versions. We can decide as a vendor whether to apply DRs to existing language versions. The fact that this is a language semantics change makes me think that we should not in this case. Repository: rG LLVM Github Monorepo

[PATCH] D134507: [Clang] add missing ClangABICompat check

ychen added a comment. ping. To avoid confusion, note that this and D128745 are not about C++20 or newer language versions. They are implementing DRs which should apply to all language versions >= C++11 since the variadic template is involved.

[PATCH] D134507: [Clang] add missing ClangABICompat check

probinson added a comment. In D134507#3818765 , @ychen wrote: > In D134507#3817928 , @probinson > wrote: > >> It feels odd to use a ClangABI check for something that is affecting what >> source is accepted, but

[PATCH] D134507: [Clang] add missing ClangABICompat check

rjmccall added a comment. I don't think I agree that changes to template partial ordering rules are "ABI-breaking changes". They're breaking changes to *language semantics*, and the ABI break is downstream of that, just like any other semantic change to overload resolution would be. I think

[PATCH] D134507: [Clang] add missing ClangABICompat check

ychen added a comment. In D134507#3817928 , @probinson wrote: > It feels odd to use a ClangABI check for something that is affecting what > source is accepted, but this is not my area of expertise. > @aaron.ballman or @rjmccall would probably be the

[PATCH] D134507: [Clang] add missing ClangABICompat check

probinson added a subscriber: rjmccall. probinson added a comment. It feels odd to use a ClangABI check for something that is affecting what source is accepted, but this is not my area of expertise. @aaron.ballman or @rjmccall would probably be the right people to weigh in on this.

[PATCH] D134507: [Clang] add missing ClangABICompat check

ychen added a comment. In D134507#3811488 , @probinson wrote: > Would this be clearer if the BEFORE-15/AFTER-15 test cases used the same > template construct? Currently they are different so it's hard to work out > the effect of the change. In

[PATCH] D134507: [Clang] add missing ClangABICompat check

probinson added a comment. Hi @ychen I saw you updated the patch description, thanks. IIUC, the change causes some difference in the interpretation of particular template stuff. Would this be clearer if the BEFORE-15/AFTER-15 test cases used the same template construct? Currently they are

[PATCH] D134507: [Clang] add missing ClangABICompat check

probinson added a comment. Anytime there's a Clang ABI version check, I have to wonder whether PS4/PS5 need an exception. (And I'm sorry I missed the earlier patches in this series.) We are pretty much locked to however things worked for Clang 3.2. Is this specific to C++20 (or later)? In

[PATCH] D134507: [Clang] add missing ClangABICompat check

ychen added a comment. Address comments in https://reviews.llvm.org/D128745/new/#3809692. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134507/new/ https://reviews.llvm.org/D134507 ___ cfe-commits

[PATCH] D134507: [Clang] add missing ClangABICompat check

ychen created this revision. ychen added reviewers: aaron.ballman, erichkeane. Herald added a project: All. ychen requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. In the same spirit of da6187f566b7881cb