jordan_rose added a subscriber: dergachev.a.
jordan_rose added a comment.
Sorry for the delay. I think this is mostly good, but I do still have a concern
about the diagnostics change.
================
Comment at: lib/Sema/SemaExpr.cpp:7117
+ if (E && S.checkNonNullExpr(E))
+ return NullabilityKind::Nullable;
+
----------------
ahatanak wrote:
> jordan_rose wrote:
> > This isn't quite correct, unfortunately. `(_Nonnull id)nil` should be
> > considered non-nullable, since it's the canonical way to avoid all these
> > warnings. It might just be good enough to move this check below the
> > `getNullability` one, though.
> Sema::CheckNonNullExpr checks the nullability of the type of the expression
> first and returns false if there is a cast to `_Nonnull`.
Hm, okay, I see you have a test. Sorry for the noise.
================
Comment at: lib/Sema/SemaExpr.cpp:7162
+ bool IsBin, Expr *LHSExpr,
+ Expr *RHSExpr, ASTContext &Ctx) {
if (!ResTy->isAnyPointerType())
----------------
Nitpick: you could use `const` on the Exprs here.
================
Comment at: lib/Sema/SemaExpr.cpp:11103
+ if (const auto *DeclRef = dyn_cast<DeclRefExpr>(LHSExpr))
+ checkNullConstantToNonNull(DeclRef->getType(), RHS.get());
----------------
This could come later, but what about struct members or ObjC properties or ObjC
subscripts? Seems like you could just check the type of the LHS.
================
Comment at: test/Analysis/nullability_nullonly.mm:103
void testObjCARCExplicitZeroInitialization() {
- TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning
{{nil assigned to a pointer which is expected to have non-null value}}
+ TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning
{{nil assigned to a pointer which is expected to have non-null value}}
expected-warning{{implicitly casting a null constant to non-nullable pointer
type 'TestObject * _Nonnull __strong'}}
}
----------------
@dergachev.a, what do you think here? Is it okay that the analyzer diagnoses
this in addition to the new warning?
================
Comment at: test/Sema/conditional-expr.c:20
vp = 0 ? (double *)0 : (void *)0;
- ip = 0 ? (double *)0 : (void *)0; // expected-warning {{incompatible pointer
types assigning to 'int *' from 'double *'}}
+ ip = 0 ? (double *)0 : (void *)0; // expected-warning {{incompatible pointer
types assigning to 'int *' from 'double * _Nullable'}}
----------------
This seems like an unfortunate change to make, since most people do not bother
with nullability.
Repository:
rC Clang
https://reviews.llvm.org/D22391
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
