nickdesaulniers updated this revision to Diff 249254.
nickdesaulniers added a comment.
This revision is now accepted and ready to land.
- DRY up getting the Qualifier, and checking Tok.is
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/lib/Parse/ParseStmtAsm.cpp
clang/lib/Parse/Parser.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/Parse/Parser.cpp
===================================================================
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -1528,13 +1528,15 @@
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 (isGNUAsmQualifier(Tok)) {
+ // 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)
- << FixItHint::CreateRemoval(RemovalRange);
+ const auto AQ = GNUAsmQualifiers::getQualifierForTokenKind(Tok.getKind());
+ Diag(Tok, diag::err_global_asm_qualifier_ignored)
+ << GNUAsmQualifiers::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,14 @@
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.
+bool Parser::isGCCAsmStatement(const Token &TokAfterAsm) const {
+ return TokAfterAsm.is(tok::l_paren) || isGNUAsmQualifier(TokAfterAsm);
}
-// 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);
+bool Parser::isGNUAsmQualifier(const Token &TokAfterAsm) const {
+ return 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 +667,42 @@
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(GNUAsmQualifiers *AQ) {
+ while (1) {
+ const auto A = GNUAsmQualifiers::getQualifierForTokenKind(Tok.getKind());
+
+ if (A == GNUAsmQualifiers::AQ_unspecified) {
+ if (Tok.isNot(tok::l_paren)) {
+ SkipUntil(tok::r_paren, StopAtSemi);
+ Diag(Tok.getLocation(), diag::err_asm_qualifier_ignored);
+ return true;
+ }
+ return false;
+ }
+ if (AQ->setAsmQualifier(A))
+ Diag(Tok.getLocation(), diag::err_asm_duplicate_qual)
+ << GNUAsmQualifiers::getSpecifierName(A);
+ 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
@@ -712,34 +724,11 @@
return ParseMicrosoftAsmStatement(AsmLoc);
}
- 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);
+ GNUAsmQualifiers GAQ;
+ if (ParseGNUAsmQualifierListOpt(&GAQ))
return StmtError();
- }
+
BalancedDelimiterTracker T(*this, tok::l_paren);
T.consumeOpen();
@@ -767,11 +756,10 @@
if (Tok.is(tok::r_paren)) {
// We have a simple asm expression like 'asm("foo")'.
T.consumeClose();
- return Actions.ActOnGCCAsmStmt(AsmLoc, /*isSimple*/ true, isVolatile,
- /*NumOutputs*/ 0, /*NumInputs*/ 0, nullptr,
- Constraints, Exprs, AsmString.get(),
- Clobbers, /*NumLabels*/ 0,
- T.getCloseLocation());
+ return Actions.ActOnGCCAsmStmt(
+ AsmLoc, /*isSimple*/ true, GAQ.isVolatile(),
+ /*NumOutputs*/ 0, /*NumInputs*/ 0, nullptr, Constraints, Exprs,
+ AsmString.get(), Clobbers, /*NumLabels*/ 0, T.getCloseLocation());
}
// Parse Outputs, if present.
@@ -829,7 +817,7 @@
}
}
}
- if (!isGotoAsm && (Tok.isNot(tok::r_paren) || AteExtraColon)) {
+ if (!GAQ.isGoto() && (Tok.isNot(tok::r_paren) || AteExtraColon)) {
Diag(Tok, diag::err_expected) << tok::r_paren;
SkipUntil(tok::r_paren, StopAtSemi);
return StmtError();
@@ -862,16 +850,16 @@
if (!TryConsumeToken(tok::comma))
break;
}
- } else if (isGotoAsm) {
+ } else if (GAQ.isGoto()) {
Diag(Tok, diag::err_expected) << tok::colon;
SkipUntil(tok::r_paren, StopAtSemi);
return StmtError();
}
T.consumeClose();
- return Actions.ActOnGCCAsmStmt(
- AsmLoc, false, isVolatile, NumOutputs, NumInputs, Names.data(),
- Constraints, Exprs, AsmString.get(), Clobbers, NumLabels,
- T.getCloseLocation());
+ return Actions.ActOnGCCAsmStmt(AsmLoc, false, GAQ.isVolatile(), NumOutputs,
+ NumInputs, Names.data(), Constraints, Exprs,
+ AsmString.get(), Clobbers, NumLabels,
+ T.getCloseLocation());
}
/// ParseAsmOperands - Parse the asm-operands production as used by
@@ -942,3 +930,27 @@
return false;
}
}
+
+const char *Parser::GNUAsmQualifiers::getSpecifierName(const AQ AQ) {
+ switch (AQ) {
+ case AQ_volatile: return "volatile";
+ case AQ_inline: return "inline";
+ case AQ_goto: return "goto";
+ case AQ_unspecified:;
+ }
+ llvm_unreachable("Unkown GNUAsmQualifier");
+}
+Parser::GNUAsmQualifiers::AQ
+Parser::GNUAsmQualifiers::getQualifierForTokenKind(const tok::TokenKind K) {
+ switch (K) {
+ case tok::kw_volatile: return AQ_volatile;
+ case tok::kw_inline: return AQ_inline;
+ case tok::kw_goto: return AQ_goto;
+ default: return AQ_unspecified;
+ }
+}
+bool Parser::GNUAsmQualifiers::setAsmQualifier(const AQ AQ) {
+ bool IsDuplicate = Qualifiers & AQ;
+ Qualifiers |= AQ;
+ return IsDuplicate;
+}
Index: clang/include/clang/Parse/Parser.h
===================================================================
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -3201,6 +3201,27 @@
unsigned ArgumentIndex) override;
void CodeCompleteIncludedFile(llvm::StringRef Dir, bool IsAngled) override;
void CodeCompleteNaturalLanguage() override;
+
+ class GNUAsmQualifiers {
+ unsigned Qualifiers : 3; // Bitwise OR of AQ.
+ public:
+ enum AQ {
+ AQ_unspecified = 0,
+ AQ_volatile = 1,
+ AQ_inline = 2,
+ AQ_goto = 4,
+ };
+ GNUAsmQualifiers() : Qualifiers(0) {}
+ static const char *getSpecifierName(const AQ AQ);
+ static AQ getQualifierForTokenKind(const tok::TokenKind K);
+ bool setAsmQualifier(const AQ AQ);
+ inline bool isVolatile() const { return Qualifiers & AQ_volatile; };
+ inline bool isInline() const { return Qualifiers & AQ_inline; };
+ inline bool isGoto() const { return Qualifiers & AQ_goto; }
+ };
+ bool isGCCAsmStatement(const Token &TokAfterAsm) const;
+ bool isGNUAsmQualifier(const Token &TokAfterAsm) const;
+ bool ParseGNUAsmQualifierListOpt(GNUAsmQualifiers *AQ);
};
} // end namespace clang
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_asm_qualifier_ignored : Error<
+ "expected 'volatile', 'inline', 'goto', or '('">, CatInlineAsm;
+def err_global_asm_qualifier_ignored : Error<
+ "meaningless '%0' on asm outside function">, 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
@@ -1090,9 +1090,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
@@ -91,6 +91,13 @@
of a variable in a header file. In some cases, no specific translation unit
provides a definition of the variable. The previous behavior can be restored by
specifying ``-fcommon``.
+- -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).
+- Duplicate qualifiers on asm statements (ex. `asm volatile volatile ("")`) no
+ longer produces a warning via -Wduplicate-decl-specifier, but now an error
+ (this matches GCC's behavior).
New Pragmas in Clang
--------------------
@@ -111,6 +118,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
