[clang][Sema] Fix initialization of NonTypeTemplateParmDecl...
#121768
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1985,17 +1985,22 @@ void ASTDeclWriter::VisitNonTypeTemplateParmDecl(NonTypeTemplateParmDecl *D) { | |
| // For an expanded parameter pack, record the number of expansion types here | ||
| // so that it's easier for deserialization to allocate the right amount of | ||
| // memory. | ||
|
There was a problem hiding this comment. Changes here are all NFC, right? Or am I missing a purpose to this change? There was a problem hiding this comment. There is a difference: This is not aligned when there are parsing errors. See my comment here. |
||
| Record.push_back(D->hasPlaceholderTypeConstraint()); | ||
| if (D->isExpandedParameterPack()) | ||
| Record.push_back(D->getNumExpansionTypes()); | ||
|
|
||
| VisitDeclaratorDecl(D); | ||
| // TemplateParmPosition. | ||
| Record.push_back(D->getDepth()); | ||
| Record.push_back(D->getPosition()); | ||
|
|
||
| if (D->hasPlaceholderTypeConstraint()) { | ||
| Expr *TypeConstraint = D->getPlaceholderTypeConstraint(); | ||
| Record.push_back(/*PlaceholderTypeConstraintInitialized=*/TypeConstraint != | ||
| nullptr); | ||
| if (TypeConstraint) | ||
| Record.AddStmt(TypeConstraint); | ||
| } | ||
|
|
||
| if (D->isExpandedParameterPack()) { | ||
| for (unsigned I = 0, N = D->getNumExpansionTypes(); I != N; ++I) { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| // RUN: rm -rf %t | ||
| // RUN: split-file %s %t | ||
| // RUN: cd %t | ||
|
|
||
| // RUN: %clang_cc1 -std=c++20 mod.cppm -emit-module-interface -o mod.pcm -fallow-pcm-with-compiler-errors -verify | ||
| // RUN: %clang_cc1 -std=c++20 main.cpp -fmodule-file=mod=mod.pcm -verify -fallow-pcm-with-compiler-errors -fsyntax-only -ast-dump-all | FileCheck %s | ||
|
Comment on lines
+5
to
+6
There was a problem hiding this comment. I'm wondering if this also manifests with just pch serialization - it would always be great if we can simplify the test There was a problem hiding this comment. We don't even need serialization. IIRC it can be triggered with an ast dump, but only if the uninitialized value happens not to be a 0, which is the tricky part to trigger (not using sanitizers, that is). |
||
|
|
||
| // RUN: %clang_cc1 -std=c++20 mod.cppm -emit-reduced-module-interface -o mod.pcm -fallow-pcm-with-compiler-errors -verify | ||
| // RUN: %clang_cc1 -std=c++20 main.cpp -fmodule-file=mod=mod.pcm -verify -fallow-pcm-with-compiler-errors -fsyntax-only -ast-dump-all | FileCheck %s | ||
|
|
||
| //--- mod.cppm | ||
| export module mod; | ||
|
|
||
| template <typename T, auto Q> | ||
| concept ReferenceOf = Q; | ||
|
|
||
| // expected-error@+2 {{unknown type name 'AngleIsInvalidNow'}} | ||
| // expected-error@+1 {{constexpr variable 'angle' must be initialized by a constant expression}} | ||
| constexpr struct angle {AngleIsInvalidNow e;} angle; | ||
|
|
||
| // expected-error@+1 {{non-type template argument is not a constant expression}} | ||
| template<ReferenceOf<angle> auto R, typename Rep> requires requires(Rep v) {cos(v);} | ||
| auto cos(const Rep& q); | ||
|
|
||
| // expected-error@+1 {{non-type template argument is not a constant expression}} | ||
| template<ReferenceOf<angle> auto R, typename Rep> requires requires(Rep v) {tan(v);} | ||
| auto tan(const Rep& q); | ||
|
|
||
| //--- main.cpp | ||
| // expected-no-diagnostics | ||
| import mod; | ||
|
|
||
| // CHECK: |-FunctionTemplateDecl {{.*}} <line:11:1, line:12:22> col:6 imported in mod hidden invalid cos | ||
| // CHECK-NEXT: | |-NonTypeTemplateParmDecl {{.*}} <line:11:10, col:34> col:34 imported in mod hidden referenced invalid 'ReferenceOf<angle> auto' depth 0 index 0 R | ||
| // CHECK-NEXT: | |-TemplateTypeParmDecl {{.*}} <col:37, col:46> col:46 imported in mod hidden referenced typename depth 0 index 1 Rep | ||
| // CHECK-NEXT: | |-RequiresExpr {{.*}} <col:60, col:84> 'bool' | ||
| // CHECK-NEXT: | | |-ParmVarDecl {{.*}} <col:69, col:73> col:73 imported in mod hidden referenced v 'Rep' | ||
| // CHECK-NEXT: | | `-SimpleRequirement {{.*}} dependent | ||
| // CHECK-NEXT: | | `-CallExpr {{.*}} <col:77, col:82> '<dependent type>' | ||
| // CHECK-NEXT: | | |-UnresolvedLookupExpr {{.*}} <col:77> '<overloaded function type>' lvalue (ADL) = 'cos' empty | ||
| // CHECK-NEXT: | | `-DeclRefExpr {{.*}} <col:81> 'Rep' lvalue ParmVar {{.*}} 'v' 'Rep' non_odr_use_unevaluated | ||
| // CHECK-NEXT: | `-FunctionDecl {{.*}} <line:12:1, col:22> col:6 imported in mod hidden cos 'auto (const Rep &)' | ||
| // CHECK-NEXT: | `-ParmVarDecl {{.*}} <col:10, col:21> col:21 imported in mod hidden q 'const Rep &' | ||
|
|
||
| // CHECK: |-FunctionTemplateDecl {{.*}} <line:15:1, line:16:22> col:6 imported in mod hidden invalid tan | ||
| // CHECK-NEXT: | |-NonTypeTemplateParmDecl {{.*}} <line:15:10, col:34> col:34 imported in mod hidden referenced invalid 'ReferenceOf<angle> auto' depth 0 index 0 R | ||
| // CHECK-NEXT: | |-TemplateTypeParmDecl {{.*}} <col:37, col:46> col:46 imported in mod hidden referenced typename depth 0 index 1 Rep | ||
| // CHECK-NEXT: | |-RequiresExpr {{.*}} <col:60, col:84> 'bool' | ||
| // CHECK-NEXT: | | |-ParmVarDecl {{.*}} <col:69, col:73> col:73 imported in mod hidden referenced v 'Rep' | ||
| // CHECK-NEXT: | | `-SimpleRequirement {{.*}} dependent | ||
| // CHECK-NEXT: | | `-CallExpr {{.*}} <col:77, col:82> '<dependent type>' | ||
| // CHECK-NEXT: | | |-UnresolvedLookupExpr {{.*}} <col:77> '<overloaded function type>' lvalue (ADL) = 'tan' empty | ||
| // CHECK-NEXT: | | `-DeclRefExpr {{.*}} <col:81> 'Rep' lvalue ParmVar {{.*}} 'v' 'Rep' non_odr_use_unevaluated | ||
| // CHECK-NEXT: | `-FunctionDecl {{.*}} <line:16:1, col:22> col:6 imported in mod hidden tan 'auto (const Rep &)' | ||
| // CHECK-NEXT: | `-ParmVarDecl {{.*}} <col:10, col:21> col:21 imported in mod hidden q 'const Rep &' | ||
|
Comment on lines
+33
to
+55
There was a problem hiding this comment. Please filter out line numbers on the AST dump match, otherwise this makes these tests hard to update. |
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I like the assert-safety that we get from the
PlaceholderTypeConstraintInitialized, I'm not sure it is worth the rigamarole. Why not just have thectordosetPlaceholderTypeConstraint?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both inside
SemaandASTReaderDecltheNonTypeTemplateParmDeclis created relatively far from the parsing / deserialization of the constraint.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not complaining about the 'set' method, just that the concern is that it is uninitialized. I'm saying, why not just initialize it to nullptr? Or am I misunderstanding the concern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The flag would allow to discern between "was not initialized" and "was explicitly initialized to null".
Now, I have to admit I do not know if the latter is meaningful for a constrained NTTP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, then we're on the same page. I do not see a difference between "explicitly null" and "not initialized" here. I think that is a differentiation without a cause.