nickdesaulniers updated this revision to Diff 248816.
nickdesaulniers marked an inline comment as done.
nickdesaulniers added a comment.
Herald added subscribers: jfb, aheejin.
- completely drop use of Parse::ParseTypeQualifierListOpt
- move paren parsing into helper
- fix up test cases for global asm statements
- delete duplicate test cases covered by the added test file
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75563/new/
https://reviews.llvm.org/D75563
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/DeclSpec.h
clang/lib/Parse/ParseStmtAsm.cpp
clang/lib/Parse/Parser.cpp
clang/lib/Sema/DeclSpec.cpp
clang/test/CodeGen/inline-asm-mixed-style.c
clang/test/Parser/asm-qualifiers.c
clang/test/Parser/asm.c
clang/test/Sema/asm.c
Index: clang/test/Sema/asm.c
===================================================================
--- clang/test/Sema/asm.c
+++ clang/test/Sema/asm.c
@@ -91,9 +91,6 @@
return a;
}
-// <rdar://problem/7574870>
-asm volatile (""); // expected-warning {{meaningless 'volatile' on asm outside function}}
-
// PR3904
void test8(int i) {
// A number in an input constraint can't point to a read-write constraint.
Index: clang/test/Parser/asm.c
===================================================================
--- clang/test/Parser/asm.c
+++ clang/test/Parser/asm.c
@@ -12,12 +12,6 @@
void f2() {
asm("foo" : "=r" (a)); // expected-error {{use of undeclared identifier 'a'}}
asm("foo" : : "r" (b)); // expected-error {{use of undeclared identifier 'b'}}
-
- asm const (""); // expected-warning {{ignored const qualifier on asm}}
- asm volatile ("");
- asm restrict (""); // expected-warning {{ignored restrict qualifier on asm}}
- // FIXME: Once GCC supports _Atomic, check whether it allows this.
- asm _Atomic (""); // expected-warning {{ignored _Atomic qualifier on asm}}
}
void a() __asm__(""); // expected-error {{cannot use an empty string literal in 'asm'}}
Index: clang/test/Parser/asm-qualifiers.c
===================================================================
--- /dev/null
+++ clang/test/Parser/asm-qualifiers.c
@@ -0,0 +1,59 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify %s
+
+void qualifiers(void) {
+ asm("");
+ asm volatile("");
+ asm inline("");
+ asm goto("" ::::foo);
+foo:;
+}
+
+void unknown_qualifiers(void) {
+ asm noodle(""); // expected-error {{expected 'volatile', 'inline', 'goto', or '('}}
+ asm goto noodle("" ::::foo); // expected-error {{expected 'volatile', 'inline', 'goto', or '('}}
+ asm volatile noodle inline(""); // expected-error {{expected 'volatile', 'inline', 'goto', or '('}}
+foo:;
+}
+
+void underscores(void) {
+ __asm__("");
+ __asm__ __volatile__("");
+ __asm__ __inline__("");
+ // Note: goto is not supported with underscore prefix+suffix.
+ __asm__ goto("" ::::foo);
+foo:;
+}
+
+void permutations(void) {
+ asm goto inline volatile("" ::::foo);
+ asm goto inline("");
+ asm goto volatile inline("" ::::foo);
+ asm goto volatile("");
+ asm inline goto volatile("" ::::foo);
+ asm inline goto("" ::::foo);
+ asm inline volatile goto("" ::::foo);
+ asm inline volatile("");
+ asm volatile goto("" ::::foo);
+ asm volatile inline goto("" ::::foo);
+ asm volatile inline("");
+foo:;
+}
+
+void duplicates(void) {
+ asm volatile volatile(""); // expected-error {{duplicate asm qualifier 'volatile'}}
+ __asm__ __volatile__ __volatile__(""); // expected-error {{duplicate asm qualifier 'volatile'}}
+ asm inline inline(""); // expected-error {{duplicate asm qualifier 'inline'}}
+ __asm__ __inline__ __inline__(""); // expected-error {{duplicate asm qualifier 'inline'}}
+ asm goto goto("" ::::foo); // expected-error {{duplicate asm qualifier 'goto'}}
+ __asm__ goto goto("" ::::foo); // expected-error {{duplicate asm qualifier 'goto'}}
+foo:;
+}
+
+// globals
+asm ("");
+// <rdar://problem/7574870>
+asm volatile (""); // expected-error {{meaningless 'volatile' on asm outside function}}
+asm inline (""); // expected-error {{meaningless 'inline' on asm outside function}}
+asm goto (""::::noodle); // expected-error {{meaningless 'goto' on asm outside function}}
+// expected-error@-1 {{expected ')'}}
+// expected-note@-2 {{to match this '('}}
Index: clang/test/CodeGen/inline-asm-mixed-style.c
===================================================================
--- clang/test/CodeGen/inline-asm-mixed-style.c
+++ clang/test/CodeGen/inline-asm-mixed-style.c
@@ -14,11 +14,6 @@
// CHECK: movl %ebx, %eax
// CHECK: movl %ecx, %edx
- __asm mov eax, ebx
- __asm const ("movl %ecx, %edx"); // expected-warning {{ignored const qualifier on asm}}
- // CHECK: movl %ebx, %eax
- // CHECK: movl %ecx, %edx
-
__asm volatile goto ("movl %ecx, %edx");
// CHECK: movl %ecx, %edx
Index: clang/lib/Sema/DeclSpec.cpp
===================================================================
--- clang/lib/Sema/DeclSpec.cpp
+++ clang/lib/Sema/DeclSpec.cpp
@@ -589,6 +589,16 @@
llvm_unreachable("Unknown typespec!");
}
+const char *DeclSpec::getSpecifierName(AQ A) {
+ switch (A) {
+ case DeclSpec::AQ_unspecified: return "unspecified";
+ case DeclSpec::AQ_volatile: return "volatile";
+ case DeclSpec::AQ_inline: return "inline";
+ case DeclSpec::AQ_goto: return "goto";
+ }
+ llvm_unreachable("Unknown GNUAsmQualifier!");
+}
+
bool DeclSpec::SetStorageClassSpec(Sema &S, SCS SC, SourceLocation Loc,
const char *&PrevSpec,
unsigned &DiagID,
@@ -938,6 +948,12 @@
llvm_unreachable("Unknown type qualifier!");
}
+bool DeclSpec::setGNUAsmQualifier(AQ A, SourceLocation Loc) {
+ bool IsInvalid = GNUAsmQualifiers & A;
+ GNUAsmQualifiers |= A;
+ return IsInvalid;
+}
+
bool DeclSpec::setFunctionSpecInline(SourceLocation Loc, const char *&PrevSpec,
unsigned &DiagID) {
// 'inline inline' is ok. However, since this is likely not what the user
Index: clang/lib/Parse/Parser.cpp
===================================================================
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -1528,12 +1528,19 @@
assert(Tok.is(tok::kw_asm) && "Not an asm!");
SourceLocation Loc = ConsumeToken();
- if (Tok.is(tok::kw_volatile)) {
- // Remove from the end of 'asm' to the end of 'volatile'.
+ if (Tok.is(tok::kw_volatile) || Tok.is(tok::kw_goto) || Tok.is(tok::kw_inline)) {
+ // Remove from the end of 'asm' to the end of the asm qualifier.
SourceRange RemovalRange(PP.getLocForEndOfToken(Loc),
PP.getLocForEndOfToken(Tok.getLocation()));
- Diag(Tok, diag::warn_file_asm_volatile)
+ DeclSpec::AQ AQ = DeclSpec::AQ_unspecified;
+ if (Tok.is(tok::kw_volatile))
+ AQ = DeclSpec::AQ_volatile;
+ else if (Tok.is(tok::kw_goto))
+ AQ = DeclSpec::AQ_goto;
+ else if (Tok.is(tok::kw_inline))
+ AQ = DeclSpec::AQ_inline;
+ Diag(Tok, diag::err_file_asm_volatile) << DeclSpec::getSpecifierName(AQ)
<< FixItHint::CreateRemoval(RemovalRange);
ConsumeToken();
}
Index: clang/lib/Parse/ParseStmtAsm.cpp
===================================================================
--- clang/lib/Parse/ParseStmtAsm.cpp
+++ clang/lib/Parse/ParseStmtAsm.cpp
@@ -349,31 +349,10 @@
return false;
}
-/// isTypeQualifier - Return true if the current token could be the
-/// start of a type-qualifier-list.
-static bool isTypeQualifier(const Token &Tok) {
- switch (Tok.getKind()) {
- default: return false;
- // type-qualifier
- case tok::kw_const:
- case tok::kw_volatile:
- case tok::kw_restrict:
- case tok::kw___private:
- case tok::kw___local:
- case tok::kw___global:
- case tok::kw___constant:
- case tok::kw___generic:
- case tok::kw___read_only:
- case tok::kw___read_write:
- case tok::kw___write_only:
- return true;
- }
-}
-
// Determine if this is a GCC-style asm statement.
static bool isGCCAsmStatement(const Token &TokAfterAsm) {
- return TokAfterAsm.is(tok::l_paren) || TokAfterAsm.is(tok::kw_goto) ||
- isTypeQualifier(TokAfterAsm);
+ return TokAfterAsm.is(tok::l_paren) || TokAfterAsm.is(tok::kw_volatile) ||
+ TokAfterAsm.is(tok::kw_inline) || TokAfterAsm.is(tok::kw_goto);
}
/// ParseMicrosoftAsmStatement. When -fms-extensions/-fasm-blocks is enabled,
@@ -684,13 +663,51 @@
ClobberRefs, Exprs, EndLoc);
}
+/// ParseGNUAsmQualifierListOpt - Parse a GNU extended asm qualifier list.
+/// asm-qualifier:
+/// volatile
+/// inline
+/// goto
+///
+/// asm-qualifier-list:
+/// asm-qualifier
+/// asm-qualifier-list asm-qualifier
+bool Parser::ParseGNUAsmQualifierListOpt(DeclSpec &DS) {
+ SourceLocation EndLoc;
+ while (1) {
+ SourceLocation Loc = Tok.getLocation();
+ DeclSpec::AQ AQ = DeclSpec::AQ_unspecified;
+
+ if (Tok.getKind() == tok::kw_volatile)
+ AQ = DeclSpec::AQ_volatile;
+ else if (Tok.getKind() == tok::kw_inline)
+ AQ = DeclSpec::AQ_inline;
+ else if (Tok.getKind() == tok::kw_goto)
+ AQ = DeclSpec::AQ_goto;
+ else {
+ if (EndLoc.isValid())
+ DS.SetRangeEnd(EndLoc);
+ if (Tok.isNot(tok::l_paren)) {
+ SkipUntil(tok::r_paren, StopAtSemi);
+ Diag(Loc, diag::err_asm_qualifier_ignored);
+ return true;
+ }
+ return false;
+ }
+ if (DS.setGNUAsmQualifier(AQ, Loc))
+ Diag(Loc, diag::err_asm_duplicate_qual) << DeclSpec::getSpecifierName(AQ);
+ EndLoc = ConsumeToken();
+ }
+ return false;
+}
+
/// ParseAsmStatement - Parse a GNU extended asm statement.
/// asm-statement:
/// gnu-asm-statement
/// ms-asm-statement
///
/// [GNU] gnu-asm-statement:
-/// 'asm' type-qualifier[opt] '(' asm-argument ')' ';'
+/// 'asm' asm-qualifier-list[opt] '(' asm-argument ')' ';'
///
/// [GNU] asm-argument:
/// asm-string-literal
@@ -713,33 +730,9 @@
}
DeclSpec DS(AttrFactory);
- SourceLocation Loc = Tok.getLocation();
- ParseTypeQualifierListOpt(DS, AR_VendorAttributesParsed);
-
- // GNU asms accept, but warn, about type-qualifiers other than volatile.
- if (DS.getTypeQualifiers() & DeclSpec::TQ_const)
- Diag(Loc, diag::warn_asm_qualifier_ignored) << "const";
- if (DS.getTypeQualifiers() & DeclSpec::TQ_restrict)
- Diag(Loc, diag::warn_asm_qualifier_ignored) << "restrict";
- // FIXME: Once GCC supports _Atomic, check whether it permits it here.
- if (DS.getTypeQualifiers() & DeclSpec::TQ_atomic)
- Diag(Loc, diag::warn_asm_qualifier_ignored) << "_Atomic";
-
- // Remember if this was a volatile asm.
- bool isVolatile = DS.getTypeQualifiers() & DeclSpec::TQ_volatile;
- // Remember if this was a goto asm.
- bool isGotoAsm = false;
-
- if (Tok.is(tok::kw_goto)) {
- isGotoAsm = true;
- ConsumeToken();
- }
-
- if (Tok.isNot(tok::l_paren)) {
- Diag(Tok, diag::err_expected_lparen_after) << "asm";
- SkipUntil(tok::r_paren, StopAtSemi);
+ if(ParseGNUAsmQualifierListOpt(DS))
return StmtError();
- }
+
BalancedDelimiterTracker T(*this, tok::l_paren);
T.consumeOpen();
@@ -750,7 +743,7 @@
if (!(getLangOpts().GNUAsm || AsmString.isInvalid())) {
const auto *SL = cast<StringLiteral>(AsmString.get());
if (!SL->getString().trim().empty())
- Diag(Loc, diag::err_gnu_inline_asm_disabled);
+ Diag(Tok.getLocation(), diag::err_gnu_inline_asm_disabled);
}
if (AsmString.isInvalid()) {
@@ -764,6 +757,7 @@
ExprVector Exprs;
ExprVector Clobbers;
+ bool isVolatile = DS.getGNUAsmQualifiers() & DeclSpec::AQ_volatile;
if (Tok.is(tok::r_paren)) {
// We have a simple asm expression like 'asm("foo")'.
T.consumeClose();
@@ -829,6 +823,7 @@
}
}
}
+ bool isGotoAsm = DS.getGNUAsmQualifiers() & DeclSpec::AQ_goto;
if (!isGotoAsm && (Tok.isNot(tok::r_paren) || AteExtraColon)) {
Diag(Tok, diag::err_expected) << tok::r_paren;
SkipUntil(tok::r_paren, StopAtSemi);
Index: clang/include/clang/Sema/DeclSpec.h
===================================================================
--- clang/include/clang/Sema/DeclSpec.h
+++ clang/include/clang/Sema/DeclSpec.h
@@ -321,6 +321,14 @@
TQ_atomic = 16
};
+ // GNU asm qualifiers
+ enum AQ {
+ AQ_unspecified = 0,
+ AQ_volatile = 1,
+ AQ_inline = 2,
+ AQ_goto = 4,
+ };
+
/// ParsedSpecifiers - Flags to query which specifiers were applied. This is
/// returned by getParsedSpecifiers.
enum ParsedSpecifiers {
@@ -366,6 +374,9 @@
// constexpr-specifier
unsigned ConstexprSpecifier : 2;
+ // GNU asm specifier
+ unsigned GNUAsmQualifiers : 3; // Bitwise OR of AQ.
+
union {
UnionParsedType TypeRep;
Decl *DeclRep;
@@ -436,10 +447,10 @@
TypeSpecType(TST_unspecified), TypeAltiVecVector(false),
TypeAltiVecPixel(false), TypeAltiVecBool(false), TypeSpecOwned(false),
TypeSpecPipe(false), TypeSpecSat(false), ConstrainedAuto(false),
- TypeQualifiers(TQ_unspecified),
- FS_inline_specified(false), FS_forceinline_specified(false),
- FS_virtual_specified(false), FS_noreturn_specified(false),
- Friend_specified(false), ConstexprSpecifier(CSK_unspecified),
+ TypeQualifiers(TQ_unspecified), FS_inline_specified(false),
+ FS_forceinline_specified(false), FS_virtual_specified(false),
+ FS_noreturn_specified(false), Friend_specified(false),
+ ConstexprSpecifier(CSK_unspecified), GNUAsmQualifiers(AQ_unspecified),
FS_explicit_specifier(), Attrs(attrFactory), writtenBS(),
ObjCQualifiers(nullptr) {}
@@ -543,6 +554,7 @@
static const char *getSpecifierName(DeclSpec::SCS S);
static const char *getSpecifierName(DeclSpec::TSCS S);
static const char *getSpecifierName(ConstexprSpecKind C);
+ static const char *getSpecifierName(DeclSpec::AQ A);
// type-qualifiers
@@ -566,6 +578,9 @@
TQ_pipeLoc = SourceLocation();
}
+ /// getGNUAsmQualifiers - Return a set of AQs.
+ unsigned getGNUAsmQualifiers() const { return GNUAsmQualifiers; }
+
// function-specifier
bool isInlineSpecified() const {
return FS_inline_specified | FS_forceinline_specified;
@@ -718,6 +733,8 @@
bool SetTypeQual(TQ T, SourceLocation Loc, const char *&PrevSpec,
unsigned &DiagID, const LangOptions &Lang);
+ bool setGNUAsmQualifier(AQ A, SourceLocation Loc);
+
bool setFunctionSpecInline(SourceLocation Loc, const char *&PrevSpec,
unsigned &DiagID);
bool setFunctionSpecForceInline(SourceLocation Loc, const char *&PrevSpec,
Index: clang/include/clang/Parse/Parser.h
===================================================================
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -2762,6 +2762,7 @@
DeclSpec &DS, unsigned AttrReqs = AR_AllAttributesParsed,
bool AtomicAllowed = true, bool IdentifierRequired = false,
Optional<llvm::function_ref<void()>> CodeCompletionHandler = None);
+ bool ParseGNUAsmQualifierListOpt(DeclSpec &DS);
void ParseDirectDeclarator(Declarator &D);
void ParseDecompositionDeclarator(Declarator &D);
void ParseParenDeclarator(Declarator &D);
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -12,11 +12,10 @@
let Component = "Parse" in {
-def warn_asm_qualifier_ignored : Warning<
- "ignored %0 qualifier on asm">, CatInlineAsm, InGroup<ASMIgnoredQualifier>;
-def warn_file_asm_volatile : Warning<
- "meaningless 'volatile' on asm outside function">, CatInlineAsm,
- InGroup<ASMIgnoredQualifier>;
+def err_file_asm_volatile : Error<
+ "meaningless '%0' on asm outside function">, CatInlineAsm;
+def err_asm_qualifier_ignored : Error<
+ "expected 'volatile', 'inline', 'goto', or '('">, CatInlineAsm;
let CategoryName = "Inline Assembly Issue" in {
def err_asm_empty : Error<"__asm used with no assembly instructions">;
@@ -29,6 +28,7 @@
"GNU-style inline assembly is disabled">;
def err_asm_goto_cannot_have_output : Error<
"'asm goto' cannot have output constraints">;
+def err_asm_duplicate_qual : Error<"duplicate asm qualifier '%0'">;
}
let CategoryName = "Parse Issue" in {
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1084,9 +1084,8 @@
// Inline ASM warnings.
def ASMOperandWidths : DiagGroup<"asm-operand-widths">;
-def ASMIgnoredQualifier : DiagGroup<"asm-ignored-qualifier">;
def ASM : DiagGroup<"asm", [
- ASMOperandWidths, ASMIgnoredQualifier
+ ASMOperandWidths
]>;
// OpenMP warnings.
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -84,6 +84,11 @@
Modified Compiler Flags
-----------------------
+- -Wasm-ignored-qualifier (ex. `asm const ("")`) has been removed and replaced
+ with an error (this matches a recent change in GCC-9).
+- -Wasm-file-asm-volatile (ex. `asm volatile ("")` at global scope) has been
+ removed and replaced with an error (this matches GCC's behavior).
+
New Pragmas in Clang
--------------------
@@ -104,6 +109,9 @@
- The default C language standard used when `-std=` is not specified has been
upgraded from gnu11 to gnu17.
+- Clang now supports the GNU C extension `asm inline`; it won't do anything
+ *yet*, but it will be parsed.
+
- ...
C++ Language Changes in Clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
