[PATCH] D15324: [Sema] Emit warnings when comparing result of a function with `returns_nonnull` to null

George Burgess IV via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 8 11:35:06 PST 2015


george.burgess.iv added inline comments.

================
Comment at: lib/Sema/SemaChecking.cpp:7674
@@ +7673,3 @@
+                                : diag::warn_cast_nonnull_to_bool;
+    Diag(E->getExprLoc(), DiagID) << int(IsParam) << S.str()
+      << E->getSourceRange() << Range << IsEqual;
----------------
aaron.ballman wrote:
> No need to cast IsParam to int; the diagnostic builder already does the right thing here.
Neat. Thanks!

================
Comment at: lib/Sema/SemaChecking.cpp:7682
@@ +7681,3 @@
+      if (Callee->hasAttr<ReturnsNonNullAttr>())
+        return ComplainAboutNonnullParamOrCall(false);
+
----------------
aaron.ballman wrote:
> I think this should be:
> ```
> if (Callee->hasAttr<ReturnsNonNullAttr>() &&
>     ComplainAboutNonnullParamOrCall(false))
>   return;
> ```
> Otherwise, we skip out on the rest of the checking in the presence of ReturnsNonNullAttr.
The function this is in returns `void`, not `bool`. I've separated the return from the function call in order to hopefully make things a bit more clear. :)

================
Comment at: lib/Sema/SemaChecking.cpp:7716
@@ -7711,1 +7715,3 @@
+              (AttrNonNull[i] || PV->hasAttr<NonNullAttr>()))
+            return ComplainAboutNonnullParamOrCall(true);
       }
----------------
aaron.ballman wrote:
> Same here as above. The return should only be on failure.
Separated into two lines, like above.


http://reviews.llvm.org/D15324





More information about the cfe-commits mailing list