[BoundsSafety] Support late parsing for counted_by in type positions
#166491
+383
−366
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
rapidsna
requested review from
AaronBallman,
apple-fcloutier,
bwendling,
delcypher,
hnrklssn,
kees and
patrykstefanski
llvmbot
added
clang
rapidsna
added
clang:bounds-safety
Member
|
@llvm/pr-subscribers-clang Author: Yeoul Na (rapidsna) ChangesPreviously, late parsing only supported attributes in declaration position and could not resolve references to member declarations that appeared later in the struct. Additionally, the compiler incorrectly accepted This commit adds a vector of Issue #166411 Patch is 77.84 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/166491.diff 23 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index fa509536bf021..98b470a525d11 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7012,6 +7012,9 @@ def err_builtin_counted_by_ref_invalid_use : Error<
"value returned by '__builtin_counted_by_ref' cannot be used in "
"%select{an array subscript|a binary}0 expression">;
+def err_counted_by_on_nested_pointer : Error<
+ "'%select{counted_by|sized_by|counted_by_or_null|sized_by_or_null}0' attribute on nested pointer type is not allowed">;
+
let CategoryName = "ARC Semantic Issue" in {
// ARC-mode diagnostics.
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index dad8efd0f017f..8f7c921fb2b1d 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1161,10 +1161,11 @@ class Parser : public CodeCompletionHandler {
IdentifierInfo *MacroII = nullptr;
SourceLocation AttrNameLoc;
SmallVector<Decl *, 2> Decls;
+ unsigned NestedTypeLevel;
explicit LateParsedAttribute(Parser *P, IdentifierInfo &Name,
- SourceLocation Loc)
- : Self(P), AttrName(Name), AttrNameLoc(Loc) {}
+ SourceLocation Loc, unsigned Level = 0)
+ : Self(P), AttrName(Name), AttrNameLoc(Loc), NestedTypeLevel(Level) {}
void ParseLexedAttributes() override;
@@ -1889,10 +1890,12 @@ class Parser : public CodeCompletionHandler {
DeclSpec &DS, AccessSpecifier AS, DeclSpecContext DSContext,
LateParsedAttrList *LateAttrs = nullptr);
- void ParseSpecifierQualifierList(
- DeclSpec &DS, AccessSpecifier AS = AS_none,
- DeclSpecContext DSC = DeclSpecContext::DSC_normal) {
- ParseSpecifierQualifierList(DS, getImplicitTypenameContext(DSC), AS, DSC);
+ void
+ ParseSpecifierQualifierList(DeclSpec &DS, AccessSpecifier AS = AS_none,
+ DeclSpecContext DSC = DeclSpecContext::DSC_normal,
+ LateParsedAttrList *LateAttrs = nullptr) {
+ ParseSpecifierQualifierList(DS, getImplicitTypenameContext(DSC), AS, DSC,
+ LateAttrs);
}
/// ParseSpecifierQualifierList
@@ -1903,10 +1906,12 @@ class Parser : public CodeCompletionHandler {
/// [GNU] attributes specifier-qualifier-list[opt]
/// \endverbatim
///
- void ParseSpecifierQualifierList(
- DeclSpec &DS, ImplicitTypenameContext AllowImplicitTypename,
- AccessSpecifier AS = AS_none,
- DeclSpecContext DSC = DeclSpecContext::DSC_normal);
+ void
+ ParseSpecifierQualifierList(DeclSpec &DS,
+ ImplicitTypenameContext AllowImplicitTypename,
+ AccessSpecifier AS = AS_none,
+ DeclSpecContext DSC = DeclSpecContext::DSC_normal,
+ LateParsedAttrList *LateAttrs = nullptr);
/// ParseEnumSpecifier
/// \verbatim
@@ -2444,7 +2449,8 @@ class Parser : public CodeCompletionHandler {
SourceLocation ScopeLoc,
ParsedAttr::Form Form);
- void DistributeCLateParsedAttrs(Decl *Dcl, LateParsedAttrList *LateAttrs);
+ void DistributeCLateParsedAttrs(Declarator &D, Decl *Dcl,
+ LateParsedAttrList *LateAttrs);
/// Bounds attributes (e.g., counted_by):
/// \verbatim
@@ -2610,7 +2616,8 @@ class Parser : public CodeCompletionHandler {
void ParseTypeQualifierListOpt(
DeclSpec &DS, unsigned AttrReqs = AR_AllAttributesParsed,
bool AtomicOrPtrauthAllowed = true, bool IdentifierRequired = false,
- llvm::function_ref<void()> CodeCompletionHandler = {});
+ llvm::function_ref<void()> CodeCompletionHandler = {},
+ LateParsedAttrList *LateAttrs = nullptr);
/// ParseDirectDeclarator
/// \verbatim
diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h
index 43a48c92fc305..9f633dd71c3f6 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -1238,6 +1238,9 @@ struct DeclaratorChunk {
ParsedAttributesView AttrList;
+ using LateAttrListTy = SmallVector<void *, 1>;
+ LateAttrListTy LateAttrList;
+
struct PointerTypeInfo {
/// The type qualifiers: const/volatile/restrict/unaligned/atomic.
LLVM_PREFERRED_TYPE(DeclSpec::TQ)
@@ -2325,13 +2328,16 @@ class Declarator {
/// This function takes attrs by R-Value reference because it takes ownership
/// of those attributes from the parameter.
void AddTypeInfo(const DeclaratorChunk &TI, ParsedAttributes &&attrs,
- SourceLocation EndLoc) {
+ SourceLocation EndLoc,
+ const DeclaratorChunk::LateAttrListTy &LateAttrs = {}) {
DeclTypeInfo.push_back(TI);
DeclTypeInfo.back().getAttrs().prepend(attrs.begin(), attrs.end());
getAttributePool().takeAllFrom(attrs.getPool());
if (!EndLoc.isInvalid())
SetRangeEnd(EndLoc);
+
+ DeclTypeInfo.back().LateAttrList.assign(LateAttrs);
}
/// AddTypeInfo - Add a chunk to this declarator. Also extend the range to
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index c67ed99b1f49e..becadbb1a39d6 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2457,8 +2457,8 @@ class Sema final : public SemaBase {
/// `counted_by_or_null` attribute.
///
/// \returns false iff semantically valid.
- bool CheckCountedByAttrOnField(FieldDecl *FD, Expr *E, bool CountInBytes,
- bool OrNull);
+ bool CheckCountedByAttrOnField(FieldDecl *FD, Expr *E, unsigned Level,
+ bool CountInBytes, bool OrNull);
/// Perform Bounds Safety Semantic checks for assigning to a `__counted_by` or
/// `__counted_by_or_null` pointer type \param LHSTy.
@@ -4198,7 +4198,8 @@ class Sema final : public SemaBase {
/// ActOnFinishDelayedAttribute - Invoked when we have finished parsing an
/// attribute for which parsing is delayed.
- void ActOnFinishDelayedAttribute(Scope *S, Decl *D, ParsedAttributes &Attrs);
+ void ActOnFinishDelayedAttribute(Scope *S, Decl *D, ParsedAttributes &Attrs,
+ unsigned NestedTypeLevel = 0);
/// Diagnose any unused parameters in the given sequence of
/// ParmVarDecl pointers.
@@ -5071,7 +5072,8 @@ class Sema final : public SemaBase {
void ProcessDeclAttributeList(Scope *S, Decl *D,
const ParsedAttributesView &AttrList,
const ProcessDeclAttributeOptions &Options =
- ProcessDeclAttributeOptions());
+ ProcessDeclAttributeOptions(),
+ unsigned NestedTypeLevel = 0);
/// Annotation attributes are the only attributes allowed after an access
/// specifier.
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 7e4a164e34eda..514f6daf44e2f 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -1943,7 +1943,11 @@ Parser::DeclGroupPtrTy Parser::ParseSimpleDeclaration(
ParsedTemplateInfo TemplateInfo;
DeclSpecContext DSContext = getDeclSpecContextFromDeclaratorContext(Context);
- ParseDeclarationSpecifiers(DS, TemplateInfo, AS_none, DSContext);
+ // FIXME: Why is PSoon true?
+ LateParsedAttrList BoundsSafetyLateAttrs(
+ /*PSoon=*/true, /*LateAttrParseExperimentalExtOnly=*/true);
+ ParseDeclarationSpecifiers(DS, TemplateInfo, AS_none, DSContext,
+ &BoundsSafetyLateAttrs);
// If we had a free-standing type definition with a missing semicolon, we
// may get this far before the problem becomes obvious.
@@ -2725,12 +2729,12 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
void Parser::ParseSpecifierQualifierList(
DeclSpec &DS, ImplicitTypenameContext AllowImplicitTypename,
- AccessSpecifier AS, DeclSpecContext DSC) {
+ AccessSpecifier AS, DeclSpecContext DSC, LateParsedAttrList *LateAttrs) {
ParsedTemplateInfo TemplateInfo;
/// specifier-qualifier-list is a subset of declaration-specifiers. Just
/// parse declaration-specifiers and complain about extra stuff.
/// TODO: diagnose attribute-specifiers and alignment-specifiers.
- ParseDeclarationSpecifiers(DS, TemplateInfo, AS, DSC, nullptr,
+ ParseDeclarationSpecifiers(DS, TemplateInfo, AS, DSC, LateAttrs,
AllowImplicitTypename);
// Validate declspec for type-name.
@@ -3136,15 +3140,37 @@ void Parser::ParseAlignmentSpecifier(ParsedAttributes &Attrs,
}
}
-void Parser::DistributeCLateParsedAttrs(Decl *Dcl,
+void Parser::DistributeCLateParsedAttrs(Declarator &D, Decl *Dcl,
LateParsedAttrList *LateAttrs) {
if (!LateAttrs)
return;
+ unsigned NestedLevel = 0;
+ for (unsigned i = 0; i < D.getNumTypeObjects(); ++i) {
+ DeclaratorChunk &DC = D.getTypeObject(i);
+
+ switch (DC.Kind) {
+ case DeclaratorChunk::Pointer:
+ case DeclaratorChunk::Array:
+ break;
+ default:
+ continue;
+ }
+
+ // Extract `LateParsedAttribute *` from `DeclaratorChunk`.
+ for (auto *OpaqueLA : DC.LateAttrList) {
+ auto *LA = static_cast<LateParsedAttribute *>(OpaqueLA);
+ LA->NestedTypeLevel = NestedLevel;
+ LateAttrs->push_back(LA);
+ }
+ NestedLevel++;
+ }
+
+ // Attach `Decl *` to each `LateParsedAttribute *`.
if (Dcl) {
- for (auto *LateAttr : *LateAttrs) {
- if (LateAttr->Decls.empty())
- LateAttr->addDecl(Dcl);
+ for (auto *LA : *LateAttrs) {
+ if (LA->Decls.empty())
+ LA->addDecl(Dcl);
}
}
}
@@ -3217,12 +3243,6 @@ void Parser::ParseBoundsAttribute(IdentifierInfo &AttrName,
ArgExprs.push_back(ArgExpr.get());
Parens.consumeClose();
- ASTContext &Ctx = Actions.getASTContext();
-
- ArgExprs.push_back(IntegerLiteral::Create(
- Ctx, llvm::APInt(Ctx.getTypeSize(Ctx.getSizeType()), 0),
- Ctx.getSizeType(), SourceLocation()));
-
Attrs.addNew(&AttrName, SourceRange(AttrNameLoc, Parens.getCloseLocation()),
AttributeScopeInfo(), ArgExprs.data(), ArgExprs.size(), Form);
}
@@ -4706,7 +4726,8 @@ void Parser::ParseStructDeclaration(
MaybeParseCXX11Attributes(Attrs);
// Parse the common specifier-qualifiers-list piece.
- ParseSpecifierQualifierList(DS);
+ ParseSpecifierQualifierList(DS, AS_none, DeclSpecContext::DSC_normal,
+ LateFieldAttrs);
// If there are no declarators, this is a free-standing declaration
// specifier. Let the actions module cope with it.
@@ -4768,7 +4789,7 @@ void Parser::ParseStructDeclaration(
// We're done with this declarator; invoke the callback.
Decl *Field = FieldsCallback(DeclaratorInfo);
if (Field)
- DistributeCLateParsedAttrs(Field, LateFieldAttrs);
+ DistributeCLateParsedAttrs(DeclaratorInfo.D, Field, LateFieldAttrs);
// If we don't have a comma, it is either the end of the list (a ';')
// or an error, bail out.
@@ -4825,7 +4846,8 @@ void Parser::ParseLexedCAttribute(LateParsedAttribute &LA, bool EnterScope,
SourceLocation(), ParsedAttr::Form::GNU(), nullptr);
for (auto *D : LA.Decls)
- Actions.ActOnFinishDelayedAttribute(getCurScope(), D, Attrs);
+ Actions.ActOnFinishDelayedAttribute(getCurScope(), D, Attrs,
+ LA.NestedTypeLevel);
// Due to a parsing error, we either went over the cached tokens or
// there are still cached tokens left, so we skip the leftover tokens.
@@ -6124,7 +6146,8 @@ bool Parser::isConstructorDeclarator(bool IsUnqualified, bool DeductionGuide,
void Parser::ParseTypeQualifierListOpt(
DeclSpec &DS, unsigned AttrReqs, bool AtomicOrPtrauthAllowed,
- bool IdentifierRequired, llvm::function_ref<void()> CodeCompletionHandler) {
+ bool IdentifierRequired, llvm::function_ref<void()> CodeCompletionHandler,
+ LateParsedAttrList *LateAttrs) {
if ((AttrReqs & AR_CXX11AttributesParsed) &&
isAllowedCXX11AttributeSpecifier()) {
ParsedAttributes Attrs(AttrFactory);
@@ -6266,7 +6289,9 @@ void Parser::ParseTypeQualifierListOpt(
// recovery is graceful.
if (AttrReqs & AR_GNUAttributesParsed ||
AttrReqs & AR_GNUAttributesParsedAndRejected) {
- ParseGNUAttributes(DS.getAttributes());
+
+ assert(!LateAttrs || LateAttrs->lateAttrParseExperimentalExtOnly());
+ ParseGNUAttributes(DS.getAttributes(), LateAttrs);
continue; // do *not* consume the next token!
}
// otherwise, FALL THROUGH!
@@ -6447,21 +6472,33 @@ void Parser::ParseDeclaratorInternal(Declarator &D,
((D.getContext() != DeclaratorContext::CXXNew)
? AR_GNUAttributesParsed
: AR_GNUAttributesParsedAndRejected);
+ LateParsedAttrList LateAttrs(/*PSoon=*/true,
+ /*LateAttrParseExperimentalExtOnly=*/true);
ParseTypeQualifierListOpt(DS, Reqs, /*AtomicOrPtrauthAllowed=*/true,
- !D.mayOmitIdentifier());
+ !D.mayOmitIdentifier(), {}, &LateAttrs);
D.ExtendWithDeclSpec(DS);
// Recursively parse the declarator.
Actions.runWithSufficientStackSpace(
D.getBeginLoc(), [&] { ParseDeclaratorInternal(D, DirectDeclParser); });
- if (Kind == tok::star)
+ if (Kind == tok::star) {
+ DeclaratorChunk::LateAttrListTy OpaqueLateAttrList;
+ if (getLangOpts().ExperimentalLateParseAttributes && !LateAttrs.empty()) {
+ if (!D.isFunctionDeclarator()) {
+ for (auto LA : LateAttrs) {
+ OpaqueLateAttrList.push_back(LA);
+ }
+ }
+ LateAttrs.clear();
+ }
// Remember that we parsed a pointer type, and remember the type-quals.
D.AddTypeInfo(DeclaratorChunk::getPointer(
DS.getTypeQualifiers(), Loc, DS.getConstSpecLoc(),
DS.getVolatileSpecLoc(), DS.getRestrictSpecLoc(),
DS.getAtomicSpecLoc(), DS.getUnalignedSpecLoc()),
- std::move(DS.getAttributes()), SourceLocation());
- else
+ std::move(DS.getAttributes()), SourceLocation(),
+ OpaqueLateAttrList);
+ } else
// Remember that we parsed a Block type, and remember the type-quals.
D.AddTypeInfo(
DeclaratorChunk::getBlockPointer(DS.getTypeQualifiers(), Loc),
diff --git a/clang/lib/Sema/SemaBoundsSafety.cpp b/clang/lib/Sema/SemaBoundsSafety.cpp
index 39ab13653f5fe..8411eec7dfb3f 100644
--- a/clang/lib/Sema/SemaBoundsSafety.cpp
+++ b/clang/lib/Sema/SemaBoundsSafety.cpp
@@ -50,8 +50,8 @@ enum class CountedByInvalidPointeeTypeKind {
VALID,
};
-bool Sema::CheckCountedByAttrOnField(FieldDecl *FD, Expr *E, bool CountInBytes,
- bool OrNull) {
+bool Sema::CheckCountedByAttrOnField(FieldDecl *FD, Expr *E, unsigned Level,
+ bool CountInBytes, bool OrNull) {
// Check the context the attribute is used in
unsigned Kind = getCountAttrKind(CountInBytes, OrNull);
@@ -62,6 +62,12 @@ bool Sema::CheckCountedByAttrOnField(FieldDecl *FD, Expr *E, bool CountInBytes,
return true;
}
+ if (Level != 0) {
+ Diag(FD->getBeginLoc(), diag::err_counted_by_on_nested_pointer)
+ << Kind << FD->getSourceRange();
+ return true;
+ }
+
const auto FieldTy = FD->getType();
if (FieldTy->isArrayType() && (CountInBytes || OrNull)) {
Diag(FD->getBeginLoc(),
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index fc3aabf5741ca..85e75d4e4b91a 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -16873,11 +16873,13 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body, bool IsInstantiation,
/// When we finish delayed parsing of an attribute, we must attach it to the
/// relevant Decl.
void Sema::ActOnFinishDelayedAttribute(Scope *S, Decl *D,
- ParsedAttributes &Attrs) {
+ ParsedAttributes &Attrs,
+ unsigned NestedTypeLevel) {
// Always attach attributes to the underlying decl.
if (TemplateDecl *TD = dyn_cast<TemplateDecl>(D))
D = TD->getTemplatedDecl();
- ProcessDeclAttributeList(S, D, Attrs);
+ ProcessDeclAttributeList(S, D, Attrs, ProcessDeclAttributeOptions(),
+ NestedTypeLevel);
ProcessAPINotes(D);
if (CXXMethodDecl *Method = dyn_cast_or_null<CXXMethodDecl>(D))
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index a9e7b44ac9d73..2369180713ffb 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -6546,7 +6546,8 @@ static void handleZeroCallUsedRegsAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
D->addAttr(ZeroCallUsedRegsAttr::Create(S.Context, Kind, AL));
}
-static void handleCountedByAttrField(Sema &S, Decl *D, const ParsedAttr &AL) {
+static void handleCountedByAttrField(Sema &S, Decl *D, const ParsedAttr &AL,
+ unsigned Level) {
auto *FD = dyn_cast<FieldDecl>(D);
assert(FD);
@@ -6577,7 +6578,7 @@ static void handleCountedByAttrField(Sema &S, Decl *D, const ParsedAttr &AL) {
llvm_unreachable("unexpected counted_by family attribute");
}
- if (S.CheckCountedByAttrOnField(FD, CountExpr, CountInBytes, OrNull))
+ if (S.CheckCountedByAttrOnField(FD, CountExpr, Level, CountInBytes, OrNull))
return;
QualType CAT = S.BuildCountAttributedArrayOrPointerType(
@@ -6951,7 +6952,8 @@ static bool MustDelayAttributeArguments(const ParsedAttr &AL) {
/// silently ignore it if a GNU attribute.
static void
ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
- const Sema::ProcessDeclAttributeOptions &Options) {
+ const Sema::ProcessDeclAttributeOptions &Options,
+ unsigned NestedTypeLevel = 0) {
if (AL.isInvalid() || AL.getKind() == ParsedAttr::IgnoredAttribute)
return;
@@ -7578,7 +7580,7 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
case ParsedAttr::AT_CountedByOrNull:
case ParsedAttr::AT_SizedBy:
case ParsedAttr::AT_SizedByOrNull:
- handleCountedByAttrField(S, D, AL);
+ handleCountedByAttrField(S, D, AL, NestedTypeLevel);
break;
// Microsoft attributes:
@@ -7855,14 +7857,15 @@ static bool isKernelDecl(Decl *D) {
D->hasAttr<CUDAGlobalAttr>();
}
-void Sema::ProcessDeclAttributeList(
- Scope *S, Decl *D, const ParsedAttributesView &AttrList,
- const ProcessDeclAttributeOptions &Options) {
+void Sema::ProcessDeclAttributeList(Scope *S, Decl *D,
+ const ParsedAttributesView &AttrList,
+ const ProcessDeclAttributeOptions &Options,
+ unsigned NestedTypeLevel) {
if (AttrList.empty())
return;
for (const ParsedAttr &AL : AttrList)
- ProcessDeclAttribute(*this, S, D, AL, Options);
+ ProcessDeclAttribute(*this, S, D, AL, Options, NestedTypeLevel);
// FIXME: We should be able to handle these cases in TableGen.
// GCC accepts
@@ -8171,11 +8174,22 @@ void Sema::ProcessDeclAttributes(Scope *S, Decl *D, const Declarator &PD) {
// position to the decl itself. This handles cases like:
// int *__attr__(x)** D;
// when X is a decl attribute.
+ unsigned NestedTypeLevel = 0;
for (unsigned i = 0, e = PD.getNumTypeObjects(); i != e; ++i) {
ProcessDeclAttributeList(S, D, PD.getTypeObject(i).getAttrs(),
ProcessDeclAttributeOptions()
.WithIncludeCXX11Attributes(false)
- .WithIgnoreTypeAttributes(true));
+ .WithIgnoreTypeAttributes(true),
+ NestedTypeLevel);
+
+ switch (PD.getTypeObject(i).Kind) {
+ case DeclaratorChunk::Pointer:
+ case ...
[truncated]
|
hnrklssn
reviewed
Nov 7, 2025
hnrklssn
reviewed
Nov 7, 2025
hnrklssn
reviewed
Nov 7, 2025
hnrklssn
reviewed
Nov 7, 2025
hnrklssn
reviewed
Nov 7, 2025
zmodem
reviewed
Nov 7, 2025
Collaborator
zmodem
left a comment
zmodem
left a comment
There was a problem hiding this comment.
I like all the deleted todos :-)
clang/include/clang/Sema/DeclSpec.h
Outdated
|
|
||
| ParsedAttributesView AttrList; | ||
|
|
||
| using LateAttrListTy = SmallVector<void *, 1>; |
Collaborator
There was a problem hiding this comment.
void* seems unusual. Don't we have a better type to use?
Contributor
Author
There was a problem hiding this comment.
This tracks pointers to Parser::LateParsedAttribute, a member struct of class Parser defined in Parser.h.
I couldn't include Parser.h because of circular dependencies. I also looked at a few other options:
- Duplicating the type in DeclSpec.h — In downstream, we created another struct called
LateParsedAttrInfoinDeclSpec.h, but it's essentially a duplicate ofParser::LateParsedAttribute, so I don't think that's the right approach. - Forward declaring
Parser::LateParsedAttribute— Can't do it since it's a nested member struct. - Hoisting
LateParsedAttributeout of class Parser — It inherits fromParser::LateParsedDeclaration, which is inherited by multiple other member structs and referenced all over the codebase. That would require too invasive changes.
I can still create a type alias to make it more readable:
using LateAttrOpaquePtr = void*;
Would that work for you?
Collaborator
There was a problem hiding this comment.
I see. Forward declaring it would have been nice, but that sounds very tricky. Maybe just adding a comment explaining what void* refers to is good enough?
Contributor
Author
There was a problem hiding this comment.
Sounds good. I just added comment and a type alias which was needed for ArrayRef anyway!
llvmbot
added
clang
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Contributor
Author
|
Some test failures are due to a recently merged PR to allow |
rapidsna
force-pushed
the
dev/yeoulna/late-parsing-for-type-attrs
branch
2 times, most recently
from
8ef023e to
8673fd7
Compare
Previously, late parsing only supported attributes in declaration position and could not resolve references to member declarations that appeared later in the struct. Additionally, the compiler incorrectly accepted `counted_by` on nested pointer types, which should be rejected. Instead, these attributes were incorrectly attached to the outermost pointer type because they were being treated as GNU-style declaration attributes rather than type attributes. This commit adds a vector of `LateParsedAttribute *` to `DeclaratorChunk`, similar to how `ParsedAttr` is attached to `DeclaratorChunk`. Since DeclSpec doesn't see the definition of `LateParsedAttribute`, it is treated as an opaque pointer type. When the late-attribute is actually parsed, it is now correctly attached to the appropriate nested type level, enabling references to members declared later when the late-parsed attribute is in type position, and rejection of invalid nested pointer cases. Issue llvm#166411
rapidsna
force-pushed
the
dev/yeoulna/late-parsing-for-type-attrs
branch
from
8673fd7 to
7ce24ab
Compare
erichkeane
reviewed
Nov 12, 2025
Collaborator
erichkeane
left a comment
erichkeane
left a comment
There was a problem hiding this comment.
I'm out of my depth reviewing this, I don't know much about late parsed attributes. Hopefully @AaronBallman can come along and take a look.
| IdentifierInfo *MacroII = nullptr; | ||
| SourceLocation AttrNameLoc; | ||
| SmallVector<Decl *, 2> Decls; | ||
| unsigned NestedTypeLevel; |
Collaborator
There was a problem hiding this comment.
Can you explain what this means? This is awkwardly worded and isn't clear at all what it should represent.
| /// Stores pointers to `Parser::LateParsedAttribute`. We use `void*` here | ||
| /// because `LateParsedAttribute` is a nested struct of `class Parser` and | ||
| /// cannot be forward-declared. | ||
| using LateAttrOpaquePtr = void *; |
Collaborator
There was a problem hiding this comment.
THIS feels like a code smell, and I'm pretty uncomfortable by what is happening here.
I'm hopeful that @AaronBallman can come along and take a look at this.
Contributor
Author
There was a problem hiding this comment.
We discussed other alternatives here: #166491 (comment)
I couldn't include Parser.h because of circular dependencies. I also looked at a few other options:
- Duplicating the type in
DeclSpec.h— In downstream, we created another struct calledLateParsedAttrInfoin DeclSpec.h, but it's essentially a duplicate ofParser::LateParsedAttribute, so I don't think that's the right approach. - Forward declaring
Parser::LateParsedAttribute— Can't do it since it's a nested member struct. - Hoisting
LateParsedAttributeout ofclass Parser— It inherits fromParser::LateParsedDeclaration, which is inherited by multiple other member structs and referenced all over the codebase. That would require too invasive changes.
So using an opaque type seemed like a cleanest solution, but I'm open to other suggestions.
| /// | ||
| /// \param FD The FieldDecl to apply the attribute to | ||
| /// \param E The count expression on the attribute | ||
| /// \param NestedTypeLevel The pointer indirection level where the attribute |
Collaborator
There was a problem hiding this comment.
Hmm... why does this have to be a certain number deep, and not just calculated/stored on that type?
Contributor
Author
There was a problem hiding this comment.
Because the attribute is late parsed after the struct is fully declared, so the field's type is already constructed. We could use placeholder types and backfill later, but then we'd have to walk through all the type indirections. Or record the type positions that need the backfill anyway. Passing the level directly seemed simpler and avoids that overhead.
| ParsedTemplateInfo TemplateInfo; | ||
| DeclSpecContext DSContext = getDeclSpecContextFromDeclaratorContext(Context); | ||
| ParseDeclarationSpecifiers(DS, TemplateInfo, AS_none, DSContext); | ||
| // FIXME: Why is PSoon true? |
Collaborator
There was a problem hiding this comment.
Thats a wonderful question :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Previously, late parsing only supported attributes in declaration position and could not resolve references to member declarations that appeared later in the struct. Additionally, the compiler incorrectly accepted
counted_byon nested pointer types, which should be rejected. Instead, these attributes were incorrectly attached to the outermost pointer type because they were being treated as GNU-style declaration attributes rather than type attributes.This commit adds a vector of
LateParsedAttribute *toDeclaratorChunk, similar to howParsedAttris attached toDeclaratorChunk. Since DeclSpec doesn't see the definition ofLateParsedAttribute, it is treated as an opaque pointer type. When the late-attribute is actually parsed, it is now correctly attached to the appropriate nested type level, enabling references to members declared later when the late-parsed attribute is in type position, and rejection of invalid nested pointer cases.Issue #166411