[PATCH] D65997: Add options rounding and exceptions to pragma fp

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/docs/LanguageExtensions.rst:3146
 
+The option ``rounding`` specifies rounding mode applied to floating point
+operations. It may take one of the values: ``tonearest``, ``downward``,
----------------
specifies rounding mode -> specifies the rounding mode


================
Comment at: clang/docs/LanguageExtensions.rst:3151
+value ``dynamic`` informs the compiler that it must not assume particular
+rounding mode. This option is experimental, compiler does not guarantee to
+process it in all cases.
----------------
The last sentence is a bit ambiguous because "this option" could mean "rounding modes in general" or it could mean "the dynamic rounding mode". I would reword it to:

`compiler does not guarantee to process it in all cases -> the compiler may ignore a dynamic rounding mode in some situations`

or

`compiler does not guarantee to process it in all cases -> the compiler may ignore an explicit rounding mode in some situations`





================
Comment at: clang/docs/LanguageExtensions.rst:3157
+is ``ignore``. Meaning of these values is same as for `constrained floating
+point intrinsics <http://llvm.org/docs/LangRef.html#id2115>`_. This option is
+experimental, compiler does not guarantee to process it in all cases.
----------------
Will this link remain stable?


================
Comment at: clang/docs/LanguageExtensions.rst:3158
+point intrinsics <http://llvm.org/docs/LangRef.html#id2115>`_. This option is
+experimental, compiler does not guarantee to process it in all cases.
+
----------------
Same issue here as above, reword similarly.


================
Comment at: clang/lib/Parse/ParsePragma.cpp:2670
   SmallVector<Token, 1> TokenList;
+  static const StringRef ExpectedOptionsText =
+      "'contract', 'rounding' or 'exceptions'";
----------------
Given that all of these strings are known at compile time, I would rather see use of a `%select` in the diagnostic.


================
Comment at: clang/lib/Parse/ParsePragma.cpp:2697
     PP.Lex(Tok);
+    StringRef ExpectedArgumentText;
+    switch (*FlagKind) {
----------------
Same here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65997/new/

https://reviews.llvm.org/D65997