aaron.ballman added inline comments.
================
Comment at: clang/include/clang/AST/PrettyPrinter.h:68
SuppressDefaultTemplateArgs(true), Bool(LO.Bool),
- Nullptr(LO.CPlusPlus11), Restrict(LO.C99), Alignof(LO.CPlusPlus11),
+ Nullptr(LO.CPlusPlus11), NullptrTypeInNamespace(LO.CPlusPlus),
+ Restrict(LO.C99), Alignof(LO.CPlusPlus11),
----------------
erichkeane wrote:
> Does Nullptr here not have to change? Shouldn't we care to use the "nullptr"
> in C mode instead of '0' now?
Yes it should, but it's an NFC change. The only thing using `Nullptr` is the
`APValue` pretty printer and I can't find a way to exercise that code path in
C. That said, once we add support for `constexpr` in C, I believe we will be
able to get into this path so we might as well handle it now.
If anyone can think of a test case that attempts to print a diagnostic on a
pointer typed APValue in C, I'm happy to add it.
================
Comment at: clang/include/clang/AST/PrettyPrinter.h:201
+ /// Whether 'nullptr_t' is in namespace 'std' or not.
+ unsigned NullptrTypeInNamespace;
+
----------------
erichkeane wrote:
> Is there a reason this isn't a bitfield as well?
Nope, great catch! Think-o on my part. :-)
================
Comment at: clang/include/clang/Basic/TokenKinds.def:393
CXX11_KEYWORD(noexcept , 0)
-CXX11_KEYWORD(nullptr , 0)
+CXX11_KEYWORD(nullptr , KEYC2X)
CXX11_KEYWORD(static_assert , KEYMSCOMPAT|KEYC2X)
----------------
erichkeane wrote:
> Its a shame we have to do this... the CXX11_KEYWORD (and presumably the
> reverse) is used for 'future' diagnostics IIRC. So not having this as a
> C2X_KEYWORD means we lose the 'future keyword' diagnostics, right?
Nope, we do the right thing now after Usman's changes. I'll add a test to
demonstrate so we have better coverage.
================
Comment at: clang/lib/Sema/SemaCast.cpp:2999
+ Self.Diag(SrcExpr.get()->getExprLoc(), diag::err_nullptr_cast)
+ << 0 /*nullptr to type*/ << DestType;
+ SrcExpr = ExprError();
----------------
shafik wrote:
> Curious why put the comment after? When we use `butprone-argument-comment`
> for function parameters we put them before.
No real reason aside from muscle memory. I'll switch it up.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:8730
+ // result also has that type.
+ if (LHSTy->isNullPtrType() && Context.hasSameType(LHSTy, RHSTy))
+ return ResTy;
----------------
shafik wrote:
> erichkeane wrote:
> > what does this do with the GNU ternary-thing?
> Does this cover if one is a pointer with a null pointer value?
It behaves as expected; I've added additional test coverage to demonstrate
that, good call!
================
Comment at: clang/lib/Sema/SemaExpr.cpp:12587
- // Comparison of Objective-C pointers and block pointers against nullptr_t.
- // These aren't covered by the composite pointer type rules.
- if (!IsOrdered && RHSType->isNullPtrType() &&
- (LHSType->isObjCObjectPointerType() || LHSType->isBlockPointerType()))
{
+ // C++ [expr.eq]p4:
+ // Two operands of type std::nullptr_t or one operand of type
----------------
shafik wrote:
> This was in the `if (getLangOpts().CPlusPlus)` block previously, did you mean
> to move it out?
Yup, moving it out is intentional. This now covers C2x 6.5.9p5 as well.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:12620
+
+ // Comparison of Objective-C pointers and block pointers against nullptr_t.
+ // These aren't covered by the composite pointer type rules.
----------------
shafik wrote:
> This was in the `if (getLangOpts().CPlusPlus)` block previously, did you mean
> to move it out?
Yes -- this was handling Objective-C++ behavior and now that C has nullptr we
want it to cover Objective-C as well.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135099/new/
https://reviews.llvm.org/D135099
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
