-
Notifications
You must be signed in to change notification settings - Fork 13.3k
[Clang][Sema] Fix the lambda call expression inside of a type alias declaration #82310
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
25f493d
The lambda call inside of a type alias
zyn0217 303fd1b
Format & Comments
zyn0217 62a00af
fixup
zyn0217 d3a76a1
Slightly refactor & Fix GH82104
zyn0217 be41641
Mention GH82104 in the release note
zyn0217 2e95a41
Rephrase & Format
zyn0217 4fcafbe
Validate deduced types
zyn0217 ba8e948
Merge branch 'main' into lambda-call-inside-of-type-aliases
zyn0217 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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 |
---|---|---|
|
@@ -767,6 +767,12 @@ class TreeTransform { | |
/// the body. | ||
StmtResult SkipLambdaBody(LambdaExpr *E, Stmt *Body); | ||
|
||
CXXRecordDecl::LambdaDependencyKind | ||
ComputeLambdaDependency(LambdaScopeInfo *LSI) { | ||
return static_cast<CXXRecordDecl::LambdaDependencyKind>( | ||
LSI->Lambda->getLambdaDependencyKind()); | ||
} | ||
|
||
QualType TransformReferenceType(TypeLocBuilder &TLB, ReferenceTypeLoc TL); | ||
|
||
StmtResult TransformCompoundStmt(CompoundStmt *S, bool IsStmtExpr); | ||
|
@@ -13936,6 +13942,46 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) { | |
/*IsInstantiation*/ true); | ||
SavedContext.pop(); | ||
|
||
// Recompute the dependency of the lambda so that we can defer the lambda call | ||
// construction until after we have all the necessary template arguments. For | ||
// example, given | ||
// | ||
// template <class> struct S { | ||
// template <class U> | ||
// using Type = decltype([](U){}(42.0)); | ||
// }; | ||
// void foo() { | ||
// using T = S<int>::Type<float>; | ||
// ^~~~~~ | ||
// } | ||
// | ||
// We would end up here from instantiating S<int> when ensuring its | ||
// completeness. That would transform the lambda call expression regardless of | ||
// the absence of the corresponding argument for U. | ||
// | ||
// Going ahead with unsubstituted type U makes things worse: we would soon | ||
// compare the argument type (which is float) against the parameter U | ||
// somewhere in Sema::BuildCallExpr. Then we would quickly run into a bogus | ||
// error suggesting unmatched types 'U' and 'float'! | ||
// | ||
// That said, everything will be fine if we defer that semantic checking. | ||
// Fortunately, we have such a mechanism that bypasses it if the CallExpr is | ||
// dependent. Since the CallExpr's dependency boils down to the lambda's | ||
// dependency in this case, we can harness that by recomputing the dependency | ||
// from the instantiation arguments. | ||
// | ||
// FIXME: Creating the type of a lambda requires us to have a dependency | ||
// value, which happens before its substitution. We update its dependency | ||
// *after* the substitution in case we can't decide the dependency | ||
// so early, e.g. because we want to see if any of the *substituted* | ||
// parameters are dependent. | ||
DependencyKind = getDerived().ComputeLambdaDependency(&LSICopy); | ||
Class->setLambdaDependencyKind(DependencyKind); | ||
// Clean up the type cache created previously. Then, we re-create a type for | ||
// such Decl with the new DependencyKind. | ||
Class->setTypeForDecl(nullptr); | ||
getSema().Context.getTypeDeclType(Class); | ||
|
||
Comment on lines
+13978
to
+13984
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not great, but I am not sure how we can improve. |
||
return getSema().BuildLambdaExpr(E->getBeginLoc(), Body.get()->getEndLoc(), | ||
&LSICopy); | ||
} | ||
|
Oops, something went wrong.
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.
What cases do we have where this takes more than 1 step? I also wonder if we'd be better off finding the primary
CXXRecordDecl
, then picking it up from there? We could usegetTemplateInstantiationPattern
thengetDescribedClassTemplate
I think?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.
(Sorry for replying this late, I just got around to this PR.)
The lambda itself might be defined within a template, and I presume transforming that template introduces multilevel mappings.
If we examine
TemplateInstantiator::transformedLocalDecl
, we'd see there's only handling forCXXMethodDecls
of adding such mappings that we can extract from the Decls themselves. I think it is possible that we can use the primaryCXXRecordDecl
approach, although looking for that Decl might involve handlingLocalInstantiationScopes
, which IMO is not super straightforward.