Skip to content

Commit

Permalink
[clang] Remove fixed point arithmetic error (#71884)
Browse files Browse the repository at this point in the history
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

(#67750 (comment)).

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.
  • Loading branch information
PiJoules committed Nov 13, 2023
1 parent 915e092 commit 2c65860
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 46 deletions.
2 changes: 0 additions & 2 deletions clang/include/clang/Basic/DiagnosticCommonKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -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">;

Expand Down
8 changes: 5 additions & 3 deletions clang/include/clang/Basic/TokenKinds.def
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion clang/lib/Basic/IdentifierTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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");
}
Expand Down
42 changes: 14 additions & 28 deletions clang/lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 11 additions & 0 deletions clang/test/Frontend/fixed_point_as_variables.c
Original file line number Diff line number Diff line change
@@ -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@4{{cannot combine with previous 'int' declaration specifier}}
// expected-warning@4{{declaration does not declare anything}}
#else
// expected-no-diagnostics
#endif
20 changes: 8 additions & 12 deletions clang/test/Frontend/fixed_point_not_enabled.c
Original file line number Diff line number Diff line change
@@ -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@-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@-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}}
Expand Down

0 comments on commit 2c65860

Please sign in to comment.