NoQ added a comment.

Thanks!! Here's the last couple of nits that i have.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:83-93
+static QualType getRecordType(QualType Ty) {
+  Ty = Ty.getCanonicalType();
+
+  if (Ty->isPointerType())
+    return getRecordType(Ty->getPointeeType());
 
+  if (Ty->isReferenceType())
----------------
NoQ wrote:
> Most of the time we should know exactly how many pointer/reference types we 
> need to unwrap. I really prefer we hard-assert this knowledge instead of 
> blindly unwrapping as many pointer/reference types as we want. Because every 
> time we have an unexpected number of unwraps it's an indication that 
> something went horribly wrong. So it's good to have the extra sanity checking.
I think this one is still forgotten. Maybe a FIXME?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:96-100
+static std::string getObjectName(const Expr *E, CheckerContext &C) {
+  return Lexer::getSourceText(
+      CharSourceRange::getTokenRange(E->getSourceRange()), 
C.getSourceManager(),
+      C.getASTContext().getLangOpts());
+}
----------------
I'm worried that this may occasionally get too long and/or contain line breaks. 
Let's do the usual pattern matching instead: say 'X' for a `DeclRefExpr` to 
'X', say 'field Y' for a `MemberExpr` made with field 'Y', say "the object" in 
other cases.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66325/new/

https://reviews.llvm.org/D66325



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

Reply via email to