[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked an inline comment as done.
erichkeane added a comment.

In D126907#3600876 <https://reviews.llvm.org/D126907#3600876>, @ChuanqiXu wrote:

> Great progress!
>
> In D126907#3599835 <https://reviews.llvm.org/D126907#3599835>, @erichkeane wrote:
>
>> Note that the failure comes down to:
>>
>>   template<typename T> concept C = T::f();
>>   template<template<C> class P> struct S1{};
>>   template<C> struct X{};
>>   S1<X> s11;
>>
>> and requires the -frelaxed-template-template-args flag:
>>
>>   [ekeane1 at scsel-clx-24 build]$  ./bin/clang -cc1 -std=c++20 temp.cpp -frelaxed-template-template-args
>>   temp.cpp:5:4: error: template template argument 'X' is more constrained than template template parameter 'P'
>>   S1<X> s11;
>>      ^
>>   temp.cpp:3:29: note: 'P' declared here
>>   template <template<C> class P> struct S1{};
>>                               ^
>>   temp.cpp:4:20: note: 'X' declared here
>>   template<C> struct X{};
>>                      ^
>>   1 error generated.
>
> As far as I could tell, we could omit the diagnostic by deleting https://github.com/llvm/llvm-project/blob/bc74bca5363270e987c2e3c263bfaaeb6ceab66f/clang/include/clang/Sema/SemaConcept.h#L45-L53
>
> This change is obsolutely wrong but it shows the bug comes from `ParameterMapping` so we could locate https://github.com/llvm/llvm-project/blob/bc74bca5363270e987c2e3c263bfaaeb6ceab66f/clang/lib/Sema/SemaConcept.cpp#L723 further.

Thanks for the debugging help!  I'm not sure what you mean by "Locating it further"?  Best I can tell looking so far, is the fact that the 'depths' still point based on the 'root', but this is comparing thinking they are relative to the decl themselves.  I see the 'SubstTemplateArguments' there that looks suspicious, so there is probably something about setting up my template parameters correctly there/the MLTAL.

Thanks for the comments and debugging help again!



================
Comment at: clang/test/Driver/crash-report.cpp:28
+// RUNX: cat %t/crash-report-*.cpp | FileCheck --check-prefix=CHECKSRC %s
+// RUNX: cat %t/crash-report-*.sh | FileCheck --check-prefix=CHECKSH %s
 
----------------
ChuanqiXu wrote:
> What's the meaning of RUNX?
Woops!  I didn't add this test change to GIT, but it apparently still showed up with 'diff'.  For some reason, once I switched to lldb the llvm-symbolizer takes a HUGE amount of time during this test when doing check-all, so I used the 'X' to disable the test locally.  This is not intended to be committed.


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

https://reviews.llvm.org/D126907