llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Yeoul Na (rapidsna) <details> <summary>Changes</summary> Two commits: **1. Handle the counted_by family as a type attribute** counted_by / sized_by (and their _or_null variants) were only handled as a declaration attribute: `handleCountedByAttrField` validated the attribute and then patched the field afterwards with `FieldDecl::setType`. Add `HandleCountedByAttrOnType`, dispatch the family from `processTypeAttrs`, and remove the decl-position handler, so the type is built during type construction from a single handler serving both positions. The FieldDecl-based type-shape checks are superseded by `Sema::ValidateBoundsAttrTypeShape` and deleted; no diagnostic is dropped. Test expectations are updated in the second commit, so this commit alone leaves them stale. **2. Create incomplete counted_by types and wire up the refill** Activate late parsing under `-fexperimental-late-parse-attributes`. When the attribute is seen during type construction and its argument cannot be resolved yet, build the `CountAttributedType` immediately with `getIncompleteCountAttributedType` and record it against the enclosing record; the count expression is filled in at the closing brace. Because enclosing types refer to the node by pointer, completing it in place leaves the type chain untouched -- no rebuild and no TypeLoc re-emission. Depends on #<!-- -->224552 --- Patch is 98.91 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/224554.diff 23 Files Affected: - (modified) clang/include/clang/AST/TypeBase.h (+1-1) - (modified) clang/include/clang/Parse/Parser.h (+11) - (modified) clang/include/clang/Sema/Sema.h (+23-16) - (modified) clang/lib/Parse/ParseDecl.cpp (+178-25) - (modified) clang/lib/Sema/SemaBoundsSafety.cpp (+3-109) - (modified) clang/lib/Sema/SemaDecl.cpp (+24-1) - (modified) clang/lib/Sema/SemaDeclAttr.cpp (-44) - (modified) clang/lib/Sema/SemaType.cpp (+131-1) - (modified) clang/test/AST/attr-counted-by-or-null-struct-ptrs.c (-23) - (modified) clang/test/AST/attr-counted-by-struct-ptrs.c (-26) - (modified) clang/test/AST/attr-sized-by-or-null-struct-ptrs.c (-24) - (modified) clang/test/AST/attr-sized-by-struct-ptrs.c (-24) - (modified) clang/test/Sema/attr-counted-by-late-parsed-struct-ptrs.c (+29-50) - (modified) clang/test/Sema/attr-counted-by-or-null-last-field.c (+1-3) - (modified) clang/test/Sema/attr-counted-by-or-null-late-parsed-struct-ptrs.c (+14-50) - (modified) clang/test/Sema/attr-counted-by-or-null-struct-ptrs-completable-incomplete-pointee.c (+17-20) - (modified) clang/test/Sema/attr-counted-by-or-null-struct-ptrs.c (+8-6) - (modified) clang/test/Sema/attr-counted-by-struct-ptrs-completable-incomplete-pointee.c (+15-17) - (modified) clang/test/Sema/attr-counted-by-struct-ptrs.c (+10-8) - (modified) clang/test/Sema/attr-sized-by-late-parsed-struct-ptrs.c (+10-42) - (modified) clang/test/Sema/attr-sized-by-or-null-late-parsed-struct-ptrs.c (+11-42) - (modified) clang/test/Sema/attr-sized-by-or-null-struct-ptrs.c (+8-6) - (modified) clang/test/Sema/attr-sized-by-struct-ptrs.c (+8-4) ``````````diff diff --git a/clang/include/clang/AST/TypeBase.h b/clang/include/clang/AST/TypeBase.h index 28f102fdaf534..454829ff58b43 100644 --- a/clang/include/clang/AST/TypeBase.h +++ b/clang/include/clang/AST/TypeBase.h @@ -3451,7 +3451,7 @@ class BoundsAttributedType : public Type, public llvm::FoldingSetNode { QualType WrappedTy; protected: - ArrayRef<TypeCoupledDeclRefInfo> Decls; // stored in trailing objects + ArrayRef<TypeCoupledDeclRefInfo> Decls; // allocated in the ASTContext BoundsAttributedType(TypeClass TC, QualType Wrapped, QualType Canon); diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index 6d0affa3c8822..af0f9040185f2 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -292,6 +292,8 @@ class Parser : public CodeCompletionHandler { friend class PoisonSEHIdentifiersRAIIObject; friend class ParenBraceBracketBalancer; friend class BalancedDelimiterTracker; + friend struct LateParsedAttribute; + friend struct LateParsedTypeAttribute; Parser(Preprocessor &PP, Sema &Actions, bool SkipFunctionBodies); ~Parser() override; @@ -2246,6 +2248,8 @@ class Parser : public CodeCompletionHandler { ParsedAttributes Attrs(AttrFactory); ParseGNUAttributes(Attrs, LateAttrs, &D); D.takeAttributesAppending(Attrs); + if (LateAttrs) + Parser::TakeTypeAttrsAppendingFrom(D.getLateAttributes(), *LateAttrs); } } @@ -8172,6 +8176,13 @@ class Parser : public CodeCompletionHandler { QualType &type, unsigned pointerNestLevel); + /// The late-parsed type attributes of the record currently being parsed, so a + /// nested anonymous record can hand its unresolved attributes to the enclosing + /// record whose scope makes their arguments visible. Null outside a record + /// body. + SmallVectorImpl<LateParsedTypeAttribute *> *CurRecordLateParsedTypeAttrs = + nullptr; + /// We've parsed something that could plausibly be intended to be a template /// name (\p LHS) followed by a '<' token, and the following code can't /// possibly be an expression. Determine if this is likely to be a template-id diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index f643f1cb328d8..952a15d029569 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -142,6 +142,8 @@ class InitializationKind; class InitializationSequence; class InitializedEntity; enum class LangAS : unsigned int; +struct LateParsedAttribute; +struct LateParsedTypeAttribute; class LocalInstantiationScope; class LookupResult; class MangleNumberingContext; @@ -2538,22 +2540,27 @@ class Sema final : public SemaBase { bool AllowRedecl = false, Expr *AttrArg = nullptr); - /// Check if applying the specified attribute variant from the "counted by" - /// family of attributes to FieldDecl \p FD is semantically valid. If - /// semantically invalid diagnostics will be emitted explaining the problems. - /// - /// \param FD The FieldDecl to apply the attribute to - /// \param E The count expression on the attribute - /// \param CountInBytes If true the attribute is from the "sized_by" family of - /// attributes. If the false the attribute is from - /// "counted_by" family of attributes. - /// \param OrNull If true the attribute is from the "_or_null" suffixed family - /// of attributes. If false the attribute does not have the - /// suffix. - /// - /// Together \p CountInBytes and \p OrNull decide the attribute variant. E.g. - /// \p CountInBytes and \p OrNull both being true indicates the - /// `counted_by_or_null` attribute. + /// Perform semantic validation on a FieldDecl with a "counted_by" family + /// attribute. This is called after the attribute has been attached to the + /// field's type (as a CountAttributedType) to validate the attribute is + /// correctly applied. + /// + /// This performs declaration-level checks that require the FieldDecl to + /// exist, complementing the type-level checks performed in + /// HandleCountedByAttrOnType during type processing. Specifically, this + /// validates: + /// - Field is not in a union + /// - For array fields, the field is a flexible array member + /// - Count expression is an integer type (not bool) + /// - Count expression references a field in the same struct + /// - Count field is not in a union + /// + /// \param FD The FieldDecl with the attribute + /// \param E The count expression from the attribute + /// \param CountInBytes If true the attribute is from the "sized_by" family. + /// If false the attribute is from the "counted_by" + /// family. + /// \param OrNull If true the attribute has the "_or_null" suffix. /// /// \returns false iff semantically valid. bool CheckCountedByAttrOnField(FieldDecl *FD, Expr *E, bool CountInBytes, diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index 6b121119550b1..fe2cfe8a5e641 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -34,6 +34,7 @@ #include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SmallSet.h" #include "llvm/ADT/StringSwitch.h" +#include "llvm/Support/SaveAndRestore.h" #include <optional> using namespace clang; @@ -118,6 +119,23 @@ static bool IsAttributeArgsParsedInFunctionScope(const IdentifierInfo &II) { #undef CLANG_ATTR_PARSE_ARGS_IN_FUNCTION_SCOPE_LIST } +/// returns true iff the attribute appertains to a type (a TYPE_ATTR or +/// DECL_OR_TYPE_ATTR in `Attr.td`). +static bool IsAttributeTypeAttr(ParsedAttr::Kind Kind) { + switch (Kind) { +#define ATTR(NAME) +#define DECL_OR_TYPE_ATTR(NAME) case ParsedAttr::AT_##NAME: +#define TYPE_ATTR(NAME) case ParsedAttr::AT_##NAME: +#include "clang/Basic/AttrList.inc" + return true; + default: + return false; +#undef DECL_OR_TYPE_ATTR +#undef TYPE_ATTR +#undef ATTR + } +} + /// Check if the a start and end source location expand to the same macro. static bool FindLocsWithCommonFileID(Preprocessor &PP, SourceLocation StartLoc, SourceLocation EndLoc) { @@ -167,6 +185,9 @@ bool Parser::ParseSingleGNUAttribute(ParsedAttributes &Attrs, return false; } + ParsedAttr::Kind AttrKind = ParsedAttr::getParsedKind( + AttrName, nullptr, ParsedAttr::Form::GNU().getSyntax()); + bool LateParse = false; if (!LateAttrs) LateParse = false; @@ -175,7 +196,9 @@ bool Parser::ParseSingleGNUAttribute(ParsedAttributes &Attrs, // parsed for `LateAttrParseExperimentalExt` attributes. This will // only be late parsed if the experimental language option is enabled. LateParse = getLangOpts().ExperimentalLateParseAttributes && - IsAttributeLateParsedExperimentalExt(*AttrName); + IsAttributeLateParsedExperimentalExt(*AttrName) && + (IsAttributeTypeAttr(AttrKind) || + !LateAttrs->lateAttrParseTypeAttrOnly()); } else { // The caller did not restrict late parsing to only // `LateAttrParseExperimentalExt` attributes so late parse @@ -193,10 +216,24 @@ bool Parser::ParseSingleGNUAttribute(ParsedAttributes &Attrs, } // Handle attributes with arguments that require late parsing. - LateParsedAttribute *LA = - new LateParsedAttribute(this, *AttrName, AttrNameLoc); + // Late parsing for type attributes isn't properly supported in C++ yet. + LateParsedAttribute *LA = nullptr; + if (IsAttributeTypeAttr(AttrKind) && !getLangOpts().CPlusPlus) + LA = new LateParsedTypeAttribute(this, *AttrName, AttrNameLoc); + else + LA = new LateParsedAttribute(this, *AttrName, AttrNameLoc); + LateAttrs->push_back(LA); + // Record type attributes against the record currently being parsed, whose + // closing brace is when their arguments become resolvable. `LateAttrs` can't + // serve here: for a declarator-position attribute it is a transient local + // that is drained into a DeclaratorChunk, and for a decl-spec-position one + // TakeTypeAttrsAppendingFrom moves the entry into the DeclSpec. + if (auto *LTA = dyn_cast<LateParsedTypeAttribute>(LA); + LTA && CurRecordLateParsedTypeAttrs) + CurRecordLateParsedTypeAttrs->push_back(LTA); + // Attributes in a class are parsed at the end of the class, along // with other late-parsed declarations. if (!ClassStack.empty() && !LateAttrs->parseSoon()) @@ -3177,10 +3214,11 @@ void Parser::DistributeCLateParsedAttrs(Decl *Dcl, if (!LateAttrs) return; + // 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); } } } @@ -3253,12 +3291,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); } @@ -3498,6 +3530,10 @@ void Parser::ParseDeclarationSpecifiers( DS.takeAttributesAppendingingFrom(attrs); } + if (LateAttrs) { + Parser::TakeTypeAttrsAppendingFrom(DS.getLateAttributes(), *LateAttrs); + } + // If this is not a declaration specifier token, we're done reading decl // specifiers. First verify that DeclSpec's are consistent. DS.Finish(Actions, Policy); @@ -4030,7 +4066,6 @@ void Parser::ParseDeclarationSpecifiers( case tok::kw___declspec: ParseAttributes(PAKM_GNU | PAKM_Declspec, DS.getAttributes(), LateAttrs); continue; - // Microsoft single token adornments. case tok::kw___forceinline: { isInvalid = DS.setFunctionSpecForceInline(Loc, PrevSpec, DiagID); @@ -4782,8 +4817,20 @@ void Parser::ParseStructDeclaration( ParsedAttributes Attrs(AttrFactory); MaybeParseCXX11Attributes(Attrs); + // Late-parsed type attributes written in declaration-specifier position (e.g. + // `IP __counted_by(n) a, b;` where `IP` is a pointer typedef) belong to every + // declarator in this declaration, so remember where this declaration's entries + // start before the specifier list is parsed. Indices, not iterators: the side + // list is a SmallVector and only ever grows within a record body. + unsigned DeclSpecMark = + CurRecordLateParsedTypeAttrs ? CurRecordLateParsedTypeAttrs->size() : 0; + // Parse the common specifier-qualifiers-list piece. - ParseSpecifierQualifierList(DS); + ParseSpecifierQualifierList(DS, AS_none, DeclSpecContext::DSC_normal, + LateFieldAttrs); + + unsigned AfterDeclSpecMark = + CurRecordLateParsedTypeAttrs ? CurRecordLateParsedTypeAttrs->size() : 0; // If there are no declarators, this is a free-standing declaration // specifier. Let the actions module cope with it. @@ -4819,6 +4866,8 @@ void Parser::ParseStructDeclaration( /// struct-declarator: declarator /// struct-declarator: declarator[opt] ':' constant-expression + unsigned DeclMark = + CurRecordLateParsedTypeAttrs ? CurRecordLateParsedTypeAttrs->size() : 0; if (Tok.isNot(tok::colon)) { // Don't parse FOO:BAR as if it were a typo for FOO::BAR. ColonProtectionRAIIObject X(*this); @@ -4847,6 +4896,31 @@ void Parser::ParseStructDeclaration( if (Field) DistributeCLateParsedAttrs(Field, LateFieldAttrs); + // Record the field each pending late-parsed type attribute belongs to, in + // the base class's coupled-decl list. The callback above ran + // GetTypeForDeclarator, so the attribute's type node exists by now; pairing + // it with the field here means the completion pass at the closing brace + // needs no search -- which matters because a bounds type may sit nested + // inside the field's type, where it cannot be recovered by inspecting the + // field's top-level type. + // + // Two ranges apply: attributes from the shared declaration-specifier (every + // declarator in this declaration gets appended), and those from this + // declarator alone. + if (auto *FD = dyn_cast_if_present<FieldDecl>(Field); + FD && CurRecordLateParsedTypeAttrs) { + unsigned Size = CurRecordLateParsedTypeAttrs->size(); + assert(Size >= DeclMark && DeclMark >= AfterDeclSpecMark && + AfterDeclSpecMark >= DeclSpecMark && + "late-parsed type attribute list must only grow"); + auto Attach = [&](unsigned First, unsigned Last) { + for (unsigned I = First; I != Last; ++I) + (*CurRecordLateParsedTypeAttrs)[I]->addDecl(FD); + }; + Attach(DeclSpecMark, AfterDeclSpecMark); + Attach(DeclMark, Size); + } + // If we don't have a comma, it is either the end of the list (a ';') // or an error, bail out. if (!TryConsumeToken(tok::comma, CommaLoc)) @@ -4897,9 +4971,6 @@ ParsedAttributes Parser::ParseLexedAttributeTokens(LateParsedAttribute &LPA) { void Parser::ParseLexedTypeAttribute(LateParsedTypeAttribute &LA, ParsedAttributes &OutAttrs) { - assert(LA.Decls.size() <= 1 && - "late field attribute expects to have at most one declaration."); - ParsedAttributes Attrs = ParseLexedAttributeTokens(LA); OutAttrs.takeAllAppendingFrom(Attrs); } @@ -5020,6 +5091,14 @@ void Parser::ParseStructUnionBody(SourceLocation RecordLoc, LateParsedAttrList LateFieldAttrs(/*PSoon=*/true, /*LateAttrParseExperimentalExtOnly=*/true); + // Pending late-parsed type attributes for this record, populated as its + // fields are parsed and drained at the closing brace. Exposed to nested + // bodies so an anonymous nested record can hand its own up to us; + // `Enclosing.get()` is our caller's list, or null for the outermost record. + SmallVector<LateParsedTypeAttribute *, 2> LateTypeAttrs; + llvm::SaveAndRestore<SmallVectorImpl<LateParsedTypeAttribute *> *> Enclosing( + CurRecordLateParsedTypeAttrs, &LateTypeAttrs); + // While we still have something to read, read the declarations in the struct. while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) && Tok.isNot(tok::eof)) { @@ -5126,15 +5205,49 @@ void Parser::ParseStructUnionBody(SourceLocation RecordLoc, ParsedAttributes attrs(AttrFactory); // If attributes exist after struct contents, parse them. MaybeParseGNUAttributes(attrs, &LateFieldAttrs); - SmallVector<Decl *, 32> FieldDecls(TagDecl->fields()); Actions.ActOnFields(getCurScope(), RecordLoc, TagDecl, FieldDecls, T.getOpenLocation(), T.getCloseLocation(), attrs); // Late parse field attributes if necessary. + // + // Late-parsed type attributes are owned by CompleteLateParsedTypeAttributes + // via the record's side list, which parses their tokens and deletes them. + // They are only in this generic list to be routed to type construction; if + // any remain (e.g. a type attribute that wasn't moved into a DeclSpec / + // DeclaratorChunk), drop them here so ParseLexedAttributeList doesn't parse + // and free them a second time. + llvm::erase_if(LateFieldAttrs, [](LateParsedAttribute *LA) { + return isa<LateParsedTypeAttribute>(LA); + }); ParseLexedAttributeList(LateFieldAttrs, /*D=*/nullptr, /*EnterScope=*/false, /*OnDefinition=*/false); + + // Resolve late-parsed type attributes while this record's fields are still in + // scope. A truly anonymous record can't do that yet — its count may live in + // the enclosing record and only becomes visible once its members are + // flattened in — so it hands its pending attributes up instead. + // + // `isAnonymousStructOrUnion()` isn't set until the enclosing context sees + // whether a declarator follows, which happens after we return. Determine it + // the way the parser can: no tag name and no declarator after the body. Any + // attribute-specifiers between `}` and the `;`/declarator are skipped with a + // reverting tentative parse, so a trailing `[[...]]` / `__attribute__` etc. + // doesn't defeat the check. + if (getLangOpts().ExperimentalLateParseAttributes && + !LateTypeAttrs.empty()) { + bool IsAnonymous = false; + if (!TagDecl->getIdentifier()) { + TentativeParsingAction TPA(*this); + IsAnonymous = TrySkipAttributes() && Tok.is(tok::semi); + TPA.Revert(); + } + if (IsAnonymous && Enclosing.get()) + llvm::append_range(*Enclosing.get(), LateTypeAttrs); + else + CompleteLateParsedTypeAttributes(LateTypeAttrs); + } StructScope.Exit(); Actions.ActOnTagFinishDefinition(getCurScope(), TagDecl, T.getRange()); } @@ -6467,7 +6580,9 @@ void Parser::ParseTypeQualifierListOpt( // recovery is graceful. if (AttrReqs & AR_GNUAttributesParsed || AttrReqs & AR_GNUAttributesParsedAndRejected) { - ParseGNUAttributes(DS.getAttributes()); + + // FIXME: Late parse only when some flag is set. + ParseGNUAttributes(DS.getAttributes(), LateAttrs); continue; // do *not* consume the next token! } // otherwise, FALL THROUGH! @@ -6627,6 +6742,8 @@ void Parser::ParseDeclaratorInternal(Declarator &D, DeclSpec DS(AttrFactory); ParseTypeQualifierListOpt(DS); + assert(DS.getLateAttributes().empty()); + D.AddTypeInfo( DeclaratorChunk::getPipe(DS.getTypeQualifiers(), DS.getPipeLoc()), std::move(DS.getAttributes()), SourceLocation()); @@ -6654,26 +6771,48 @@ void Parser::ParseDeclaratorInternal(Declarator &D, ((D.getContext() != DeclaratorContext::CXXNew) ? AR_GNUAttributesParsed : AR_GNUAttributesParsedAndRejected); + + // Late-parsed type attributes apply to members and function parameters, + // not variables. Completion is driven by the enclosing record + // (CompleteLateParsedTypeAttributes), so only late-parse when there is one: + // a free-function prototype (e.g. `void f(int *__counted_by(n), int n)`) + // has no record to complete into, and late-parsing there would leave a + // CountAttributedType with a null count in the AST. Such parameters fall + // back to eager handling instead. A function-pointer parameter inside a + // struct field is still late-parsed, since that record completes it. + bool LateParsingContext = (D.getContext() == DeclaratorContext::Member || + D.getContext() == DeclaratorContext::Prototype) && + CurRecordLateParsedTypeAttrs != nullptr; + + // No guard on ExperimentalLateParseAttributes is needed here; + // DS.getLateAttributes() already initializes with + // LateAttrParseExperimentalExtOnly. + LateParsedAttrList *LateAttrs = + LateParsingContext ? &DS.getLateAttributes() : nullptr; + 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) { // 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(), DS.getOverflowBehaviorLoc(), DS.isWrapSpecified()), - std::move(DS.getAttributes()), SourceLocation()); - else + ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/224554 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
