[clang] bd41371 - [clang] Fix FIXME in isAlignasAttribute()
Richard Sandiford via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 13 02:16:03 PDT 2023
Author: Richard Sandiford
Date: 2023-04-13T10:14:50+01:00
New Revision: bd41371be02f6f5713459a2f6fe109cd3c01b4a4
URL: https://github.com/llvm/llvm-project/commit/bd41371be02f6f5713459a2f6fe109cd3c01b4a4
DIFF: https://github.com/llvm/llvm-project/commit/bd41371be02f6f5713459a2f6fe109cd3c01b4a4.diff
LOG: [clang] Fix FIXME in isAlignasAttribute()
AttributeCommonInfo::isAlignasAttribute() was used in one place:
isCXX11Attribute(). The intention was for isAlignasAttribute()
to return true for the C++ alignas keyword. However, as a FIXME
noted, the function also returned true for the C _Alignas keyword.
This meant that isCXX11Attribute() returned true for _Alignas as
well as for alignas.
AttributeCommonInfos are now always constructed with an
AttributeCommonInfo::Form. We can use that Form to convey whether
a keyword is alignas or not.
The patch uses 1 bit of an 8-bit hole in the current layout
of AttributeCommonInfo. This might not be the best long-term design,
but it should be easy to adapt the layout if necessary (that is,
if other uses are found for the spare bits).
I don't know of a way of testing this (other than grep -c FIXME)
Differential Revision: https://reviews.llvm.org/D148105
Added:
Modified:
clang/include/clang/Basic/AttributeCommonInfo.h
clang/lib/Sema/SemaType.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/utils/TableGen/ClangAttrEmitter.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/AttributeCommonInfo.h b/clang/include/clang/Basic/AttributeCommonInfo.h
index a5ba5715a60fa..a68f8d97e91a7 100644
--- a/clang/include/clang/Basic/AttributeCommonInfo.h
+++ b/clang/include/clang/Basic/AttributeCommonInfo.h
@@ -76,6 +76,7 @@ class AttributeCommonInfo {
/// Corresponds to the Syntax enum.
unsigned SyntaxUsed : 4;
unsigned SpellingIndex : 4;
+ unsigned IsAlignas : 1;
protected:
static constexpr unsigned SpellingNotCalculated = 0xf;
@@ -85,20 +86,25 @@ class AttributeCommonInfo {
/// including its syntax and spelling.
class Form {
public:
- constexpr Form(Syntax SyntaxUsed, unsigned SpellingIndex)
- : SyntaxUsed(SyntaxUsed), SpellingIndex(SpellingIndex) {}
- constexpr Form(tok::TokenKind)
- : SyntaxUsed(AS_Keyword), SpellingIndex(SpellingNotCalculated) {}
+ constexpr Form(Syntax SyntaxUsed, unsigned SpellingIndex, bool IsAlignas)
+ : SyntaxUsed(SyntaxUsed), SpellingIndex(SpellingIndex),
+ IsAlignas(IsAlignas) {}
+ constexpr Form(tok::TokenKind Tok)
+ : SyntaxUsed(AS_Keyword), SpellingIndex(SpellingNotCalculated),
+ IsAlignas(Tok == tok::kw_alignas) {}
Syntax getSyntax() const { return Syntax(SyntaxUsed); }
unsigned getSpellingIndex() const { return SpellingIndex; }
+ bool isAlignas() const { return IsAlignas; }
static Form GNU() { return AS_GNU; }
static Form CXX11() { return AS_CXX11; }
static Form C2x() { return AS_C2x; }
static Form Declspec() { return AS_Declspec; }
static Form Microsoft() { return AS_Microsoft; }
- static Form Keyword() { return AS_Keyword; }
+ static Form Keyword(bool IsAlignas) {
+ return Form(AS_Keyword, SpellingNotCalculated, IsAlignas);
+ }
static Form Pragma() { return AS_Pragma; }
static Form ContextSensitiveKeyword() { return AS_ContextSensitiveKeyword; }
static Form HLSLSemantic() { return AS_HLSLSemantic; }
@@ -106,10 +112,12 @@ class AttributeCommonInfo {
private:
constexpr Form(Syntax SyntaxUsed)
- : SyntaxUsed(SyntaxUsed), SpellingIndex(SpellingNotCalculated) {}
+ : SyntaxUsed(SyntaxUsed), SpellingIndex(SpellingNotCalculated),
+ IsAlignas(0) {}
unsigned SyntaxUsed : 4;
unsigned SpellingIndex : 4;
+ unsigned IsAlignas : 1;
};
AttributeCommonInfo(const IdentifierInfo *AttrName,
@@ -119,7 +127,8 @@ class AttributeCommonInfo {
ScopeLoc(ScopeLoc),
AttrKind(getParsedKind(AttrName, ScopeName, FormUsed.getSyntax())),
SyntaxUsed(FormUsed.getSyntax()),
- SpellingIndex(FormUsed.getSpellingIndex()) {}
+ SpellingIndex(FormUsed.getSpellingIndex()),
+ IsAlignas(FormUsed.isAlignas()) {}
AttributeCommonInfo(const IdentifierInfo *AttrName,
const IdentifierInfo *ScopeName, SourceRange AttrRange,
@@ -127,7 +136,8 @@ class AttributeCommonInfo {
: AttrName(AttrName), ScopeName(ScopeName), AttrRange(AttrRange),
ScopeLoc(ScopeLoc), AttrKind(AttrKind),
SyntaxUsed(FormUsed.getSyntax()),
- SpellingIndex(FormUsed.getSpellingIndex()) {}
+ SpellingIndex(FormUsed.getSpellingIndex()),
+ IsAlignas(FormUsed.isAlignas()) {}
AttributeCommonInfo(const IdentifierInfo *AttrName, SourceRange AttrRange,
Form FormUsed)
@@ -135,19 +145,21 @@ class AttributeCommonInfo {
ScopeLoc(),
AttrKind(getParsedKind(AttrName, ScopeName, FormUsed.getSyntax())),
SyntaxUsed(FormUsed.getSyntax()),
- SpellingIndex(FormUsed.getSpellingIndex()) {}
+ SpellingIndex(FormUsed.getSpellingIndex()),
+ IsAlignas(FormUsed.isAlignas()) {}
AttributeCommonInfo(SourceRange AttrRange, Kind K, Form FormUsed)
: AttrName(nullptr), ScopeName(nullptr), AttrRange(AttrRange), ScopeLoc(),
AttrKind(K), SyntaxUsed(FormUsed.getSyntax()),
- SpellingIndex(FormUsed.getSpellingIndex()) {}
+ SpellingIndex(FormUsed.getSpellingIndex()),
+ IsAlignas(FormUsed.isAlignas()) {}
AttributeCommonInfo(AttributeCommonInfo &&) = default;
AttributeCommonInfo(const AttributeCommonInfo &) = default;
Kind getParsedKind() const { return Kind(AttrKind); }
Syntax getSyntax() const { return Syntax(SyntaxUsed); }
- Form getForm() const { return Form(getSyntax(), SpellingIndex); }
+ Form getForm() const { return Form(getSyntax(), SpellingIndex, IsAlignas); }
const IdentifierInfo *getAttrName() const { return AttrName; }
SourceLocation getLoc() const { return AttrRange.getBegin(); }
SourceRange getRange() const { return AttrRange; }
@@ -168,29 +180,7 @@ class AttributeCommonInfo {
bool isGNUScope() const;
bool isClangScope() const;
- bool isAlignasAttribute() const {
- // FIXME: Use a better mechanism to determine this.
- // We use this in `isCXX11Attribute` below, so it _should_ only return
- // true for the `alignas` spelling, but it currently also returns true
- // for the `_Alignas` spelling, which only exists in C11. Distinguishing
- // between the two is important because they behave
diff erently:
- // - `alignas` may only appear in the attribute-specifier-seq before
- // the decl-specifier-seq and is therefore associated with the
- // declaration.
- // - `_Alignas` may appear anywhere within the declaration-specifiers
- // and is therefore associated with the `DeclSpec`.
- // It's not clear how best to fix this:
- // - We have the necessary information in the form of the `SpellingIndex`,
- // but we would need to compare against AlignedAttr::Keyword_alignas,
- // and we can't depend on clang/AST/Attr.h here.
- // - We could test `getAttrName()->getName() == "alignas"`, but this is
- // inefficient.
- return getParsedKind() == AT_Aligned && isKeywordAttribute();
- }
-
- bool isCXX11Attribute() const {
- return SyntaxUsed == AS_CXX11 || isAlignasAttribute();
- }
+ bool isCXX11Attribute() const { return SyntaxUsed == AS_CXX11 || IsAlignas; }
bool isC2xAttribute() const { return SyntaxUsed == AS_C2x; }
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 3f798adfe6bb5..be44803947501 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -4893,9 +4893,9 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
// If we're supposed to infer nullability, do so now.
if (inferNullability && !inferNullabilityInnerOnlyComplete) {
- ParsedAttr::Form form = inferNullabilityCS
- ? ParsedAttr::Form::ContextSensitiveKeyword()
- : ParsedAttr::Form::Keyword();
+ ParsedAttr::Form form =
+ inferNullabilityCS ? ParsedAttr::Form::ContextSensitiveKeyword()
+ : ParsedAttr::Form::Keyword(false /*IsAlignAs*/);
ParsedAttr *nullabilityAttr = Pool.create(
S.getNullabilityKeyword(*inferNullability), SourceRange(pointerLoc),
nullptr, SourceLocation(), nullptr, 0, form);
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index c72761c2b44ca..5ce7007d8acef 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3091,11 +3091,14 @@ Attr *ASTRecordReader::readAttr() {
unsigned ParsedKind = Record.readInt();
unsigned Syntax = Record.readInt();
unsigned SpellingIndex = Record.readInt();
+ bool IsAlignas = (ParsedKind == AttributeCommonInfo::AT_Aligned &&
+ Syntax == AttributeCommonInfo::AS_Keyword &&
+ SpellingIndex == AlignedAttr::Keyword_alignas);
AttributeCommonInfo Info(
AttrName, ScopeName, AttrRange, ScopeLoc,
AttributeCommonInfo::Kind(ParsedKind),
- {AttributeCommonInfo::Syntax(Syntax), SpellingIndex});
+ {AttributeCommonInfo::Syntax(Syntax), SpellingIndex, IsAlignas});
#include "clang/Serialization/AttrPCHRead.inc"
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp
index d094e8bb36ce5..231b10f3d7f9d 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -2381,8 +2381,11 @@ static void emitClangAttrAcceptsExprPack(RecordKeeper &Records,
static void emitFormInitializer(raw_ostream &OS,
const FlattenedSpelling &Spelling,
StringRef SpellingIndex) {
+ bool IsAlignas =
+ (Spelling.variety() == "Keyword" && Spelling.name() == "alignas");
OS << "{AttributeCommonInfo::AS_" << Spelling.variety() << ", "
- << SpellingIndex << "}";
+ << SpellingIndex << ", " << (IsAlignas ? "true" : "false")
+ << " /*IsAlignas*/}";
}
static void emitAttributes(RecordKeeper &Records, raw_ostream &OS,
More information about the cfe-commits
mailing list