[PATCH] D108643: Introduce _BitInt, deprecate _ExtInt

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1489
+  InGroup<DiagGroup<"bit-int-extension">>;
+def warn_prec2x_compat_bit_int : Warning<
+  "'_BitInt' is incompatible with C standards before C2x">,
----------------
erichkeane wrote:
> should this be warn_cpre2x here, just to be more consistent with the 'group'?
It turns out that other diagnostics don't put `pre` in the identifier. Going with `warn_c17_compat_bit_int` to be consistent with `warn_c17_compat_static_assert_no_message` which is in the same group.


================
Comment at: clang/lib/AST/Type.cpp:2021
+    return IT->isSigned();
+  if (const auto *IT = dyn_cast<DependentBitIntType>(CanonicalType))
     return IT->isSigned();
----------------
erichkeane wrote:
> Note to others: Aaron and I discussed this offline.  I don't think it is NECESSARY (as I don't think we really call these functions on dependent types), but I thought it was a good idea to be here for completeness.
+1 -- I could not find a new test case to write for this, but it seemed like an "obvious" oversight because the dependency doesn't matter for determining whether the type is signed or not.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:3873
                                      DiagID, Policy);
-      break;
-    case tok::kw__ExtInt: {
+      break;    
+    case tok::kw__ExtInt:
----------------
erichkeane wrote:
> Is this extra trailing WS?
*gasps* the nerve of some patch authors, right? ;-)


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

https://reviews.llvm.org/D108643