[clang] [clang] Remove fixed point arithmetic error (PR #71884)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 9 16:58:47 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: None (PiJoules)
<details>
<summary>Changes</summary>
Prior to this, clang would always report
```
compile with '-ffixed-point' to enable fixed point types
```
whenever it sees `_Accum`, `_Fract`, or `_Sat` when fixed point arithmetic is not enabled. This can break existing code that uses these as variable names and doesn't use fixed point arithmetic like in some microsoft headers
(https://github.com/llvm/llvm-project/pull/67750#issuecomment-1775264907).
Fixed point should not raise this error for these cases, so this removes the error altogether and defaults to the usual error clang gives where it can see these keywords as either unknown types or regular variables.
---
Full diff: https://github.com/llvm/llvm-project/pull/71884.diff
6 Files Affected:
- (modified) clang/include/clang/Basic/DiagnosticCommonKinds.td (-2)
- (modified) clang/include/clang/Basic/TokenKinds.def (+5-3)
- (modified) clang/lib/Basic/IdentifierTable.cpp (+4-1)
- (modified) clang/lib/Parse/ParseDecl.cpp (+14-28)
- (added) clang/test/Frontend/fixed_point_as_variables.c (+11)
- (modified) clang/test/Frontend/fixed_point_not_enabled.c (+8-12)
``````````diff
diff --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index 9f0ccd255a32148..1b6b030c86c839e 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -260,8 +260,6 @@ def ext_clang_diagnose_if : Extension<"'diagnose_if' is a clang extension">,
InGroup<GccCompat>;
def err_too_large_for_fixed_point : Error<
"this value is too large for this fixed point type">;
-def err_fixed_point_not_enabled : Error<"compile with "
- "'-ffixed-point' to enable fixed point types">;
def err_unimplemented_conversion_with_fixed_point_type : Error<
"conversion between fixed point and %0 is not yet supported">;
diff --git a/clang/include/clang/Basic/TokenKinds.def b/clang/include/clang/Basic/TokenKinds.def
index 3ce317d318f9bb6..b62b2e05fe7dbd3 100644
--- a/clang/include/clang/Basic/TokenKinds.def
+++ b/clang/include/clang/Basic/TokenKinds.def
@@ -284,6 +284,8 @@ PUNCTUATOR(caretcaret, "^^")
// HALFSUPPORT - This is a keyword if 'half' is a built-in type
// WCHARSUPPORT - This is a keyword if 'wchar_t' is a built-in type
// CHAR8SUPPORT - This is a keyword if 'char8_t' is a built-in type
+// KEYFIXEDPOINT - This is a keyword according to the N1169 fixed point
+// extension.
//
KEYWORD(auto , KEYALL)
KEYWORD(break , KEYALL)
@@ -423,9 +425,9 @@ C23_KEYWORD(typeof , KEYGNU)
C23_KEYWORD(typeof_unqual , 0)
// ISO/IEC JTC1 SC22 WG14 N1169 Extension
-KEYWORD(_Accum , KEYNOCXX)
-KEYWORD(_Fract , KEYNOCXX)
-KEYWORD(_Sat , KEYNOCXX)
+KEYWORD(_Accum , KEYFIXEDPOINT)
+KEYWORD(_Fract , KEYFIXEDPOINT)
+KEYWORD(_Sat , KEYFIXEDPOINT)
// GNU Extensions (in impl-reserved namespace)
KEYWORD(_Decimal32 , KEYALL)
diff --git a/clang/lib/Basic/IdentifierTable.cpp b/clang/lib/Basic/IdentifierTable.cpp
index c4c5a6eeced2832..38150d1640d709d 100644
--- a/clang/lib/Basic/IdentifierTable.cpp
+++ b/clang/lib/Basic/IdentifierTable.cpp
@@ -108,7 +108,8 @@ namespace {
KEYSYCL = 0x800000,
KEYCUDA = 0x1000000,
KEYHLSL = 0x2000000,
- KEYMAX = KEYHLSL, // The maximum key
+ KEYFIXEDPOINT = 0x4000000,
+ KEYMAX = KEYFIXEDPOINT, // The maximum key
KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20,
KEYALL = (KEYMAX | (KEYMAX-1)) & ~KEYNOMS18 &
~KEYNOOPENCL // KEYNOMS18 and KEYNOOPENCL are used to exclude.
@@ -210,6 +211,8 @@ static KeywordStatus getKeywordStatusHelper(const LangOptions &LangOpts,
case KEYNOMS18:
// The disable behavior for this is handled in getKeywordStatus.
return KS_Unknown;
+ case KEYFIXEDPOINT:
+ return LangOpts.FixedPoint ? KS_Enabled : KS_Disabled;
default:
llvm_unreachable("Unknown KeywordStatus flag");
}
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 78c3ab72979a007..06f2c8798b049f8 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -3288,17 +3288,6 @@ Parser::DiagnoseMissingSemiAfterTagDefinition(DeclSpec &DS, AccessSpecifier AS,
return false;
}
-// Choose the apprpriate diagnostic error for why fixed point types are
-// disabled, set the previous specifier, and mark as invalid.
-static void SetupFixedPointError(const LangOptions &LangOpts,
- const char *&PrevSpec, unsigned &DiagID,
- bool &isInvalid) {
- assert(!LangOpts.FixedPoint);
- DiagID = diag::err_fixed_point_not_enabled;
- PrevSpec = ""; // Not used by diagnostic
- isInvalid = true;
-}
-
/// ParseDeclarationSpecifiers
/// declaration-specifiers: [C99 6.7]
/// storage-class-specifier declaration-specifiers[opt]
@@ -4275,27 +4264,24 @@ void Parser::ParseDeclarationSpecifiers(
DiagID, Policy);
break;
case tok::kw__Accum:
- if (!getLangOpts().FixedPoint) {
- SetupFixedPointError(getLangOpts(), PrevSpec, DiagID, isInvalid);
- } else {
- isInvalid = DS.SetTypeSpecType(DeclSpec::TST_accum, Loc, PrevSpec,
- DiagID, Policy);
- }
+ assert(getLangOpts().FixedPoint &&
+ "This keyword is only used when fixed point types are enabled "
+ "with `-ffixed-point`");
+ isInvalid = DS.SetTypeSpecType(DeclSpec::TST_accum, Loc, PrevSpec, DiagID,
+ Policy);
break;
case tok::kw__Fract:
- if (!getLangOpts().FixedPoint) {
- SetupFixedPointError(getLangOpts(), PrevSpec, DiagID, isInvalid);
- } else {
- isInvalid = DS.SetTypeSpecType(DeclSpec::TST_fract, Loc, PrevSpec,
- DiagID, Policy);
- }
+ assert(getLangOpts().FixedPoint &&
+ "This keyword is only used when fixed point types are enabled "
+ "with `-ffixed-point`");
+ isInvalid = DS.SetTypeSpecType(DeclSpec::TST_fract, Loc, PrevSpec, DiagID,
+ Policy);
break;
case tok::kw__Sat:
- if (!getLangOpts().FixedPoint) {
- SetupFixedPointError(getLangOpts(), PrevSpec, DiagID, isInvalid);
- } else {
- isInvalid = DS.SetTypeSpecSat(Loc, PrevSpec, DiagID);
- }
+ assert(getLangOpts().FixedPoint &&
+ "This keyword is only used when fixed point types are enabled "
+ "with `-ffixed-point`");
+ isInvalid = DS.SetTypeSpecSat(Loc, PrevSpec, DiagID);
break;
case tok::kw___float128:
isInvalid = DS.SetTypeSpecType(DeclSpec::TST_float128, Loc, PrevSpec,
diff --git a/clang/test/Frontend/fixed_point_as_variables.c b/clang/test/Frontend/fixed_point_as_variables.c
new file mode 100644
index 000000000000000..1eb969a355d3b72
--- /dev/null
+++ b/clang/test/Frontend/fixed_point_as_variables.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -x c -verify %s
+// RUN: %clang_cc1 -x c -verify %s -ffixed-point -DFIXED_POINT=1
+
+int _Accum;
+
+#ifdef FIXED_POINT
+// expected-error at 4{{cannot combine with previous 'int' declaration specifier}}
+// expected-warning at 4{{declaration does not declare anything}}
+#else
+// expected-no-diagnostics
+#endif
diff --git a/clang/test/Frontend/fixed_point_not_enabled.c b/clang/test/Frontend/fixed_point_not_enabled.c
index a1a60c5a6fa8007..c381f1c8f1835a1 100644
--- a/clang/test/Frontend/fixed_point_not_enabled.c
+++ b/clang/test/Frontend/fixed_point_not_enabled.c
@@ -1,18 +1,14 @@
// RUN: %clang_cc1 -x c -verify %s
// Primary fixed point types
-signed short _Accum s_short_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
-signed _Accum s_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
-signed long _Accum s_long_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
-unsigned short _Accum u_short_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
-unsigned _Accum u_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
-unsigned long _Accum u_long_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
-
-// Aliased fixed point types
-short _Accum short_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
-_Accum accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
- // expected-error at -1{{type specifier missing, defaults to 'int'}}
-long _Accum long_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+// Without `-ffixed-point`, these keywords are now treated as typedef'd types or identifiers.
+_Accum accum; // expected-error{{unknown type name '_Accum'}}
+_Fract fract; // expected-error{{unknown type name '_Fract'}}
+_Sat _Accum sat_accum; // expected-error{{unknown type name '_Sat'}}
+ // expected-error at -1{{expected ';' after top level declarator}}
+signed _Accum signed_accum; // expected-error{{expected ';' after top level declarator}}
+signed _Fract signed_fract; // expected-error{{expected ';' after top level declarator}}
+signed _Sat _Accum signed_sat_accum; // expected-error{{expected ';' after top level declarator}}
// Cannot use fixed point suffixes
int accum_int = 10k; // expected-error{{invalid suffix 'k' on integer constant}}
``````````
</details>
https://github.com/llvm/llvm-project/pull/71884
More information about the cfe-commits
mailing list