[clang] 2c65860 - [clang] Remove fixed point arithmetic error (#71884)

via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 13 12:34:06 PST 2023


Author: PiJoules
Date: 2023-11-13T12:31:49-08:00
New Revision: 2c65860667e202e182264d5c6dfe4c2961542f7d

URL: https://github.com/llvm/llvm-project/commit/2c65860667e202e182264d5c6dfe4c2961542f7d
DIFF: https://github.com/llvm/llvm-project/commit/2c65860667e202e182264d5c6dfe4c2961542f7d.diff

LOG: [clang] Remove fixed point arithmetic error (#71884)

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.

Added: 
    clang/test/Frontend/fixed_point_as_variables.c

Modified: 
    clang/include/clang/Basic/DiagnosticCommonKinds.td
    clang/include/clang/Basic/TokenKinds.def
    clang/lib/Basic/IdentifierTable.cpp
    clang/lib/Parse/ParseDecl.cpp
    clang/test/Frontend/fixed_point_not_enabled.c

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index 8084a4ce0d1751b..906ca6b8e47c50e 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 6cb4b3f250c4032..82a503d01068d53 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)
@@ -424,9 +426,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}}


        


More information about the cfe-commits mailing list