[analyzer] Address code review for r179395

Mostly refactoring + handle the nested fields by printing the innermost field only.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179572 91177308-0d34-0410-b5e6-96231b3b80d8

3 files changed, 158 insertions, 19 deletions

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h b/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
index 7ae432e389..0d513f8326 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -169,7 +169,15 @@ public:
   /// \brief Print the region for use in diagnostics.
   virtual void printPretty(raw_ostream &os) const;
 
-  virtual void printPrettyNoQuotes(raw_ostream &os) const;
+  /// \brief Returns true if this region's textual representation can be used
+  /// as part of a larger expression.
+  virtual bool canPrintPrettyAsExpr() const;
+
+  /// \brief Print the region as expression.
+  ///
+  /// When this region represents a subexpression, the method is for printing
+  /// an expression containing it.
+  virtual void printPrettyAsExpr(raw_ostream &os) const;
 
   Kind getKind() const { return kind; }
 
@@ -878,7 +886,7 @@ public:
 
   bool canPrintPretty() const;
 
-  void printPrettyNoQuotes(raw_ostream &os) const;
+  void printPrettyAsExpr(raw_ostream &os) const;
 };
   
 /// CXXThisRegion - Represents the region for the implicit 'this' parameter
@@ -940,7 +948,8 @@ public:
 
   bool canPrintPretty() const;
   void printPretty(raw_ostream &os) const;
-  void printPrettyNoQuotes(raw_ostream &os) const;
+  bool canPrintPrettyAsExpr() const;
+  void printPrettyAsExpr(raw_ostream &os) const;
 };
 
 class ObjCIvarRegion : public DeclRegion {
@@ -957,7 +966,7 @@ public:
   QualType getValueType() const;
 
   bool canPrintPretty() const;
-  void printPrettyNoQuotes(raw_ostream &os) const;
+  void printPrettyAsExpr(raw_ostream &os) const;
 
   void dumpToStream(raw_ostream &os) const;
 
diff --git a/lib/StaticAnalyzer/Core/MemRegion.cpp b/lib/StaticAnalyzer/Core/MemRegion.cpp
index e244d31afa..32e7f7713b 100644
--- a/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -558,16 +558,20 @@ bool MemRegion::canPrintPretty() const {
   return false;
 }
 
+bool MemRegion::canPrintPrettyAsExpr() const {
+  return canPrintPretty();
+}
+
 void MemRegion::printPretty(raw_ostream &os) const {
   assert(canPrintPretty() && "This region cannot be printed pretty.");
   os << "'";
-  printPrettyNoQuotes(os);
+  printPrettyAsExpr(os);
   os << "'";
   return;
 }
 
-void MemRegion::printPrettyNoQuotes(raw_ostream &os) const {
-  assert(canPrintPretty() && "This region cannot be printed pretty.");
+void MemRegion::printPrettyAsExpr(raw_ostream &os) const {
+  llvm_unreachable("This region cannot be printed pretty.");
   return;
 }
 
@@ -575,7 +579,7 @@ bool VarRegion::canPrintPretty() const {
   return true;
 }
 
-void VarRegion::printPrettyNoQuotes(raw_ostream &os) const {
+void VarRegion::printPrettyAsExpr(raw_ostream &os) const {
   os << getDecl()->getName();
 }
 
@@ -583,7 +587,7 @@ bool ObjCIvarRegion::canPrintPretty() const {
   return true;
 }
 
-void ObjCIvarRegion::printPrettyNoQuotes(raw_ostream &os) const {
+void ObjCIvarRegion::printPrettyAsExpr(raw_ostream &os) const {
   os << getDecl()->getName();
 }
 
@@ -591,26 +595,31 @@ bool FieldRegion::canPrintPretty() const {
   return true;
 }
 
-void FieldRegion::printPrettyNoQuotes(raw_ostream &os) const {
-  if (superRegion->canPrintPretty()) {
-    superRegion->printPrettyNoQuotes(os);
-    os << "." << getDecl()->getName();
-  } else {
-    os << "field " << "\'" << getDecl()->getName() << "'";
-  }
+bool FieldRegion::canPrintPrettyAsExpr() const {
+  return superRegion->canPrintPrettyAsExpr();
+}
+
+void FieldRegion::printPrettyAsExpr(raw_ostream &os) const {
+  assert(canPrintPrettyAsExpr());
+  superRegion->printPrettyAsExpr(os);
+  os << "." << getDecl()->getName();
 }
 
 void FieldRegion::printPretty(raw_ostream &os) const {
-  if (superRegion->canPrintPretty()) {
+  if (canPrintPrettyAsExpr()) {
     os << "\'";
-    printPrettyNoQuotes(os);
+    printPrettyAsExpr(os);
     os << "'";
   } else {
-    printPrettyNoQuotes(os);
+    os << "field " << "\'" << getDecl()->getName() << "'";
   }
   return;
 }
 
+bool FieldRegion::canPrintPrettyAsExpr() const {
+  return superRegion->canPrintPrettyAsExpr();
+}
+
 //===----------------------------------------------------------------------===//
 // MemRegionManager methods.
 //===----------------------------------------------------------------------===//
diff --git a/test/Analysis/inlining/path-notes.c b/test/Analysis/inlining/path-notes.c
index 15140bdf52..d4fa461e5e 100644
--- a/test/Analysis/inlining/path-notes.c
+++ b/test/Analysis/inlining/path-notes.c
@@ -121,6 +121,18 @@ int testSetFieldToNull(struct X *x) {
   // expected-note@-2 {{Dereference of null pointer (loaded from field 'p')}}
 }
 
+struct Outer {
+  struct Inner {
+    int *p;
+  } inner;
+};
+
+void test(struct Outer *wrapperPtr) {
+  wrapperPtr->inner.p = 0;  // expected-note {{Null pointer value stored to field 'p'}}
+  *wrapperPtr->inner.p = 1; //expected-warning {{Dereference of null pointer (loaded from field 'p')}}
+                            // expected-note@-1 {{Dereference of null pointer (loaded from field 'p')}}
+}
+
 // CHECK:  <key>diagnostics</key>
 // CHECK-NEXT:  <array>
 // CHECK-NEXT:   <dict>
@@ -2900,4 +2912,113 @@ int testSetFieldToNull(struct X *x) {
 // CHECK-NEXT:    <key>file</key><integer>0</integer>
 // CHECK-NEXT:   </dict>
 // CHECK-NEXT:   </dict>
+// CHECK-NEXT:   <dict>
+// CHECK-NEXT:    <key>path</key>
+// CHECK-NEXT:    <array>
+// CHECK-NEXT:     <dict>
+// CHECK-NEXT:      <key>kind</key><string>event</string>
+// CHECK-NEXT:      <key>location</key>
+// CHECK-NEXT:      <dict>
+// CHECK-NEXT:       <key>line</key><integer>131</integer>
+// CHECK-NEXT:       <key>col</key><integer>3</integer>
+// CHECK-NEXT:       <key>file</key><integer>0</integer>
+// CHECK-NEXT:      </dict>
+// CHECK-NEXT:      <key>ranges</key>
+// CHECK-NEXT:      <array>
+// CHECK-NEXT:        <array>
+// CHECK-NEXT:         <dict>
+// CHECK-NEXT:          <key>line</key><integer>131</integer>
+// CHECK-NEXT:          <key>col</key><integer>3</integer>
+// CHECK-NEXT:          <key>file</key><integer>0</integer>
+// CHECK-NEXT:         </dict>
+// CHECK-NEXT:         <dict>
+// CHECK-NEXT:          <key>line</key><integer>131</integer>
+// CHECK-NEXT:          <key>col</key><integer>25</integer>
+// CHECK-NEXT:          <key>file</key><integer>0</integer>
+// CHECK-NEXT:         </dict>
+// CHECK-NEXT:        </array>
+// CHECK-NEXT:      </array>
+// CHECK-NEXT:      <key>depth</key><integer>0</integer>
+// CHECK-NEXT:      <key>extended_message</key>
+// CHECK-NEXT:      <string>Null pointer value stored to field &apos;p&apos;</string>
+// CHECK-NEXT:      <key>message</key>
+// CHECK-NEXT:      <string>Null pointer value stored to field &apos;p&apos;</string>
+// CHECK-NEXT:     </dict>
+// CHECK-NEXT:     <dict>
+// CHECK-NEXT:      <key>kind</key><string>control</string>
+// CHECK-NEXT:      <key>edges</key>
+// CHECK-NEXT:       <array>
+// CHECK-NEXT:        <dict>
+// CHECK-NEXT:         <key>start</key>
+// CHECK-NEXT:          <array>
+// CHECK-NEXT:           <dict>
+// CHECK-NEXT:            <key>line</key><integer>131</integer>
+// CHECK-NEXT:            <key>col</key><integer>3</integer>
+// CHECK-NEXT:            <key>file</key><integer>0</integer>
+// CHECK-NEXT:           </dict>
+// CHECK-NEXT:           <dict>
+// CHECK-NEXT:            <key>line</key><integer>131</integer>
+// CHECK-NEXT:            <key>col</key><integer>12</integer>
+// CHECK-NEXT:            <key>file</key><integer>0</integer>
+// CHECK-NEXT:           </dict>
+// CHECK-NEXT:          </array>
+// CHECK-NEXT:         <key>end</key>
+// CHECK-NEXT:          <array>
+// CHECK-NEXT:           <dict>
+// CHECK-NEXT:            <key>line</key><integer>132</integer>
+// CHECK-NEXT:            <key>col</key><integer>3</integer>
+// CHECK-NEXT:            <key>file</key><integer>0</integer>
+// CHECK-NEXT:           </dict>
+// CHECK-NEXT:           <dict>
+// CHECK-NEXT:            <key>line</key><integer>132</integer>
+// CHECK-NEXT:            <key>col</key><integer>3</integer>
+// CHECK-NEXT:            <key>file</key><integer>0</integer>
+// CHECK-NEXT:           </dict>
+// CHECK-NEXT:          </array>
+// CHECK-NEXT:        </dict>
+// CHECK-NEXT:       </array>
+// CHECK-NEXT:     </dict>
+// CHECK-NEXT:     <dict>
+// CHECK-NEXT:      <key>kind</key><string>event</string>
+// CHECK-NEXT:      <key>location</key>
+// CHECK-NEXT:      <dict>
+// CHECK-NEXT:       <key>line</key><integer>132</integer>
+// CHECK-NEXT:       <key>col</key><integer>3</integer>
+// CHECK-NEXT:       <key>file</key><integer>0</integer>
+// CHECK-NEXT:      </dict>
+// CHECK-NEXT:      <key>ranges</key>
+// CHECK-NEXT:      <array>
+// CHECK-NEXT:        <array>
+// CHECK-NEXT:         <dict>
+// CHECK-NEXT:          <key>line</key><integer>132</integer>
+// CHECK-NEXT:          <key>col</key><integer>22</integer>
+// CHECK-NEXT:          <key>file</key><integer>0</integer>
+// CHECK-NEXT:         </dict>
+// CHECK-NEXT:         <dict>
+// CHECK-NEXT:          <key>line</key><integer>132</integer>
+// CHECK-NEXT:          <key>col</key><integer>22</integer>
+// CHECK-NEXT:          <key>file</key><integer>0</integer>
+// CHECK-NEXT:         </dict>
+// CHECK-NEXT:        </array>
+// CHECK-NEXT:      </array>
+// CHECK-NEXT:      <key>depth</key><integer>0</integer>
+// CHECK-NEXT:      <key>extended_message</key>
+// CHECK-NEXT:      <string>Dereference of null pointer (loaded from field &apos;p&apos;)</string>
+// CHECK-NEXT:      <key>message</key>
+// CHECK-NEXT:      <string>Dereference of null pointer (loaded from field &apos;p&apos;)</string>
+// CHECK-NEXT:     </dict>
+// CHECK-NEXT:    </array>
+// CHECK-NEXT:    <key>description</key><string>Dereference of null pointer (loaded from field &apos;p&apos;)</string>
+// CHECK-NEXT:    <key>category</key><string>Logic error</string>
+// CHECK-NEXT:    <key>type</key><string>Dereference of null pointer</string>
+// CHECK-NEXT:   <key>issue_context_kind</key><string>function</string>
+// CHECK-NEXT:   <key>issue_context</key><string>test</string>
+// CHECK-NEXT:   <key>issue_hash</key><string>2</string>
+// CHECK-NEXT:   <key>location</key>
+// CHECK-NEXT:   <dict>
+// CHECK-NEXT:    <key>line</key><integer>132</integer>
+// CHECK-NEXT:    <key>col</key><integer>3</integer>
+// CHECK-NEXT:    <key>file</key><integer>0</integer>
+// CHECK-NEXT:   </dict>
+// CHECK-NEXT:   </dict>
 // CHECK-NEXT:  </array>