Skip to content

Commit 2acf77f

Browse files
mikerice1969mikerice1969
authored
[clang] Update argument checking tablegen code to use a 'full' name (#99993)
In 92fc1eb the HLSLLoopHint attribute was added with an 'unroll' spelling. There is an existing LoopHint attribute with the same spelling. These attributes have different arguments. The tablegen used to produce checks on arguments uses only the attribute name, making it impossible to return correct info for attribute with different argument types but the same name. Improve the situation by using a 'full' name that combines the syntax, scope, and name. This allows, for example, #pragma unroll and [[unroll(x)]] to coexist correctly even with different argument types. Also fix a bug in the StrictEnumParameters tablegen. If will now correctly specify each parameter instead of only the first.
1 parent 39b6900 commit 2acf77f

File tree

7 files changed

+400
-72
lines changed

7 files changed

+400
-72
lines changed

‎clang/include/clang/Basic/AttributeCommonInfo.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,12 @@ class AttributeCommonInfo {
191191
/// __gnu__::__attr__ will be normalized to gnu::attr).
192192
std::string getNormalizedFullName() const;
193193

194+
/// Generate a normalized full name, with syntax, scope and name.
195+
static std::string
196+
normalizeFullNameWithSyntax(const IdentifierInfo *Name,
197+
const IdentifierInfo *Scope,
198+
AttributeCommonInfo::Syntax SyntaxUsed);
199+
194200
bool isDeclspecAttribute() const { return SyntaxUsed == AS_Declspec; }
195201
bool isMicrosoftAttribute() const { return SyntaxUsed == AS_Microsoft; }
196202

‎clang/lib/Basic/Attributes.cpp‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,40 @@ std::string AttributeCommonInfo::getNormalizedFullName() const {
153153
normalizeName(getAttrName(), getScopeName(), getSyntax()));
154154
}
155155

156+
static StringRef getSyntaxName(AttributeCommonInfo::Syntax SyntaxUsed) {
157+
switch (SyntaxUsed) {
158+
case AttributeCommonInfo::AS_GNU:
159+
return "GNU";
160+
case AttributeCommonInfo::AS_CXX11:
161+
return "CXX11";
162+
case AttributeCommonInfo::AS_C23:
163+
return "C23";
164+
case AttributeCommonInfo::AS_Declspec:
165+
return "Declspec";
166+
case AttributeCommonInfo::AS_Microsoft:
167+
return "Microsoft";
168+
case AttributeCommonInfo::AS_Keyword:
169+
return "Keyword";
170+
case AttributeCommonInfo::AS_Pragma:
171+
return "Pragma";
172+
case AttributeCommonInfo::AS_ContextSensitiveKeyword:
173+
return "ContextSensitiveKeyword";
174+
case AttributeCommonInfo::AS_HLSLAnnotation:
175+
return "HLSLAnnotation";
176+
case AttributeCommonInfo::AS_Implicit:
177+
return "Implicit";
178+
}
179+
llvm_unreachable("Invalid attribute syntax");
180+
}
181+
182+
std::string AttributeCommonInfo::normalizeFullNameWithSyntax(
183+
const IdentifierInfo *Name, const IdentifierInfo *ScopeName,
184+
Syntax SyntaxUsed) {
185+
return (Twine(getSyntaxName(SyntaxUsed)) +
186+
"::" + normalizeName(Name, ScopeName, SyntaxUsed))
187+
.str();
188+
}
189+
156190
unsigned AttributeCommonInfo::calculateAttributeSpellingListIndex() const {
157191
// Both variables will be used in tablegen generated
158192
// attribute spell list index matching code.

‎clang/lib/Parse/ParseDecl.cpp‎

Lines changed: 79 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -314,64 +314,92 @@ void Parser::ParseGNUAttributes(ParsedAttributes &Attrs,
314314
}
315315

316316
/// Determine whether the given attribute has an identifier argument.
317-
static bool attributeHasIdentifierArg(const IdentifierInfo &II) {
317+
static bool attributeHasIdentifierArg(const IdentifierInfo &II,
318+
ParsedAttr::Syntax Syntax,
319+
IdentifierInfo *ScopeName) {
320+
std::string FullName =
321+
AttributeCommonInfo::normalizeFullNameWithSyntax(&II, ScopeName, Syntax);
318322
#define CLANG_ATTR_IDENTIFIER_ARG_LIST
319-
return llvm::StringSwitch<bool>(normalizeAttrName(II.getName()))
323+
return llvm::StringSwitch<bool>(FullName)
320324
#include "clang/Parse/AttrParserStringSwitches.inc"
321-
.Default(false);
325+
.Default(false);
322326
#undef CLANG_ATTR_IDENTIFIER_ARG_LIST
323327
}
324328

325329
/// Determine whether the given attribute has an identifier argument.
326330
static ParsedAttributeArgumentsProperties
327-
attributeStringLiteralListArg(const llvm::Triple &T, const IdentifierInfo &II) {
331+
attributeStringLiteralListArg(const llvm::Triple &T, const IdentifierInfo &II,
332+
ParsedAttr::Syntax Syntax,
333+
IdentifierInfo *ScopeName) {
334+
std::string FullName =
335+
AttributeCommonInfo::normalizeFullNameWithSyntax(&II, ScopeName, Syntax);
328336
#define CLANG_ATTR_STRING_LITERAL_ARG_LIST
329-
return llvm::StringSwitch<uint32_t>(normalizeAttrName(II.getName()))
337+
return llvm::StringSwitch<uint32_t>(FullName)
330338
#include "clang/Parse/AttrParserStringSwitches.inc"
331339
.Default(0);
332340
#undef CLANG_ATTR_STRING_LITERAL_ARG_LIST
333341
}
334342

335343
/// Determine whether the given attribute has a variadic identifier argument.
336-
static bool attributeHasVariadicIdentifierArg(const IdentifierInfo &II) {
344+
static bool attributeHasVariadicIdentifierArg(const IdentifierInfo &II,
345+
ParsedAttr::Syntax Syntax,
346+
IdentifierInfo *ScopeName) {
347+
std::string FullName =
348+
AttributeCommonInfo::normalizeFullNameWithSyntax(&II, ScopeName, Syntax);
337349
#define CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST
338-
return llvm::StringSwitch<bool>(normalizeAttrName(II.getName()))
350+
return llvm::StringSwitch<bool>(FullName)
339351
#include "clang/Parse/AttrParserStringSwitches.inc"
340-
.Default(false);
352+
.Default(false);
341353
#undef CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST
342354
}
343355

344356
/// Determine whether the given attribute treats kw_this as an identifier.
345-
static bool attributeTreatsKeywordThisAsIdentifier(const IdentifierInfo &II) {
357+
static bool attributeTreatsKeywordThisAsIdentifier(const IdentifierInfo &II,
358+
ParsedAttr::Syntax Syntax,
359+
IdentifierInfo *ScopeName) {
360+
std::string FullName =
361+
AttributeCommonInfo::normalizeFullNameWithSyntax(&II, ScopeName, Syntax);
346362
#define CLANG_ATTR_THIS_ISA_IDENTIFIER_ARG_LIST
347-
return llvm::StringSwitch<bool>(normalizeAttrName(II.getName()))
363+
return llvm::StringSwitch<bool>(FullName)
348364
#include "clang/Parse/AttrParserStringSwitches.inc"
349-
.Default(false);
365+
.Default(false);
350366
#undef CLANG_ATTR_THIS_ISA_IDENTIFIER_ARG_LIST
351367
}
352368

353369
/// Determine if an attribute accepts parameter packs.
354-
static bool attributeAcceptsExprPack(const IdentifierInfo &II) {
370+
static bool attributeAcceptsExprPack(const IdentifierInfo &II,
371+
ParsedAttr::Syntax Syntax,
372+
IdentifierInfo *ScopeName) {
373+
std::string FullName =
374+
AttributeCommonInfo::normalizeFullNameWithSyntax(&II, ScopeName, Syntax);
355375
#define CLANG_ATTR_ACCEPTS_EXPR_PACK
356-
return llvm::StringSwitch<bool>(normalizeAttrName(II.getName()))
376+
return llvm::StringSwitch<bool>(FullName)
357377
#include "clang/Parse/AttrParserStringSwitches.inc"
358378
.Default(false);
359379
#undef CLANG_ATTR_ACCEPTS_EXPR_PACK
360380
}
361381

362382
/// Determine whether the given attribute parses a type argument.
363-
static bool attributeIsTypeArgAttr(const IdentifierInfo &II) {
383+
static bool attributeIsTypeArgAttr(const IdentifierInfo &II,
384+
ParsedAttr::Syntax Syntax,
385+
IdentifierInfo *ScopeName) {
386+
std::string FullName =
387+
AttributeCommonInfo::normalizeFullNameWithSyntax(&II, ScopeName, Syntax);
364388
#define CLANG_ATTR_TYPE_ARG_LIST
365-
return llvm::StringSwitch<bool>(normalizeAttrName(II.getName()))
389+
return llvm::StringSwitch<bool>(FullName)
366390
#include "clang/Parse/AttrParserStringSwitches.inc"
367-
.Default(false);
391+
.Default(false);
368392
#undef CLANG_ATTR_TYPE_ARG_LIST
369393
}
370394

371395
/// Determine whether the given attribute takes identifier arguments.
372-
static bool attributeHasStrictIdentifierArgs(const IdentifierInfo &II) {
396+
static bool attributeHasStrictIdentifierArgs(const IdentifierInfo &II,
397+
ParsedAttr::Syntax Syntax,
398+
IdentifierInfo *ScopeName) {
399+
std::string FullName =
400+
AttributeCommonInfo::normalizeFullNameWithSyntax(&II, ScopeName, Syntax);
373401
#define CLANG_ATTR_STRICT_IDENTIFIER_ARG_AT_INDEX_LIST
374-
return (llvm::StringSwitch<uint64_t>(normalizeAttrName(II.getName()))
402+
return (llvm::StringSwitch<uint64_t>(FullName)
375403
#include "clang/Parse/AttrParserStringSwitches.inc"
376404
.Default(0)) != 0;
377405
#undef CLANG_ATTR_STRICT_IDENTIFIER_ARG_AT_INDEX_LIST
@@ -380,9 +408,13 @@ static bool attributeHasStrictIdentifierArgs(const IdentifierInfo &II) {
380408
/// Determine whether the given attribute takes an identifier argument at a
381409
/// specific index
382410
static bool attributeHasStrictIdentifierArgAtIndex(const IdentifierInfo &II,
411+
ParsedAttr::Syntax Syntax,
412+
IdentifierInfo *ScopeName,
383413
size_t argIndex) {
414+
std::string FullName =
415+
AttributeCommonInfo::normalizeFullNameWithSyntax(&II, ScopeName, Syntax);
384416
#define CLANG_ATTR_STRICT_IDENTIFIER_ARG_AT_INDEX_LIST
385-
return (llvm::StringSwitch<uint64_t>(normalizeAttrName(II.getName()))
417+
return (llvm::StringSwitch<uint64_t>(FullName)
386418
#include "clang/Parse/AttrParserStringSwitches.inc"
387419
.Default(0)) &
388420
(1ull << argIndex);
@@ -391,11 +423,15 @@ static bool attributeHasStrictIdentifierArgAtIndex(const IdentifierInfo &II,
391423

392424
/// Determine whether the given attribute requires parsing its arguments
393425
/// in an unevaluated context or not.
394-
static bool attributeParsedArgsUnevaluated(const IdentifierInfo &II) {
426+
static bool attributeParsedArgsUnevaluated(const IdentifierInfo &II,
427+
ParsedAttr::Syntax Syntax,
428+
IdentifierInfo *ScopeName) {
429+
std::string FullName =
430+
AttributeCommonInfo::normalizeFullNameWithSyntax(&II, ScopeName, Syntax);
395431
#define CLANG_ATTR_ARG_CONTEXT_LIST
396-
return llvm::StringSwitch<bool>(normalizeAttrName(II.getName()))
432+
return llvm::StringSwitch<bool>(FullName)
397433
#include "clang/Parse/AttrParserStringSwitches.inc"
398-
.Default(false);
434+
.Default(false);
399435
#undef CLANG_ATTR_ARG_CONTEXT_LIST
400436
}
401437

@@ -523,10 +559,12 @@ unsigned Parser::ParseAttributeArgsCommon(
523559
// Ignore the left paren location for now.
524560
ConsumeParen();
525561

526-
bool ChangeKWThisToIdent = attributeTreatsKeywordThisAsIdentifier(*AttrName);
527-
bool AttributeIsTypeArgAttr = attributeIsTypeArgAttr(*AttrName);
562+
bool ChangeKWThisToIdent = attributeTreatsKeywordThisAsIdentifier(
563+
*AttrName, Form.getSyntax(), ScopeName);
564+
bool AttributeIsTypeArgAttr =
565+
attributeIsTypeArgAttr(*AttrName, Form.getSyntax(), ScopeName);
528566
bool AttributeHasVariadicIdentifierArg =
529-
attributeHasVariadicIdentifierArg(*AttrName);
567+
attributeHasVariadicIdentifierArg(*AttrName, Form.getSyntax(), ScopeName);
530568

531569
// Interpret "kw_this" as an identifier if the attributed requests it.
532570
if (ChangeKWThisToIdent && Tok.is(tok::kw_this))
@@ -535,8 +573,9 @@ unsigned Parser::ParseAttributeArgsCommon(
535573
ArgsVector ArgExprs;
536574
if (Tok.is(tok::identifier)) {
537575
// If this attribute wants an 'identifier' argument, make it so.
538-
bool IsIdentifierArg = AttributeHasVariadicIdentifierArg ||
539-
attributeHasIdentifierArg(*AttrName);
576+
bool IsIdentifierArg =
577+
AttributeHasVariadicIdentifierArg ||
578+
attributeHasIdentifierArg(*AttrName, Form.getSyntax(), ScopeName);
540579
ParsedAttr::Kind AttrKind =
541580
ParsedAttr::getParsedKind(AttrName, ScopeName, Form.getSyntax());
542581

@@ -568,7 +607,8 @@ unsigned Parser::ParseAttributeArgsCommon(
568607
if (T.isUsable())
569608
TheParsedType = T.get();
570609
} else if (AttributeHasVariadicIdentifierArg ||
571-
attributeHasStrictIdentifierArgs(*AttrName)) {
610+
attributeHasStrictIdentifierArgs(*AttrName, Form.getSyntax(),
611+
ScopeName)) {
572612
// Parse variadic identifier arg. This can either consume identifiers or
573613
// expressions. Variadic identifier args do not support parameter packs
574614
// because those are typically used for attributes with enumeration
@@ -579,8 +619,9 @@ unsigned Parser::ParseAttributeArgsCommon(
579619
if (ChangeKWThisToIdent && Tok.is(tok::kw_this))
580620
Tok.setKind(tok::identifier);
581621

582-
if (Tok.is(tok::identifier) && attributeHasStrictIdentifierArgAtIndex(
583-
*AttrName, ArgExprs.size())) {
622+
if (Tok.is(tok::identifier) &&
623+
attributeHasStrictIdentifierArgAtIndex(
624+
*AttrName, Form.getSyntax(), ScopeName, ArgExprs.size())) {
584625
ArgExprs.push_back(ParseIdentifierLoc());
585626
continue;
586627
}
@@ -589,7 +630,8 @@ unsigned Parser::ParseAttributeArgsCommon(
589630
if (Tok.is(tok::identifier)) {
590631
ArgExprs.push_back(ParseIdentifierLoc());
591632
} else {
592-
bool Uneval = attributeParsedArgsUnevaluated(*AttrName);
633+
bool Uneval = attributeParsedArgsUnevaluated(
634+
*AttrName, Form.getSyntax(), ScopeName);
593635
EnterExpressionEvaluationContext Unevaluated(
594636
Actions,
595637
Uneval ? Sema::ExpressionEvaluationContext::Unevaluated
@@ -610,7 +652,8 @@ unsigned Parser::ParseAttributeArgsCommon(
610652
} while (TryConsumeToken(tok::comma));
611653
} else {
612654
// General case. Parse all available expressions.
613-
bool Uneval = attributeParsedArgsUnevaluated(*AttrName);
655+
bool Uneval = attributeParsedArgsUnevaluated(*AttrName, Form.getSyntax(),
656+
ScopeName);
614657
EnterExpressionEvaluationContext Unevaluated(
615658
Actions,
616659
Uneval ? Sema::ExpressionEvaluationContext::Unevaluated
@@ -621,7 +664,8 @@ unsigned Parser::ParseAttributeArgsCommon(
621664

622665
ExprVector ParsedExprs;
623666
ParsedAttributeArgumentsProperties ArgProperties =
624-
attributeStringLiteralListArg(getTargetInfo().getTriple(), *AttrName);
667+
attributeStringLiteralListArg(getTargetInfo().getTriple(), *AttrName,
668+
Form.getSyntax(), ScopeName);
625669
if (ParseAttributeArgumentList(*AttrName, ParsedExprs, ArgProperties)) {
626670
SkipUntil(tok::r_paren, StopAtSemi);
627671
return 0;
@@ -632,7 +676,7 @@ unsigned Parser::ParseAttributeArgsCommon(
632676
if (!isa<PackExpansionExpr>(ParsedExprs[I]))
633677
continue;
634678

635-
if (!attributeAcceptsExprPack(*AttrName)) {
679+
if (!attributeAcceptsExprPack(*AttrName, Form.getSyntax(), ScopeName)) {
636680
Diag(Tok.getLocation(),
637681
diag::err_attribute_argument_parm_pack_not_supported)
638682
<< AttrName;
@@ -696,7 +740,7 @@ void Parser::ParseGNUAttributeArgs(
696740
ParseTypeTagForDatatypeAttribute(*AttrName, AttrNameLoc, Attrs, EndLoc,
697741
ScopeName, ScopeLoc, Form);
698742
return;
699-
} else if (attributeIsTypeArgAttr(*AttrName)) {
743+
} else if (attributeIsTypeArgAttr(*AttrName, Form.getSyntax(), ScopeName)) {
700744
ParseAttributeWithTypeArg(*AttrName, AttrNameLoc, Attrs, ScopeName,
701745
ScopeLoc, Form);
702746
return;

‎clang/lib/Sema/SemaStmtAttr.cpp‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -593,13 +593,6 @@ static Attr *handleHLSLLoopHintAttr(Sema &S, Stmt *St, const ParsedAttr &A,
593593

594594
unsigned UnrollFactor = 0;
595595
if (A.getNumArgs() == 1) {
596-
597-
if (A.isArgIdent(0)) {
598-
S.Diag(A.getLoc(), diag::err_attribute_argument_type)
599-
<< A << AANT_ArgumentIntegerConstant << A.getRange();
600-
return nullptr;
601-
}
602-
603596
Expr *E = A.getArgAsExpr(0);
604597

605598
if (S.CheckLoopHintExpr(E, St->getBeginLoc(),

‎clang/test/SemaHLSL/Loops/unroll.hlsl‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
// RUN: %clang_cc1 -O0 -finclude-default-header -fsyntax-only -triple dxil-pc-shadermodel6.6-library %s -verify
22
void unroll_no_vars() {
3+
// expected-note@+1 {{declared here}}
34
int I = 3;
4-
[unroll(I)] // expected-error {{'unroll' attribute requires an integer constant}}
5+
// expected-error@+2 {{expression is not an integral constant expression}}
6+
// expected-note@+1 {{read of non-const variable 'I' is not allowed in a constant expression}}
7+
[unroll(I)]
58
while (I--);
69
}
710

0 commit comments

Comments
 (0)