ychen added inline comments.
================ Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:2488 + bool XHasMoreArg = X.pack_size() > Y.pack_size(); + if (!(XHasMoreArg && X.pack_elements().back().isPackExpansion()) && + !(!XHasMoreArg && Y.pack_elements().back().isPackExpansion())) ---------------- FYI: `isSameTemplateArg` is for class/variable partial ordering deduction. The corresponding check for function template deduction is skipped by intention. See https://reviews.llvm.org/rG86a1b135f0c67a022ef628f092c6691892752876 and https://github.com/llvm/llvm-project/blob/e6f1f062457c928c18a88c612f39d9e168f65a85/clang/lib/Sema/SemaTemplateDeduction.cpp#L5064-L5066. ================ Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5184 - // FIXME: This mimics what GCC implements, but doesn't match up with the - // proposed resolution for core issue 692. This area needs to be sorted out, ---------------- aaron.ballman wrote: > ychen wrote: > > aaron.ballman wrote: > > > aaron.ballman wrote: > > > > We tend not to use top-level const on locals in the project as a matter > > > > of style. > > > Does GCC still implement it this way? > > > > > > One concern I have is that this will be an ABI breaking change, and I'm > > > not certain how disruptive it will be. If GCC already made that change, > > > it may be reasonable for us to also break it without having to add ABI > > > tags or something special. But if these changes diverge us from GCC, that > > > may require some extra effort to mitigate. > > Unfortunately, GCC is still using the old/non-conforming behavior. > > https://clang.godbolt.org/z/5K4916W71. What is the usual way to mitigate > > this? > You would use the `ClangABI` enumeration to alter behavior between ABI > versions: > https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/LangOptions.h#L174 > like done in: > https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/DeclCXX.cpp#L929 Looking at how `ClangABI` is currently used, it looks like it is for low-level object layout ABI compatibility. For library/language ABI compatibility, maybe we should not use `ClangABI`? Looking at https://reviews.llvm.org/rG86a1b135f0c67a022ef628f092c6691892752876, I guess the practice is just committed and see? If it breaks important or many existing libraries, just revert or add compiler options? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128745/new/ https://reviews.llvm.org/D128745 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
