Re: [PATCH] D19962: [scan-build] fix warnings emitted on Clang StaticAnalyzer code base

[Anna Zaks via cfe-commits]

zaks.anna added inline comments.


Comment at: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:686
@@ -685,3 +685,3 @@
 const Expr *ArgExpr = nullptr;
 if (Idx < Call.getNumArgs())
   ArgExpr = Call.getArgExpr(Idx);

This conditional will assert if:
assert(Arg < NumArgs && "Arg access out of range!");

So removing this conditional will assert in the same cases as the assert you 
are adding.


Comment at: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:696
@@ -695,1 +695,3 @@
 
+assert(ArgExpr && "cannot get the type of a NULL expression");
+

Asserting might not be the right thing to do here.


http://reviews.llvm.org/D19962



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D19962: [scan-build] fix warnings emitted on Clang StaticAnalyzer code base

[Apelete Seketeli via cfe-commits]

apelete updated this revision to Diff 56556.
apelete added a comment.

[scan-build] fix warnings emitted on Clang StaticAnalyzer code base

- lib/StaticAnalyzer/Core/PlistDiagnostics.cpp: factorize assert and check on 
'!Diags.empty' condition.


http://reviews.llvm.org/D19962

Files:
  lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

Index: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -294,9 +294,8 @@
   SmallVector Fids;
   const SourceManager* SM = nullptr;
 
-  if (!Diags.empty())
-SM = ()->path.front()->getLocation().getManager();
-
+  assert(!Diags.empty() && "cannot iterate through empty PathDiagnostic");
+  SM = ()->path.front()->getLocation().getManager();
 
   for (std::vector::iterator DI = Diags.begin(),
DE = Diags.end(); DI != DE; ++DI) {
Index: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -693,6 +693,8 @@
 !Param->getType()->isReferenceType())
   continue;
 
+assert(ArgExpr && "cannot get the type of a NULL expression");
+
 NullConstraint Nullness = getNullConstraint(*ArgSVal, State);
 
 Nullability RequiredNullability =


Index: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -294,9 +294,8 @@
   SmallVector Fids;
   const SourceManager* SM = nullptr;
 
-  if (!Diags.empty())
-SM = ()->path.front()->getLocation().getManager();
-
+  assert(!Diags.empty() && "cannot iterate through empty PathDiagnostic");
+  SM = ()->path.front()->getLocation().getManager();
 
   for (std::vector::iterator DI = Diags.begin(),
DE = Diags.end(); DI != DE; ++DI) {
Index: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -693,6 +693,8 @@
 !Param->getType()->isReferenceType())
   continue;
 
+assert(ArgExpr && "cannot get the type of a NULL expression");
+
 NullConstraint Nullness = getNullConstraint(*ArgSVal, State);
 
 Nullability RequiredNullability =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D19962: [scan-build] fix warnings emitted on Clang StaticAnalyzer code base

[Apelete Seketeli via cfe-commits]

apelete added inline comments.


Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:300
@@ -299,2 +299,3 @@
 
+  assert(SM && "SourceManager is NULL, cannot iterate through the 
diagnostics");
 

dblaikie wrote:
> This assertion seems to be equivalent to replacing the prior 'if' with an 
> assertion of the same condition, no? Is that correct? (& if it is, we should 
> just do that)
You're right, I overlooked the simplification.
Will fix in next revision.


http://reviews.llvm.org/D19962



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D19962: [scan-build] fix warnings emitted on Clang StaticAnalyzer code base

[David Blaikie via cfe-commits]

dblaikie added a subscriber: dblaikie.


Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:300
@@ -299,2 +299,3 @@
 
+  assert(SM && "SourceManager is NULL, cannot iterate through the 
diagnostics");
 

This assertion seems to be equivalent to replacing the prior 'if' with an 
assertion of the same condition, no? Is that correct? (& if it is, we should 
just do that)


http://reviews.llvm.org/D19962



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits