[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D128750#3758750 <https://reviews.llvm.org/D128750#3758750>, @mizvekov wrote:

> Just a first glance at the patch, will try to do a more comprehensive review later.

Thanks!



================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1838
 
-    QualType Replacement = Arg.getAsType();
 
----------------
mizvekov wrote:
> I think this change could be masking a bug.
> 
> The arguments to an instantiation should always be canonical.
> 
> We could implement one day instantiation with non-canonical args, but this would not be the only change required to get this to work.
> 
> And regardless it should be done in separate patch with proper test cases :)
Agreed.

It turns out this is because I was using injected template arguments to instantiate a function declaration (to compare function parameter types between two function templates). The injected template type arguments seem to be not canonical. 

Now I realized that I don't need this instantiation. Comparing the function parameter canonical types directly should be fine since the template parameters are uniqued by their kind/index/level etc. which is comparable between two function templates.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128750