aaron.ballman added reviewers: erichkeane, clang-language-wg.
aaron.ballman added a comment.

Thank you for working on this! Can you please add more details to the patch 
summary about the changes?



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


================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5184-5185
 
-  // 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,
-  // but for now we attempt to maintain compatibility.
+  const unsigned NumParams1 = FT1->getTemplatedDecl()->getNumParams();
+  const unsigned NumParams2 = FT2->getTemplatedDecl()->getNumParams();
+
----------------
We tend not to use top-level const on locals in the project as a matter of 
style.


================
Comment at: clang/test/CXX/drs/dr6xx.cpp:1084
 
 namespace dr692 { // dr692: no
   namespace temp_func_order_example2 {
----------------
This DR is also about partial class specializations -- are changes not needed 
in `Sema::getMoreSpecializedPartialSpecialization()` as well?

It'd be helpful to add the examples from the DR here to make sure they're 
handled properly.


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

Reply via email to