[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
Sirraide wrote: ping https://github.com/llvm/llvm-project/pull/85325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
Sirraide wrote: ping https://github.com/llvm/llvm-project/pull/85325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
@@ -3140,13 +3140,35 @@ const FunctionType
*ASTContext::adjustFunctionType(const FunctionType *T,
return cast(Result.getTypePtr());
}
+QualType ASTContext::adjustFunctionResultType(QualType FunctionType,
+ QualType ResultType) {
+ // Might be wrapped in a macro qualified type.
+ if (const auto *MQT = dyn_cast(FunctionType)) {
+return getMacroQualifiedType(
+adjustFunctionResultType(MQT->getUnderlyingType(), ResultType),
+MQT->getMacroIdentifier());
+ }
+
+ // Might have a calling-convention attribute.
+ if (const auto *AT = dyn_cast(FunctionType)) {
+return getAttributedType(
+AT->getAttrKind(),
+adjustFunctionResultType(AT->getModifiedType(), ResultType),
+adjustFunctionResultType(AT->getEquivalentType(), ResultType));
+ }
+
+ // Anything else must be a function type. Rebuild it with the new return
+ // value.
Sirraide wrote:
Just refactored this and extracted the code that handles the sugar into a
separate function (see the latest commit).
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
Sirraide wrote: Seeing as there are now two places where we want to adjust function types but also preserve sugar, I’ve gone ahead and added a generic `adjustType()` function that handles all of the sugar and takes a `function_ref` to perform the actual adjustment. The intent here is that, in the future, functions like `getFunctionTypeWithExceptionSpec()` should be able to delegate most of the work to that function instead. https://github.com/llvm/llvm-project/pull/85325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
https://github.com/Sirraide updated
https://github.com/llvm/llvm-project/pull/85325
>From 907210a3ad3d829a8e49a5c976d129f8653801bf Mon Sep 17 00:00:00 2001
From: Sirraide
Date: Thu, 14 Mar 2024 18:24:37 +0100
Subject: [PATCH 01/11] [Clang] Add `dump()` method for `Attr`
---
clang/include/clang/AST/Attr.h | 2 ++
clang/lib/AST/ASTDumper.cpp| 8
2 files changed, 10 insertions(+)
diff --git a/clang/include/clang/AST/Attr.h b/clang/include/clang/AST/Attr.h
index 8e9b7ad8b46826..6400023947863f 100644
--- a/clang/include/clang/AST/Attr.h
+++ b/clang/include/clang/AST/Attr.h
@@ -112,6 +112,8 @@ class Attr : public AttributeCommonInfo {
// Pretty print this attribute.
void printPretty(raw_ostream , const PrintingPolicy ) const;
+ void dump() const;
+
static StringRef getDocumentation(attr::Kind);
};
diff --git a/clang/lib/AST/ASTDumper.cpp b/clang/lib/AST/ASTDumper.cpp
index 6efc5bb92e28d2..8d8b8621341ef7 100644
--- a/clang/lib/AST/ASTDumper.cpp
+++ b/clang/lib/AST/ASTDumper.cpp
@@ -361,3 +361,11 @@ LLVM_DUMP_METHOD void ConceptReference::dump(raw_ostream
) const {
ASTDumper P(OS, Ctx, Ctx.getDiagnostics().getShowColors());
P.Visit(this);
}
+
+//===--===//
+// Attr method implementations
+//===--===//
+LLVM_DUMP_METHOD void Attr::dump() const {
+ ASTDumper P(llvm::errs(), /*ShowColors=*/false);
+ P.Visit(this);
+}
>From d719a7605c89ed4ea88734b5386b6009931450f6 Mon Sep 17 00:00:00 2001
From: Sirraide
Date: Thu, 14 Mar 2024 18:25:18 +0100
Subject: [PATCH 02/11] [Clang] Do not instantiate the same (FunctionProto)Type
twice
---
clang/lib/Sema/TreeTransform.h | 22 --
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 2d22692f3ab750..cf781792935f18 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -7231,12 +7231,22 @@ QualType
TreeTransform::TransformAttributedType(
// FIXME: dependent operand expressions?
if (getDerived().AlwaysRebuild() ||
modifiedType != oldType->getModifiedType()) {
-TypeLocBuilder AuxiliaryTLB;
-AuxiliaryTLB.reserve(TL.getFullDataSize());
-QualType equivalentType =
-getDerived().TransformType(AuxiliaryTLB, TL.getEquivalentTypeLoc());
-if (equivalentType.isNull())
- return QualType();
+// Do not transform the equivalent type if it is equal to the modified
type.
+//
+// This is because, 1. it’s the same type, instantiating it again will
yield
+// the same result anyway, and if it doesn't, then that could be a bug in
+// and of itself, and 2. instantiating the same TypeLoc twice is a really
+// bad idea if it's a FunctionProtoType, because instantiating the same
+// ParmVarDecls twice will cause assertion failures.
+QualType equivalentType = modifiedType;
+if (TL.getModifiedLoc().getType() != TL.getEquivalentTypeLoc().getType()) {
+ TypeLocBuilder AuxiliaryTLB;
+ AuxiliaryTLB.reserve(TL.getFullDataSize());
+ equivalentType =
+ getDerived().TransformType(AuxiliaryTLB, TL.getEquivalentTypeLoc());
+ if (equivalentType.isNull())
+return QualType();
+}
// Check whether we can add nullability; it is only represented as
// type sugar, and therefore cannot be diagnosed in any other way.
>From a9c753ab46b40bd7da6f27eb11655fe43acb11de Mon Sep 17 00:00:00 2001
From: Sirraide
Date: Thu, 14 Mar 2024 21:40:41 +0100
Subject: [PATCH 03/11] [Clang] Refactor instantiation of a lambda's
FunctionProtoType
-
Co-authored-by: Yuxuan Chen
---
clang/include/clang/Sema/Template.h| 14 +++-
clang/lib/Sema/SemaTemplateInstantiate.cpp | 13 +++-
clang/lib/Sema/TreeTransform.h | 90 +++---
3 files changed, 49 insertions(+), 68 deletions(-)
diff --git a/clang/include/clang/Sema/Template.h
b/clang/include/clang/Sema/Template.h
index ce44aca797b0fb..8c379f51ca3d5d 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -411,6 +411,11 @@ enum class TemplateSubstitutionKind : char {
/// lookup will search our outer scope.
bool CombineWithOuterScope;
+/// Whether this scope is being used to instantiate a lambda expression,
+/// in which case it should be reused for instantiating the lambda's
+/// FunctionProtoType.
+bool InstantiatingLambda = false;
+
/// If non-NULL, the template parameter pack that has been
/// partially substituted per C++0x [temp.arg.explicit]p9.
NamedDecl *PartiallySubstitutedPack = nullptr;
@@ -425,9 +430,11 @@ enum class TemplateSubstitutionKind : char {
unsigned NumArgsInPartiallySubstitutedPack;
public:
-LocalInstantiationScope(Sema , bool CombineWithOuterScope = false)
+LocalInstantiationScope(Sema , bool
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
https://github.com/dougsonos edited https://github.com/llvm/llvm-project/pull/85325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
@@ -3140,13 +3140,35 @@ const FunctionType
*ASTContext::adjustFunctionType(const FunctionType *T,
return cast(Result.getTypePtr());
}
+QualType ASTContext::adjustFunctionResultType(QualType FunctionType,
+ QualType ResultType) {
+ // Might be wrapped in a macro qualified type.
+ if (const auto *MQT = dyn_cast(FunctionType)) {
+return getMacroQualifiedType(
+adjustFunctionResultType(MQT->getUnderlyingType(), ResultType),
+MQT->getMacroIdentifier());
+ }
+
+ // Might have a calling-convention attribute.
+ if (const auto *AT = dyn_cast(FunctionType)) {
+return getAttributedType(
+AT->getAttrKind(),
+adjustFunctionResultType(AT->getModifiedType(), ResultType),
+adjustFunctionResultType(AT->getEquivalentType(), ResultType));
+ }
+
+ // Anything else must be a function type. Rebuild it with the new return
+ // value.
dougsonos wrote:
> Also, I just noticed, this code seems to be modelled after
> `getFunctionTypeWithExceptionSpec()`, which is like two functions down in the
> same file and which does pretty much the same thing that this is doing here.
> It might make sense to merge the two.
I was hacking just enough to make it work, apologies.
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
@@ -3140,13 +3140,35 @@ const FunctionType
*ASTContext::adjustFunctionType(const FunctionType *T,
return cast(Result.getTypePtr());
}
+QualType ASTContext::adjustFunctionResultType(QualType FunctionType,
+ QualType ResultType) {
+ // Might be wrapped in a macro qualified type.
+ if (const auto *MQT = dyn_cast(FunctionType)) {
+return getMacroQualifiedType(
+adjustFunctionResultType(MQT->getUnderlyingType(), ResultType),
+MQT->getMacroIdentifier());
+ }
+
+ // Might have a calling-convention attribute.
+ if (const auto *AT = dyn_cast(FunctionType)) {
+return getAttributedType(
+AT->getAttrKind(),
+adjustFunctionResultType(AT->getModifiedType(), ResultType),
+adjustFunctionResultType(AT->getEquivalentType(), ResultType));
+ }
+
+ // Anything else must be a function type. Rebuild it with the new return
+ // value.
Sirraide wrote:
Also, I just noticed, this code seems to be modelled after
`getFunctionTypeWithExceptionSpec()`, which is like two functions down in the
same file and which does pretty much the same thing that this is doing here. It
might make sense to merge the two.
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
@@ -3140,13 +3140,35 @@ const FunctionType
*ASTContext::adjustFunctionType(const FunctionType *T,
return cast(Result.getTypePtr());
}
+QualType ASTContext::adjustFunctionResultType(QualType FunctionType,
+ QualType ResultType) {
+ // Might be wrapped in a macro qualified type.
+ if (const auto *MQT = dyn_cast(FunctionType)) {
+return getMacroQualifiedType(
+adjustFunctionResultType(MQT->getUnderlyingType(), ResultType),
+MQT->getMacroIdentifier());
+ }
+
+ // Might have a calling-convention attribute.
+ if (const auto *AT = dyn_cast(FunctionType)) {
+return getAttributedType(
+AT->getAttrKind(),
+adjustFunctionResultType(AT->getModifiedType(), ResultType),
+adjustFunctionResultType(AT->getEquivalentType(), ResultType));
+ }
+
+ // Anything else must be a function type. Rebuild it with the new return
+ // value.
Sirraide wrote:
The thing is, I’m not sure how well an AST visitor would work for this specific
use case (rebuilding a function type with a different result type).
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
@@ -3140,13 +3140,35 @@ const FunctionType
*ASTContext::adjustFunctionType(const FunctionType *T,
return cast(Result.getTypePtr());
}
+QualType ASTContext::adjustFunctionResultType(QualType FunctionType,
+ QualType ResultType) {
+ // Might be wrapped in a macro qualified type.
+ if (const auto *MQT = dyn_cast(FunctionType)) {
+return getMacroQualifiedType(
+adjustFunctionResultType(MQT->getUnderlyingType(), ResultType),
+MQT->getMacroIdentifier());
+ }
+
+ // Might have a calling-convention attribute.
+ if (const auto *AT = dyn_cast(FunctionType)) {
+return getAttributedType(
+AT->getAttrKind(),
+adjustFunctionResultType(AT->getModifiedType(), ResultType),
+adjustFunctionResultType(AT->getEquivalentType(), ResultType));
+ }
+
+ // Anything else must be a function type. Rebuild it with the new return
+ // value.
Sirraide wrote:
Also, I just noticed, `RecursiveASTVisitor` *does* seem to support `Type`s and
`TypeLoc`s? I’ll take another look at that.
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
https://github.com/Sirraide updated
https://github.com/llvm/llvm-project/pull/85325
>From 907210a3ad3d829a8e49a5c976d129f8653801bf Mon Sep 17 00:00:00 2001
From: Sirraide
Date: Thu, 14 Mar 2024 18:24:37 +0100
Subject: [PATCH 01/10] [Clang] Add `dump()` method for `Attr`
---
clang/include/clang/AST/Attr.h | 2 ++
clang/lib/AST/ASTDumper.cpp| 8
2 files changed, 10 insertions(+)
diff --git a/clang/include/clang/AST/Attr.h b/clang/include/clang/AST/Attr.h
index 8e9b7ad8b46826..6400023947863f 100644
--- a/clang/include/clang/AST/Attr.h
+++ b/clang/include/clang/AST/Attr.h
@@ -112,6 +112,8 @@ class Attr : public AttributeCommonInfo {
// Pretty print this attribute.
void printPretty(raw_ostream , const PrintingPolicy ) const;
+ void dump() const;
+
static StringRef getDocumentation(attr::Kind);
};
diff --git a/clang/lib/AST/ASTDumper.cpp b/clang/lib/AST/ASTDumper.cpp
index 6efc5bb92e28d2..8d8b8621341ef7 100644
--- a/clang/lib/AST/ASTDumper.cpp
+++ b/clang/lib/AST/ASTDumper.cpp
@@ -361,3 +361,11 @@ LLVM_DUMP_METHOD void ConceptReference::dump(raw_ostream
) const {
ASTDumper P(OS, Ctx, Ctx.getDiagnostics().getShowColors());
P.Visit(this);
}
+
+//===--===//
+// Attr method implementations
+//===--===//
+LLVM_DUMP_METHOD void Attr::dump() const {
+ ASTDumper P(llvm::errs(), /*ShowColors=*/false);
+ P.Visit(this);
+}
>From d719a7605c89ed4ea88734b5386b6009931450f6 Mon Sep 17 00:00:00 2001
From: Sirraide
Date: Thu, 14 Mar 2024 18:25:18 +0100
Subject: [PATCH 02/10] [Clang] Do not instantiate the same (FunctionProto)Type
twice
---
clang/lib/Sema/TreeTransform.h | 22 --
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 2d22692f3ab750..cf781792935f18 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -7231,12 +7231,22 @@ QualType
TreeTransform::TransformAttributedType(
// FIXME: dependent operand expressions?
if (getDerived().AlwaysRebuild() ||
modifiedType != oldType->getModifiedType()) {
-TypeLocBuilder AuxiliaryTLB;
-AuxiliaryTLB.reserve(TL.getFullDataSize());
-QualType equivalentType =
-getDerived().TransformType(AuxiliaryTLB, TL.getEquivalentTypeLoc());
-if (equivalentType.isNull())
- return QualType();
+// Do not transform the equivalent type if it is equal to the modified
type.
+//
+// This is because, 1. it’s the same type, instantiating it again will
yield
+// the same result anyway, and if it doesn't, then that could be a bug in
+// and of itself, and 2. instantiating the same TypeLoc twice is a really
+// bad idea if it's a FunctionProtoType, because instantiating the same
+// ParmVarDecls twice will cause assertion failures.
+QualType equivalentType = modifiedType;
+if (TL.getModifiedLoc().getType() != TL.getEquivalentTypeLoc().getType()) {
+ TypeLocBuilder AuxiliaryTLB;
+ AuxiliaryTLB.reserve(TL.getFullDataSize());
+ equivalentType =
+ getDerived().TransformType(AuxiliaryTLB, TL.getEquivalentTypeLoc());
+ if (equivalentType.isNull())
+return QualType();
+}
// Check whether we can add nullability; it is only represented as
// type sugar, and therefore cannot be diagnosed in any other way.
>From a9c753ab46b40bd7da6f27eb11655fe43acb11de Mon Sep 17 00:00:00 2001
From: Sirraide
Date: Thu, 14 Mar 2024 21:40:41 +0100
Subject: [PATCH 03/10] [Clang] Refactor instantiation of a lambda's
FunctionProtoType
-
Co-authored-by: Yuxuan Chen
---
clang/include/clang/Sema/Template.h| 14 +++-
clang/lib/Sema/SemaTemplateInstantiate.cpp | 13 +++-
clang/lib/Sema/TreeTransform.h | 90 +++---
3 files changed, 49 insertions(+), 68 deletions(-)
diff --git a/clang/include/clang/Sema/Template.h
b/clang/include/clang/Sema/Template.h
index ce44aca797b0fb..8c379f51ca3d5d 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -411,6 +411,11 @@ enum class TemplateSubstitutionKind : char {
/// lookup will search our outer scope.
bool CombineWithOuterScope;
+/// Whether this scope is being used to instantiate a lambda expression,
+/// in which case it should be reused for instantiating the lambda's
+/// FunctionProtoType.
+bool InstantiatingLambda = false;
+
/// If non-NULL, the template parameter pack that has been
/// partially substituted per C++0x [temp.arg.explicit]p9.
NamedDecl *PartiallySubstitutedPack = nullptr;
@@ -425,9 +430,11 @@ enum class TemplateSubstitutionKind : char {
unsigned NumArgsInPartiallySubstitutedPack;
public:
-LocalInstantiationScope(Sema , bool CombineWithOuterScope = false)
+LocalInstantiationScope(Sema , bool
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff bde7a6b791872b63456cb4e50e63046728a65196
d6a57b5a5aa123cf38ce3657b764c74c4c5a86f7 --
clang/test/SemaCXX/lambda-attributes.cpp clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/Attr.h clang/include/clang/Sema/Template.h
clang/lib/AST/ASTContext.cpp clang/lib/AST/ASTDumper.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp clang/lib/Sema/TreeTransform.h
clang/test/SemaCXX/lambda-conversion-op-cc.cpp
``
View the diff from clang-format here.
``diff
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index c83e0b0094..079821c053 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -13840,8 +13840,8 @@ TreeTransform::TransformLambdaExpr(LambdaExpr
*E) {
E->getCallOperator()->getInnerLocStart(),
E->getCallOperator()->getTrailingRequiresClause(), NewCallOpTSI,
E->getCallOperator()->getConstexprKind(),
- E->getCallOperator()->getStorageClass(),
- FPTL.getParams(), E->hasExplicitResultType());
+ E->getCallOperator()->getStorageClass(), FPTL.getParams(),
+ E->hasExplicitResultType());
getDerived().transformAttrs(E->getCallOperator(), NewCallOperator);
getDerived().transformedLocalDecl(E->getCallOperator(), {NewCallOperator});
``
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
@@ -3140,13 +3140,35 @@ const FunctionType
*ASTContext::adjustFunctionType(const FunctionType *T,
return cast(Result.getTypePtr());
}
+QualType ASTContext::adjustFunctionResultType(QualType FunctionType,
+ QualType ResultType) {
+ // Might be wrapped in a macro qualified type.
+ if (const auto *MQT = dyn_cast(FunctionType)) {
+return getMacroQualifiedType(
+adjustFunctionResultType(MQT->getUnderlyingType(), ResultType),
+MQT->getMacroIdentifier());
+ }
+
+ // Might have a calling-convention attribute.
+ if (const auto *AT = dyn_cast(FunctionType)) {
+return getAttributedType(
+AT->getAttrKind(),
+adjustFunctionResultType(AT->getModifiedType(), ResultType),
+adjustFunctionResultType(AT->getEquivalentType(), ResultType));
+ }
+
+ // Anything else must be a function type. Rebuild it with the new return
+ // value.
Sirraide wrote:
Yup, that works in tree transform, so that we shouldn’t have to worry about
that part anymore.
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
https://github.com/Sirraide updated
https://github.com/llvm/llvm-project/pull/85325
>From 907210a3ad3d829a8e49a5c976d129f8653801bf Mon Sep 17 00:00:00 2001
From: Sirraide
Date: Thu, 14 Mar 2024 18:24:37 +0100
Subject: [PATCH 1/9] [Clang] Add `dump()` method for `Attr`
---
clang/include/clang/AST/Attr.h | 2 ++
clang/lib/AST/ASTDumper.cpp| 8
2 files changed, 10 insertions(+)
diff --git a/clang/include/clang/AST/Attr.h b/clang/include/clang/AST/Attr.h
index 8e9b7ad8b46826..6400023947863f 100644
--- a/clang/include/clang/AST/Attr.h
+++ b/clang/include/clang/AST/Attr.h
@@ -112,6 +112,8 @@ class Attr : public AttributeCommonInfo {
// Pretty print this attribute.
void printPretty(raw_ostream , const PrintingPolicy ) const;
+ void dump() const;
+
static StringRef getDocumentation(attr::Kind);
};
diff --git a/clang/lib/AST/ASTDumper.cpp b/clang/lib/AST/ASTDumper.cpp
index 6efc5bb92e28d2..8d8b8621341ef7 100644
--- a/clang/lib/AST/ASTDumper.cpp
+++ b/clang/lib/AST/ASTDumper.cpp
@@ -361,3 +361,11 @@ LLVM_DUMP_METHOD void ConceptReference::dump(raw_ostream
) const {
ASTDumper P(OS, Ctx, Ctx.getDiagnostics().getShowColors());
P.Visit(this);
}
+
+//===--===//
+// Attr method implementations
+//===--===//
+LLVM_DUMP_METHOD void Attr::dump() const {
+ ASTDumper P(llvm::errs(), /*ShowColors=*/false);
+ P.Visit(this);
+}
>From d719a7605c89ed4ea88734b5386b6009931450f6 Mon Sep 17 00:00:00 2001
From: Sirraide
Date: Thu, 14 Mar 2024 18:25:18 +0100
Subject: [PATCH 2/9] [Clang] Do not instantiate the same (FunctionProto)Type
twice
---
clang/lib/Sema/TreeTransform.h | 22 --
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 2d22692f3ab750..cf781792935f18 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -7231,12 +7231,22 @@ QualType
TreeTransform::TransformAttributedType(
// FIXME: dependent operand expressions?
if (getDerived().AlwaysRebuild() ||
modifiedType != oldType->getModifiedType()) {
-TypeLocBuilder AuxiliaryTLB;
-AuxiliaryTLB.reserve(TL.getFullDataSize());
-QualType equivalentType =
-getDerived().TransformType(AuxiliaryTLB, TL.getEquivalentTypeLoc());
-if (equivalentType.isNull())
- return QualType();
+// Do not transform the equivalent type if it is equal to the modified
type.
+//
+// This is because, 1. it’s the same type, instantiating it again will
yield
+// the same result anyway, and if it doesn't, then that could be a bug in
+// and of itself, and 2. instantiating the same TypeLoc twice is a really
+// bad idea if it's a FunctionProtoType, because instantiating the same
+// ParmVarDecls twice will cause assertion failures.
+QualType equivalentType = modifiedType;
+if (TL.getModifiedLoc().getType() != TL.getEquivalentTypeLoc().getType()) {
+ TypeLocBuilder AuxiliaryTLB;
+ AuxiliaryTLB.reserve(TL.getFullDataSize());
+ equivalentType =
+ getDerived().TransformType(AuxiliaryTLB, TL.getEquivalentTypeLoc());
+ if (equivalentType.isNull())
+return QualType();
+}
// Check whether we can add nullability; it is only represented as
// type sugar, and therefore cannot be diagnosed in any other way.
>From a9c753ab46b40bd7da6f27eb11655fe43acb11de Mon Sep 17 00:00:00 2001
From: Sirraide
Date: Thu, 14 Mar 2024 21:40:41 +0100
Subject: [PATCH 3/9] [Clang] Refactor instantiation of a lambda's
FunctionProtoType
-
Co-authored-by: Yuxuan Chen
---
clang/include/clang/Sema/Template.h| 14 +++-
clang/lib/Sema/SemaTemplateInstantiate.cpp | 13 +++-
clang/lib/Sema/TreeTransform.h | 90 +++---
3 files changed, 49 insertions(+), 68 deletions(-)
diff --git a/clang/include/clang/Sema/Template.h
b/clang/include/clang/Sema/Template.h
index ce44aca797b0fb..8c379f51ca3d5d 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -411,6 +411,11 @@ enum class TemplateSubstitutionKind : char {
/// lookup will search our outer scope.
bool CombineWithOuterScope;
+/// Whether this scope is being used to instantiate a lambda expression,
+/// in which case it should be reused for instantiating the lambda's
+/// FunctionProtoType.
+bool InstantiatingLambda = false;
+
/// If non-NULL, the template parameter pack that has been
/// partially substituted per C++0x [temp.arg.explicit]p9.
NamedDecl *PartiallySubstitutedPack = nullptr;
@@ -425,9 +430,11 @@ enum class TemplateSubstitutionKind : char {
unsigned NumArgsInPartiallySubstitutedPack;
public:
-LocalInstantiationScope(Sema , bool CombineWithOuterScope = false)
+LocalInstantiationScope(Sema , bool
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
@@ -3140,13 +3140,35 @@ const FunctionType
*ASTContext::adjustFunctionType(const FunctionType *T,
return cast(Result.getTypePtr());
}
+QualType ASTContext::adjustFunctionResultType(QualType FunctionType,
+ QualType ResultType) {
+ // Might be wrapped in a macro qualified type.
+ if (const auto *MQT = dyn_cast(FunctionType)) {
+return getMacroQualifiedType(
+adjustFunctionResultType(MQT->getUnderlyingType(), ResultType),
+MQT->getMacroIdentifier());
+ }
+
+ // Might have a calling-convention attribute.
+ if (const auto *AT = dyn_cast(FunctionType)) {
+return getAttributedType(
+AT->getAttrKind(),
+adjustFunctionResultType(AT->getModifiedType(), ResultType),
+adjustFunctionResultType(AT->getEquivalentType(), ResultType));
+ }
+
+ // Anything else must be a function type. Rebuild it with the new return
+ // value.
Sirraide wrote:
There’s `TypeLoc::getAsAdjusted()`; that might work for at least the
`TransformLambdaExpr` case.
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
@@ -3140,13 +3140,35 @@ const FunctionType
*ASTContext::adjustFunctionType(const FunctionType *T,
return cast(Result.getTypePtr());
}
+QualType ASTContext::adjustFunctionResultType(QualType FunctionType,
+ QualType ResultType) {
+ // Might be wrapped in a macro qualified type.
+ if (const auto *MQT = dyn_cast(FunctionType)) {
+return getMacroQualifiedType(
+adjustFunctionResultType(MQT->getUnderlyingType(), ResultType),
+MQT->getMacroIdentifier());
+ }
+
+ // Might have a calling-convention attribute.
+ if (const auto *AT = dyn_cast(FunctionType)) {
+return getAttributedType(
+AT->getAttrKind(),
+adjustFunctionResultType(AT->getModifiedType(), ResultType),
+adjustFunctionResultType(AT->getEquivalentType(), ResultType));
+ }
+
+ // Anything else must be a function type. Rebuild it with the new return
+ // value.
Sirraide wrote:
I’ve been thinking about this too; something like that or some way to strip
`TypeLoc`s that count as ‘sugar’; we’re doing similar things all over the
place, but from what I can tell, what is or isn’t stripped is highly dependent
on context.
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
@@ -3140,13 +3140,35 @@ const FunctionType
*ASTContext::adjustFunctionType(const FunctionType *T,
return cast(Result.getTypePtr());
}
+QualType ASTContext::adjustFunctionResultType(QualType FunctionType,
+ QualType ResultType) {
+ // Might be wrapped in a macro qualified type.
+ if (const auto *MQT = dyn_cast(FunctionType)) {
+return getMacroQualifiedType(
+adjustFunctionResultType(MQT->getUnderlyingType(), ResultType),
+MQT->getMacroIdentifier());
+ }
+
+ // Might have a calling-convention attribute.
+ if (const auto *AT = dyn_cast(FunctionType)) {
+return getAttributedType(
+AT->getAttrKind(),
+adjustFunctionResultType(AT->getModifiedType(), ResultType),
+adjustFunctionResultType(AT->getEquivalentType(), ResultType));
+ }
+
+ // Anything else must be a function type. Rebuild it with the new return
+ // value.
yuxuanchen1997 wrote:
I think what would be required for this type of work is to have the general
ability to traverse `Type`s and `TypeLoc`s. Implementation like this + the one
I provided in `TreeTransform::TransformLambdaExpr` is error prone and wouldn't
scale.
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
https://github.com/Sirraide updated
https://github.com/llvm/llvm-project/pull/85325
>From 907210a3ad3d829a8e49a5c976d129f8653801bf Mon Sep 17 00:00:00 2001
From: Sirraide
Date: Thu, 14 Mar 2024 18:24:37 +0100
Subject: [PATCH 1/8] [Clang] Add `dump()` method for `Attr`
---
clang/include/clang/AST/Attr.h | 2 ++
clang/lib/AST/ASTDumper.cpp| 8
2 files changed, 10 insertions(+)
diff --git a/clang/include/clang/AST/Attr.h b/clang/include/clang/AST/Attr.h
index 8e9b7ad8b46826..6400023947863f 100644
--- a/clang/include/clang/AST/Attr.h
+++ b/clang/include/clang/AST/Attr.h
@@ -112,6 +112,8 @@ class Attr : public AttributeCommonInfo {
// Pretty print this attribute.
void printPretty(raw_ostream , const PrintingPolicy ) const;
+ void dump() const;
+
static StringRef getDocumentation(attr::Kind);
};
diff --git a/clang/lib/AST/ASTDumper.cpp b/clang/lib/AST/ASTDumper.cpp
index 6efc5bb92e28d2..8d8b8621341ef7 100644
--- a/clang/lib/AST/ASTDumper.cpp
+++ b/clang/lib/AST/ASTDumper.cpp
@@ -361,3 +361,11 @@ LLVM_DUMP_METHOD void ConceptReference::dump(raw_ostream
) const {
ASTDumper P(OS, Ctx, Ctx.getDiagnostics().getShowColors());
P.Visit(this);
}
+
+//===--===//
+// Attr method implementations
+//===--===//
+LLVM_DUMP_METHOD void Attr::dump() const {
+ ASTDumper P(llvm::errs(), /*ShowColors=*/false);
+ P.Visit(this);
+}
>From d719a7605c89ed4ea88734b5386b6009931450f6 Mon Sep 17 00:00:00 2001
From: Sirraide
Date: Thu, 14 Mar 2024 18:25:18 +0100
Subject: [PATCH 2/8] [Clang] Do not instantiate the same (FunctionProto)Type
twice
---
clang/lib/Sema/TreeTransform.h | 22 --
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 2d22692f3ab750..cf781792935f18 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -7231,12 +7231,22 @@ QualType
TreeTransform::TransformAttributedType(
// FIXME: dependent operand expressions?
if (getDerived().AlwaysRebuild() ||
modifiedType != oldType->getModifiedType()) {
-TypeLocBuilder AuxiliaryTLB;
-AuxiliaryTLB.reserve(TL.getFullDataSize());
-QualType equivalentType =
-getDerived().TransformType(AuxiliaryTLB, TL.getEquivalentTypeLoc());
-if (equivalentType.isNull())
- return QualType();
+// Do not transform the equivalent type if it is equal to the modified
type.
+//
+// This is because, 1. it’s the same type, instantiating it again will
yield
+// the same result anyway, and if it doesn't, then that could be a bug in
+// and of itself, and 2. instantiating the same TypeLoc twice is a really
+// bad idea if it's a FunctionProtoType, because instantiating the same
+// ParmVarDecls twice will cause assertion failures.
+QualType equivalentType = modifiedType;
+if (TL.getModifiedLoc().getType() != TL.getEquivalentTypeLoc().getType()) {
+ TypeLocBuilder AuxiliaryTLB;
+ AuxiliaryTLB.reserve(TL.getFullDataSize());
+ equivalentType =
+ getDerived().TransformType(AuxiliaryTLB, TL.getEquivalentTypeLoc());
+ if (equivalentType.isNull())
+return QualType();
+}
// Check whether we can add nullability; it is only represented as
// type sugar, and therefore cannot be diagnosed in any other way.
>From a9c753ab46b40bd7da6f27eb11655fe43acb11de Mon Sep 17 00:00:00 2001
From: Sirraide
Date: Thu, 14 Mar 2024 21:40:41 +0100
Subject: [PATCH 3/8] [Clang] Refactor instantiation of a lambda's
FunctionProtoType
-
Co-authored-by: Yuxuan Chen
---
clang/include/clang/Sema/Template.h| 14 +++-
clang/lib/Sema/SemaTemplateInstantiate.cpp | 13 +++-
clang/lib/Sema/TreeTransform.h | 90 +++---
3 files changed, 49 insertions(+), 68 deletions(-)
diff --git a/clang/include/clang/Sema/Template.h
b/clang/include/clang/Sema/Template.h
index ce44aca797b0fb..8c379f51ca3d5d 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -411,6 +411,11 @@ enum class TemplateSubstitutionKind : char {
/// lookup will search our outer scope.
bool CombineWithOuterScope;
+/// Whether this scope is being used to instantiate a lambda expression,
+/// in which case it should be reused for instantiating the lambda's
+/// FunctionProtoType.
+bool InstantiatingLambda = false;
+
/// If non-NULL, the template parameter pack that has been
/// partially substituted per C++0x [temp.arg.explicit]p9.
NamedDecl *PartiallySubstitutedPack = nullptr;
@@ -425,9 +430,11 @@ enum class TemplateSubstitutionKind : char {
unsigned NumArgsInPartiallySubstitutedPack;
public:
-LocalInstantiationScope(Sema , bool CombineWithOuterScope = false)
+LocalInstantiationScope(Sema , bool
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
@@ -3140,13 +3140,35 @@ const FunctionType
*ASTContext::adjustFunctionType(const FunctionType *T,
return cast(Result.getTypePtr());
}
+QualType ASTContext::adjustFunctionResultType(QualType FunctionType,
+ QualType ResultType) {
+ // Might be wrapped in a macro qualified type.
+ if (const auto *MQT = dyn_cast(FunctionType)) {
+return getMacroQualifiedType(
+adjustFunctionResultType(MQT->getUnderlyingType(), ResultType),
+MQT->getMacroIdentifier());
+ }
+
+ // Might have a calling-convention attribute.
+ if (const auto *AT = dyn_cast(FunctionType)) {
+return getAttributedType(
+AT->getAttrKind(),
+adjustFunctionResultType(AT->getModifiedType(), ResultType),
+adjustFunctionResultType(AT->getEquivalentType(), ResultType));
+ }
+
+ // Anything else must be a function type. Rebuild it with the new return
+ // value.
Sirraide wrote:
There is at least one other case that we might want to handle
(`BTFTagAttributedType`), but I have so far been unsuccessful in trying to
actually create a declaration that has this attribute; I also have no idea what
that attribute is for.
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
@@ -13830,62 +13820,40 @@
TreeTransform::TransformLambdaExpr(LambdaExpr *E) {
getSema().AddTemplateParametersToLambdaCallOperator(NewCallOperator, Class,
TPL);
- // 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
- // transformed parameters.
TypeSourceInfo *NewCallOpTSI = nullptr;
Sirraide wrote:
Oh yeah, that makes sense; I didn’t even realise that somehow.
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
@@ -3140,13 +3140,35 @@ const FunctionType
*ASTContext::adjustFunctionType(const FunctionType *T,
return cast(Result.getTypePtr());
}
+QualType ASTContext::adjustFunctionResultType(QualType FunctionType,
+ QualType ResultType) {
+ // Might be wrapped in a macro qualified type.
+ if (const auto *MQT = dyn_cast(FunctionType)) {
+return getMacroQualifiedType(
+adjustFunctionResultType(MQT->getUnderlyingType(), ResultType),
+MQT->getMacroIdentifier());
+ }
+
+ // Might have a calling-convention attribute.
+ if (const auto *AT = dyn_cast(FunctionType)) {
+return getAttributedType(
+AT->getAttrKind(),
+adjustFunctionResultType(AT->getModifiedType(), ResultType),
+adjustFunctionResultType(AT->getEquivalentType(), ResultType));
+ }
+
+ // Anything else must be a function type. Rebuild it with the new return
+ // value.
yuxuanchen1997 wrote:
Is this assumption true? I added the two to my original PR because that's the
two I have seen but no confidence.
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
@@ -13830,62 +13820,40 @@
TreeTransform::TransformLambdaExpr(LambdaExpr *E) {
getSema().AddTemplateParametersToLambdaCallOperator(NewCallOperator, Class,
TPL);
- // 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
- // transformed parameters.
TypeSourceInfo *NewCallOpTSI = nullptr;
yuxuanchen1997 wrote:
Branches don't exist anymore, inline this decl to where it's assigned?
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
@@ -13830,62 +13820,40 @@
TreeTransform::TransformLambdaExpr(LambdaExpr *E) {
getSema().AddTemplateParametersToLambdaCallOperator(NewCallOperator, Class,
TPL);
- // 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
- // transformed parameters.
TypeSourceInfo *NewCallOpTSI = nullptr;
- {
-auto OldCallOpTypeLoc =
-E->getCallOperator()->getTypeSourceInfo()->getTypeLoc();
-
-auto TransformFunctionProtoTypeLoc =
-[this](TypeLocBuilder , FunctionProtoTypeLoc FPTL) -> QualType {
- SmallVector ExceptionStorage;
- return this->TransformFunctionProtoType(
- TLB, FPTL, nullptr, Qualifiers(),
- [&](FunctionProtoType::ExceptionSpecInfo , bool ) {
-return TransformExceptionSpec(FPTL.getBeginLoc(), ESI,
- ExceptionStorage, Changed);
- });
-};
-
-QualType NewCallOpType;
-TypeLocBuilder NewCallOpTLBuilder;
-
-if (auto ATL = OldCallOpTypeLoc.getAs()) {
- NewCallOpType = this->TransformAttributedType(
- NewCallOpTLBuilder, ATL,
- [&](TypeLocBuilder , TypeLoc TL) -> QualType {
-return TransformFunctionProtoTypeLoc(
-TLB, TL.castAs());
- });
-} else {
- auto FPTL = OldCallOpTypeLoc.castAs();
- NewCallOpType = TransformFunctionProtoTypeLoc(NewCallOpTLBuilder, FPTL);
-}
+ auto OldCallOpTypeLoc =
+ E->getCallOperator()->getTypeSourceInfo()->getTypeLoc();
-if (NewCallOpType.isNull())
- return ExprError();
-NewCallOpTSI =
-NewCallOpTLBuilder.getTypeSourceInfo(getSema().Context, NewCallOpType);
- }
+ QualType NewCallOpType;
yuxuanchen1997 wrote:
ditto.
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
yuxuanchen1997 wrote: Great. Thanks! https://github.com/llvm/llvm-project/pull/85325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
https://github.com/Sirraide edited https://github.com/llvm/llvm-project/pull/85325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
llvmbot wrote: @llvm/pr-subscribers-clang Author: None (Sirraide) Changes ## Summary This pr fixes a crash when we attempt to instantiate a lambda with an `AnnotatedType`, refactors the code that handles transforming the function type of a lambda, and improves source fidelity for lambda function types. The background for this is that, while working on #84983, the author observed that `AnnotatedType`s were not being propagated correctly when instantiating lambdas, which is a problem for anyone who actually wants to use this information for anything. ## Crash The cause of the crash was the following: When instantiating the `FunctionProtoTypeLoc` of a lambda, we were calling `TransformAttributedType`; however, an `AttributedType` stores two `QualType`s: a `ModifiedType` and an `EquivalentType`, both of which were being transformed in that function. The problem is that instantiating a `FunctionProtoTypeLoc` also instantiates the `ParmVarDecl`s associated with that type loc, which causes a link to be established between the original, dependent `ParmVarDecl`s and their instantiations in the current `LocalInstantiationScope`. If there already is a link (which was the case when instantiating the `EquivalentType` after instantiating the `ModifiedType`, if they were the same type), we assert. As a fix, we now only instantiate the `EquivalentType` iff it is not equal to the `ModifiedType`. Unfortunately, this doesn’t solve everything, as there is another problem here: the code that instantiates the function type of a lambda *did* account for the case of it being an `AttributedTypeLoc`, but it was also bending over backwards to avoid calling `getDerived().TransformFunctionProtoType` (which ends up being `TemplateInstantiator`’s override) and instead call `TreeTransform`’s `TransformFunctionProtoType`. The reason for this is that the former creates a new `LocalInstantiationScope`, whereas the latter does not—but when we begin instantiating a lambda (in `TemplateInstantiator::TransformLambdaExpr`), we already push a `LocalInstantiationScope` for the lambda, and having two scopes for the same lambda would cause things to go horribly wrong, which is why `TreeTransform`’s `TransformLambdaExpr` ends up being horribly messy because it has to somehow avoid calling into `TemplateInstantiator` (and `TransformAttributedType` likewise suffered from this since it *also* had to transform the `FunctionProtoType` in question). Instead of doing any of that, `TemplateInstantiator` now indicates that the first `LocalInstantiationScope` it creates is meant for instantiating a lambda (this is tracked in the `LocalInstantiationScope`), and its `TransformFunctionProtoType` only creates a new scope, iff the current scope is not a lambda. As a result, a lot of the tree transform code for this is now a lot simpler. Note: There already is a pr (#78801) that makes changes similar to the ones in this pr wrt refactoring tree transform only, but that pr hasn’t received any updates for over a month, and though the author has reached out to me saying that they’d be happy to help (which I definitely appreciate), I’ve still opted to instead add (some of) their changes to this pr instead—simply because this pr also does more than just address this issue, but trying to split the fixes up into separate prs doesn’t really work too well imo (for instance, there is no testing if `AttributedType`s are being propagated correctly if we crash trying to instantiate them), and I found it easier to just combine everything into one pr. I’ve added @yuxuanchen1997 as a co-author to the commit that deals with the tree transform refactor. I hope they’re fine with my doing so. ## Source Fidelity When rebuilding function types in `adjustDeducedFunctionResultType`, we now also rebuild `AttributedType`s and `MacroQualfiedType`s. This means that it should now be possible to access any type attributes attached to a lambda’s function type. This code was largely taken from #85147; I’ve also added @dougsonos as a co-author to the relevant commit. Side note: I know this doesn’t really belong in this pr, but I’ve also added a `dump()` function to `Attr` since it was lacking one, which was a bit annoying when I was debugging this earlier; I can open a separate pr for that if it’s too unrelated to the other changes here. This fixes #85120 and fixes #85154. --- Patch is 20.99 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/85325.diff 9 Files Affected: - (modified) clang/include/clang/AST/ASTContext.h (+5) - (modified) clang/include/clang/AST/Attr.h (+2) - (modified) clang/include/clang/Sema/Template.h (+12-2) - (modified) clang/lib/AST/ASTContext.cpp (+25-3) - (modified) clang/lib/AST/ASTDumper.cpp (+8) - (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+10-3) - (modified) clang/lib/Sema/TreeTransform.h (+38-70) - (added)
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
Sirraide wrote: I’ll add a release note once I’ve figured out how long this bug has been in Clang. https://github.com/llvm/llvm-project/pull/85325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
https://github.com/Sirraide created https://github.com/llvm/llvm-project/pull/85325 ## Summary This pr fixes a crash when we attempt to instantiate a lambda with an `AnnotatedType`, refactors the code that handles transforming the function type of a lambda, and improves source fidelity for lambda function types. The background for this is that, while working on #84983, the author observed that `AnnotatedType`s were not being propagated correctly when instantiating lambdas, which is a problem for anyone who actually wants to use this information for anything. ## Crash The cause of the crash was the following: When instantiating the `FunctionProtoTypeLoc` of a lambda, we were calling `TransformAttributedType`; however, an `AttributedType` stores two `QualType`s: a `ModifiedType` and an `EquivalentType`, both of which were being transformed in that function. The problem is that instantiating a `FunctionProtoTypeLoc` also instantiates the `ParmVarDecl`s associated with that type loc, which causes a link to be established between the original, dependent `ParmVarDecl`s and their instantiations in the current `LocalInstantiationScope`. If there already is a link (which was the case when instantiating the `EquivalentType` after instantiating the `ModifiedType`, if they were the same type), we assert. As a fix, we now only instantiate the `EquivalentType` iff it is not equal to the `ModifiedType`. Unfortunately, this doesn’t solve everything, as there is another problem here: the code that instantiates the function type of a lambda *did* account for the case of it being an `AttributedTypeLoc`, but it was also bending over backwards to avoid calling `getDerived().TransformFunctionProtoType` (which ends up being `TemplateInstantiator`’s override) and instead call `TreeTransform`’s `TransformFunctionProtoType`. The reason for this is that the former creates a new `LocalInstantiationScope`, whereas the latter does not—but when we begin instantiating a lambda (in `TemplateInstantiator::TransformLambdaExpr`), we already push a `LocalInstantiationScope` for the lambda, and having two scopes for the same lambda would cause things to go horribly wrong, which is why `TreeTransform`’s `TransformLambdaExpr` ends up being horribly messy because it has to somehow avoid calling into `TemplateInstantiator` (and `TransformAttributedType` likewise suffered from this since it *also* had to transform the `FunctionProtoType` in question). Instead of doing any of that, `TemplateInstantiator` now indicates that the first `LocalInstantiationScope` it creates is meant for instantiating a lambda (this is tracked in the `LocalInstantiationScope`), and its `TransformFunctionProtoType` only creates a new scope, iff the current scope is not a lambda. As a result, a lot of the tree transform code for this is now a lot simpler. Note: There already is a pr (#78801) that makes changes similar to the ones in this pr wrt refactoring tree transform only, but that pr hasn’t received any updates for over a month, and though the author has reached out to me saying that they’d be happy to help (which I definitely appreciate), I’ve still opted to instead add (some of) their changes to this pr instead—simply because this pr also does more than just address this issue, but trying to split the fixes up into separate prs doesn’t really work too well imo (for instance, there is no testing if `AttributedType`s are being propagated correctly if we crash trying to instantiate them), and I found it easier to just combine everything into one pr. I’ve added @yuxuanchen1997 as a co-author to the commit that deals with the tree transform refactor. I hope they’re fine with my doing so. ## Source Fidelity When rebuilding function types in `adjustDeducedFunctionResultType`, we now also rebuild `AttributedType`s and `MacroQualfiedType`s. This means that it should now be possible to access any type attributes attached to a lambda’s function type. This code was largely taken from #85147; I’ve also added @dougsonos as a co-author to the relevant commit. Side note: I know this doesn’t really belong in this pr, but I’ve also added a `dump()` function to `Attr` since it was lacking one, which was a bit annoying when I was debugging this earlier; I can open a separate pr for that if it’s too unrelated to the other changes here. This fixes #85120 and fixes #85154. >From 907210a3ad3d829a8e49a5c976d129f8653801bf Mon Sep 17 00:00:00 2001 From: Sirraide Date: Thu, 14 Mar 2024 18:24:37 +0100 Subject: [PATCH 1/7] [Clang] Add `dump()` method for `Attr` --- clang/include/clang/AST/Attr.h | 2 ++ clang/lib/AST/ASTDumper.cpp| 8 2 files changed, 10 insertions(+) diff --git a/clang/include/clang/AST/Attr.h b/clang/include/clang/AST/Attr.h index 8e9b7ad8b46826..6400023947863f 100644 --- a/clang/include/clang/AST/Attr.h +++ b/clang/include/clang/AST/Attr.h @@ -112,6 +112,8 @@ class Attr : public
