aaron.ballman added a comment.
These changes will also need a release note at some point.
================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:662-665
+def err_type_definition_cannot_be_modified : Error<
+ "%0 type definition cannot be modified inside a scope containing "
+ "'#pragma clang fp eval_method(%1)' when a command line "
+ "option 'ffp-eval-method=%2' is used">;
----------------
I think we should add some documentation as to why this error happens. Naively,
users expect the pragma to override the command line (the command line is the
"default" and the pragma overrides that in the cases the default is wrong), so
I'd imagine a user getting this error would ask, "But why?"
================
Comment at: clang/lib/Parse/ParseStmt.cpp:1122-1123
+ return "source";
+ default:
+ llvm_unreachable("unexpected eval method value");
+ }
----------------
Does something assert that we never try to call this with `FEM_Indeterminable`?
================
Comment at: clang/lib/Parse/ParseStmt.cpp:1198-1210
+ if (Tok.is(tok::identifier)) {
+ if (Tok.getIdentifierInfo()->getName().str() == "float_t" ||
+ Tok.getIdentifierInfo()->getName().str() == "double_t") {
+ if (getLangOpts().getFPEvalMethod() !=
+ LangOptions::FPEvalMethodKind::FEM_UnsetOnCommandLine &&
+ PP.getLastFPEvalPragmaLocation().isValid() &&
+ PP.getCurrentFPEvalMethod() != getLangOpts().getFPEvalMethod())
----------------
This is not correct -- it's only going to catch the case where the user
literally writes `float_t` as the first token in a statement, so it's going to
miss `const float_t t` and `typedef float_t frobble; frobble t;` etc. Also,
this seems like it will impact compile time overhead because that's going to be
performing a lot of useless string comparisons.
This cannot be handled at the level of the parser, it needs to be handled
within Sema. I think you'll want it to live somewhere around
`GetFullTypeForDeclarator()` in SemaType.cpp.
There are other situations that should be tested:
```
char buffer[sizeof(float_t)]; // This seems like something we'd want to prevent?
typedef float_t foo; // This seems like something that's harmless to accept?
using quux = float_t; // Same here as above
foo bar; // This, however, is a problem
extern float_t blah(); // This also seems bad
_Generic(1, float_t : 0); // Maybe a bad thing?
auto lam = [](float_t f) { return f; }; // Bad?
float f = (float_t)12; // What about this? Note, the float_t is only used in
the cast...
```
================
Comment at: clang/test/CodeGen/abi-check-1.c:1-2
+// RUN: %clang_cc1 -isystem %S/Inputs -triple x86_64-linux-gnu \
+// RUN: -emit-llvm -o - %s | FileCheck %s
+
----------------
Why are these codegen tests when the diagnostic (should be) in Sema?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146148/new/
https://reviews.llvm.org/D146148
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
