Rough first stab at addressing #85120 #85147
Closed
Conversation
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
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Doug Wyatt (dougsonos) ChangesI pointed out this issue in the review for nolock/noalloc, and @Sirraide opened #85120 Here are some (very rough) bits of code I'd written to try to address the loss of type sugar, plus a subsequent crash involving lambdas (can't remember details now, and the crash may not exist any more on main). Just in case it helps. Full diff: https://github.com/llvm/llvm-project/pull/85147.diff 4 Files Affected:
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index ff6b64c7f72d57..ca90417c9bf70b 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1301,6 +1301,11 @@ class ASTContext : public RefCountedBase<ASTContext> {
/// Change the result type of a function type once it is deduced.
void adjustDeducedFunctionResultType(FunctionDecl *FD, QualType ResultType);
+ /// Transform a function type to have the provided result type, preserving
+ /// AttributedType and MacroQualifiedType sugar.
+ QualType getFunctionTypeWithResultType(QualType OrigFuncType, QualType ResultType,
+ const FunctionProtoType::ExtProtoInfo &EPI) const;
+
/// Get a function type and produce the equivalent function type with the
/// specified exception specification. Type sugar that can be present on a
/// declaration of a function with an exception specification is permitted
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 5a8fae76a43a4d..035dc19ba7011d 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -3140,13 +3140,71 @@ const FunctionType *ASTContext::adjustFunctionType(const FunctionType *T,
return cast<FunctionType>(Result.getTypePtr());
}
+// EPI is provided by the caller because in the case of adjustDeducedFunctionResultType, it
+// is copied entirely from the previous FunctionType, but with a block (ActOnBlockStmtExpr),
+// it is more complicated...
+QualType ASTContext::getFunctionTypeWithResultType(QualType OrigFuncType, QualType ResultType,
+ const FunctionProtoType::ExtProtoInfo &EPI) const
+{
+ // Might be wrapped in a macro qualified type.
+ if (const auto *MQT = dyn_cast<MacroQualifiedType>(OrigFuncType)) {
+ return getMacroQualifiedType(
+ getFunctionTypeWithResultType(MQT->getUnderlyingType(), ResultType, EPI),
+ MQT->getMacroIdentifier());
+ }
+
+ // Might have a calling-convention attribute.
+ if (const auto *AT = dyn_cast<AttributedType>(OrigFuncType)) {
+ return getAttributedType(
+ AT->getAttrKind(),
+ getFunctionTypeWithResultType(AT->getModifiedType(), ResultType, EPI),
+ getFunctionTypeWithResultType(AT->getEquivalentType(), ResultType, EPI));
+ }
+
+ // Anything else must be a function type. Rebuild it with the new return value.
+ const auto *FPT = OrigFuncType->castAs<FunctionProtoType>();
+ return getFunctionType(ResultType, FPT->getParamTypes(), EPI);
+}
+
+#if 0
+static void examineType(const char* prefix, QualType QT, const char* term)
+{
+ llvm::outs() << prefix;
+ if (const auto *MQT = dyn_cast<MacroQualifiedType>(QT)) {
+ examineType( "MacroQualifiedType <", MQT->getUnderlyingType(), ">");
+ } else if (const auto *AT = dyn_cast<AttributedType>(QT)) {
+ examineType("AttributedType <", AT->getEquivalentType(), ">");
+ } else {
+ const auto *FPT = QT->castAs<FunctionProtoType>();
+ assert(FPT);
+ llvm::outs() << QT;
+ }
+ llvm::outs() << term;
+}
+#endif
+
void ASTContext::adjustDeducedFunctionResultType(FunctionDecl *FD,
QualType ResultType) {
FD = FD->getMostRecentDecl();
while (true) {
- const auto *FPT = FD->getType()->castAs<FunctionProtoType>();
+ QualType OrigFuncType = FD->getType();
+ const auto *FPT = OrigFuncType->castAs<FunctionProtoType>();
FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
- FD->setType(getFunctionType(ResultType, FPT->getParamTypes(), EPI));
+#if 1 // my new way
+ QualType NewFuncType = getFunctionTypeWithResultType(OrigFuncType, ResultType, EPI);
+#else // original way
+ QualType NewFuncType = getFunctionType(ResultType, FPT->getParamTypes(), EPI);
+#endif
+ /*llvm::outs() << "transform " << OrigFuncType << " -> " << NewFuncType << "\n";
+ llvm::outs() << " isConstQualified " << OrigFuncType.isConstQualified() << NewFuncType.isConstQualified() << "\n";
+ llvm::outs() << " isLocalConstQualified " << OrigFuncType.isLocalConstQualified() << NewFuncType.isLocalConstQualified() << "\n";
+ llvm::outs() << " const method " << FPT->isConst() << NewFuncType->castAs<FunctionProtoType>()->isConst() << "\n";
+ llvm::outs() << " canonical " << NewFuncType.getCanonicalType() << "\n";*/
+
+ /*examineType("original ", OrigFuncType, "\n");
+ examineType("deduced ", NewFuncType, "\n");*/
+
+ FD->setType(NewFuncType);
if (FunctionDecl *Next = FD->getPreviousDecl())
FD = Next;
else
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 93f82e68ab6440..651136d7c1b1b8 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -17161,7 +17161,7 @@ ExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
EPI.TypeQuals = Qualifiers();
EPI.ExtInfo = Ext;
- BlockTy = Context.getFunctionType(RetTy, FPT->getParamTypes(), EPI);
+ BlockTy = Context.getFunctionTypeWithResultType(BSI->FunctionType, RetTy, EPI);
}
// If we don't have a function type, just build one from nothing.
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 2d22692f3ab750..9a8dac2258c1a0 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -13830,6 +13830,33 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
getSema().AddTemplateParametersToLambdaCallOperator(NewCallOperator, Class,
TPL);
+#if 0
+// Conflicting old bug fix attempt
+
+@@ -13719,10 +13719,23 @@
+ // it introduces a mapping of the original to the newly created
+ // transformed parameters.
+ TypeSourceInfo *NewCallOpTSI = nullptr;
++ // <rdar://117704214> Crash involving AttributedTypeLoc on lambda.
++ // Hack of a fix adapted from https://github.com/llvm/llvm-project/issues/58366
++ FunctionProtoTypeLoc NewCallOpFPTL;
+ {
+ TypeSourceInfo *OldCallOpTSI = E->getCallOperator()->getTypeSourceInfo();
+ auto OldCallOpFPTL =
+ OldCallOpTSI->getTypeLoc().getAs<FunctionProtoTypeLoc>();
++ AttributedTypeLoc OldCallOpATTL;
++ bool IsAttributed = false;
++ if (!OldCallOpFPTL) {
++ OldCallOpATTL = OldCallOpTSI->getTypeLoc().getAs<AttributedTypeLoc>();
++ assert(OldCallOpATTL);
++ OldCallOpFPTL =
++ OldCallOpATTL.getModifiedLoc().getAs<FunctionProtoTypeLoc>();
++ assert(OldCallOpFPTL);
++ IsAttributed = true;
++ }
+#endif
+
+
// Transform the type of the original lambda's call operator.
// The transformation MUST be done in the CurrentInstantiationScope since
// it introduces a mapping of the original to the newly created
@@ -13867,8 +13894,38 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
if (NewCallOpType.isNull())
return ExprError();
- NewCallOpTSI =
- NewCallOpTLBuilder.getTypeSourceInfo(getSema().Context, NewCallOpType);
+
+ // NewCallOpTSI =
+ // NewCallOpTLBuilder.getTypeSourceInfo(getSema().Context, NewCallOpType);
+ if (IsAttributed) {
+ const AttributedType *oldType = OldCallOpATTL.getTypePtr();
+
+ const Attr *newAttr = getDerived().TransformAttr(OldCallOpATTL.getAttr());
+ if (!newAttr)
+ return ExprError();
+
+ QualType equivalentType =
+ getDerived().TransformType(oldType->getEquivalentType());
+ if (equivalentType.isNull())
+ return ExprError();
+ QualType result = SemaRef.Context.getAttributedType(
+ OldCallOpATTL.getAttrKind(), NewCallOpType, equivalentType);
+
+ AttributedTypeLoc newTL =
+ NewCallOpTLBuilder.push<AttributedTypeLoc>(result);
+ newTL.setAttr(newAttr);
+
+ NewCallOpTSI =
+ NewCallOpTLBuilder.getTypeSourceInfo(getSema().Context, result);
+ NewCallOpFPTL = NewCallOpTSI->getTypeLoc()
+ .castAs<AttributedTypeLoc>()
+ .getModifiedLoc()
+ .castAs<FunctionProtoTypeLoc>();
+ } else {
+ NewCallOpTSI = NewCallOpTLBuilder.getTypeSourceInfo(getSema().Context,
+ NewCallOpType);
+ NewCallOpFPTL = NewCallOpTSI->getTypeLoc().castAs<FunctionProtoTypeLoc>();
+ }
}
ArrayRef<ParmVarDecl *> Params;
|
You can test this locally with the following command:git-clang-format --diff ad23127222fe23e28ac3deaa16f3ae64d13b7b6f 613f04e311f083c129acaa4598cbfd9894fe3805 -- clang/include/clang/AST/ASTContext.h clang/lib/AST/ASTContext.cpp clang/lib/Sema/SemaExpr.cpp clang/lib/Sema/TreeTransform.hView the diff from clang-format here.diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index ca90417c9b..adffdc00f6 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1303,8 +1303,9 @@ public:
/// Transform a function type to have the provided result type, preserving
/// AttributedType and MacroQualifiedType sugar.
- QualType getFunctionTypeWithResultType(QualType OrigFuncType, QualType ResultType,
- const FunctionProtoType::ExtProtoInfo &EPI) const;
+ QualType getFunctionTypeWithResultType(
+ QualType OrigFuncType, QualType ResultType,
+ const FunctionProtoType::ExtProtoInfo &EPI) const;
/// Get a function type and produce the equivalent function type with the
/// specified exception specification. Type sugar that can be present on a
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 035dc19ba7..04ea684b13 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -3140,17 +3140,18 @@ const FunctionType *ASTContext::adjustFunctionType(const FunctionType *T,
return cast<FunctionType>(Result.getTypePtr());
}
-// EPI is provided by the caller because in the case of adjustDeducedFunctionResultType, it
-// is copied entirely from the previous FunctionType, but with a block (ActOnBlockStmtExpr),
-// it is more complicated...
-QualType ASTContext::getFunctionTypeWithResultType(QualType OrigFuncType, QualType ResultType,
- const FunctionProtoType::ExtProtoInfo &EPI) const
-{
+// EPI is provided by the caller because in the case of
+// adjustDeducedFunctionResultType, it is copied entirely from the previous
+// FunctionType, but with a block (ActOnBlockStmtExpr), it is more
+// complicated...
+QualType ASTContext::getFunctionTypeWithResultType(
+ QualType OrigFuncType, QualType ResultType,
+ const FunctionProtoType::ExtProtoInfo &EPI) const {
// Might be wrapped in a macro qualified type.
if (const auto *MQT = dyn_cast<MacroQualifiedType>(OrigFuncType)) {
- return getMacroQualifiedType(
- getFunctionTypeWithResultType(MQT->getUnderlyingType(), ResultType, EPI),
- MQT->getMacroIdentifier());
+ return getMacroQualifiedType(getFunctionTypeWithResultType(
+ MQT->getUnderlyingType(), ResultType, EPI),
+ MQT->getMacroIdentifier());
}
// Might have a calling-convention attribute.
@@ -3158,10 +3159,12 @@ QualType ASTContext::getFunctionTypeWithResultType(QualType OrigFuncType, QualTy
return getAttributedType(
AT->getAttrKind(),
getFunctionTypeWithResultType(AT->getModifiedType(), ResultType, EPI),
- getFunctionTypeWithResultType(AT->getEquivalentType(), ResultType, EPI));
+ getFunctionTypeWithResultType(AT->getEquivalentType(), ResultType,
+ EPI));
}
-
- // Anything else must be a function type. Rebuild it with the new return value.
+
+ // Anything else must be a function type. Rebuild it with the new return
+ // value.
const auto *FPT = OrigFuncType->castAs<FunctionProtoType>();
return getFunctionType(ResultType, FPT->getParamTypes(), EPI);
}
@@ -3191,15 +3194,20 @@ void ASTContext::adjustDeducedFunctionResultType(FunctionDecl *FD,
const auto *FPT = OrigFuncType->castAs<FunctionProtoType>();
FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
#if 1 // my new way
- QualType NewFuncType = getFunctionTypeWithResultType(OrigFuncType, ResultType, EPI);
+ QualType NewFuncType =
+ getFunctionTypeWithResultType(OrigFuncType, ResultType, EPI);
#else // original way
- QualType NewFuncType = getFunctionType(ResultType, FPT->getParamTypes(), EPI);
+ QualType NewFuncType =
+ getFunctionType(ResultType, FPT->getParamTypes(), EPI);
#endif
- /*llvm::outs() << "transform " << OrigFuncType << " -> " << NewFuncType << "\n";
- llvm::outs() << " isConstQualified " << OrigFuncType.isConstQualified() << NewFuncType.isConstQualified() << "\n";
- llvm::outs() << " isLocalConstQualified " << OrigFuncType.isLocalConstQualified() << NewFuncType.isLocalConstQualified() << "\n";
- llvm::outs() << " const method " << FPT->isConst() << NewFuncType->castAs<FunctionProtoType>()->isConst() << "\n";
- llvm::outs() << " canonical " << NewFuncType.getCanonicalType() << "\n";*/
+ /*llvm::outs() << "transform " << OrigFuncType << " -> " << NewFuncType <<
+ "\n"; llvm::outs() << " isConstQualified " <<
+ OrigFuncType.isConstQualified() << NewFuncType.isConstQualified() << "\n";
+ llvm::outs() << " isLocalConstQualified " <<
+ OrigFuncType.isLocalConstQualified() << NewFuncType.isLocalConstQualified()
+ << "\n"; llvm::outs() << " const method " << FPT->isConst() <<
+ NewFuncType->castAs<FunctionProtoType>()->isConst() << "\n"; llvm::outs() <<
+ " canonical " << NewFuncType.getCanonicalType() << "\n";*/
/*examineType("original ", OrigFuncType, "\n");
examineType("deduced ", NewFuncType, "\n");*/
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 651136d7c1..123a2bee69 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -17161,7 +17161,8 @@ ExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
EPI.TypeQuals = Qualifiers();
EPI.ExtInfo = Ext;
- BlockTy = Context.getFunctionTypeWithResultType(BSI->FunctionType, RetTy, EPI);
+ BlockTy =
+ Context.getFunctionTypeWithResultType(BSI->FunctionType, RetTy, EPI);
}
// If we don't have a function type, just build one from nothing.
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 9a8dac2258..de12f042a7 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -13856,7 +13856,6 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
+ }
#endif
-
// Transform the type of the original lambda's call operator.
// The transformation MUST be done in the CurrentInstantiationScope since
// it introduces a mapping of the original to the newly created
@@ -13896,7 +13895,8 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
return ExprError();
// NewCallOpTSI =
- // NewCallOpTLBuilder.getTypeSourceInfo(getSema().Context, NewCallOpType);
+ // NewCallOpTLBuilder.getTypeSourceInfo(getSema().Context,
+ // NewCallOpType);
if (IsAttributed) {
const AttributedType *oldType = OldCallOpATTL.getTypePtr();
|
|
@dougsonos I may have some time to look into this since there are probably (as always) some annoying edge cases—particularly w/ templates. Would you be fine with me committing to this branch or would it be easier for you if I made a separate branch for that? Either is fine by me. |
There definitely is a crash involving templated lambdas on trunk: #85154 |
It might be simplest if you make your own branch, borrowing whatever from mine helps. Thanks. |
Yeah, it turns out there are some general issues w/ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I pointed out this issue in the review for nolock/noalloc, and @Sirraide opened #85120
Here are some (very rough) bits of code I'd written to try to address the loss of type sugar, plus a subsequent crash involving lambdas (can't remember details now, and the crash may not exist any more on main).
Just in case it helps.