erichkeane added inline comments.
================
Comment at: clang/docs/ReleaseNotes.rst:107
+- Clang now supports the _BitInt(N) family of bit-precise integer types from
+ C23. This type was previously exposed as _ExtInt(N), which is still supported
----------------
This section should probably mention the ABI break. It shouldn't be a surprise
for people upgrading.
================
Comment at: clang/docs/ReleaseNotes.rst:108
+- Clang now supports the _BitInt(N) family of bit-precise integer types from
+ C23. This type was previously exposed as _ExtInt(N), which is still supported
+ but is now deprecated in favor of the standard type. _BitInt(N) and
----------------
I would think it would be a better idea to not claim that `_ExtInt` is still
supported , but perhaps just say that it is now an alias for the 'new type'. I
realize it is a little pedantic, but `_ExtInt` is no longer a type in our type
system. I think this will make the diagnostics more understandable.
================
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">,
----------------
should this be warn_cpre2x here, just to be more consistent with the 'group'?
================
Comment at: clang/include/clang/Basic/TargetInfo.h:581
/// limitation is put into place for ABI reasons.
- virtual bool hasExtIntType() const {
+ /// FIXME: _BitInt is a required type in C23, so there's not much utility in
+ /// asking whether the target supported it or not; I think this should be
----------------
Concur on the fixme. I would expect after this lands that an llvm-dev
discussion happen to do this alert, and have us remove this pretty quickly (a
release or two?)
================
Comment at: clang/lib/AST/Type.cpp:2021
+ return IT->isSigned();
+ if (const auto *IT = dyn_cast<DependentBitIntType>(CanonicalType))
return IT->isSigned();
----------------
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.
================
Comment at: clang/lib/Parse/ParseDecl.cpp:3873
DiagID, Policy);
- break;
- case tok::kw__ExtInt: {
+ break;
+ case tok::kw__ExtInt:
----------------
Is this extra trailing WS?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108643/new/
https://reviews.llvm.org/D108643
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
