<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>I know I suggested the "-ing" form, but it actually didn't come out as nice as I thought. Some alternate suggestions inline:</div><div><br></div><br><div><div>On Apr 12, 2013, at 11:40 , Anna Zaks <<a href="mailto:ganna@apple.com">ganna@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Author: zaks<br>Date: Fri Apr 12 13:40:27 2013<br>New Revision: 179396<br><br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project?rev=179396&view=rev">http://llvm.org/viewvc/llvm-project?rev=179396&view=rev</a><br>Log:<br>[analyzer] Print a diagnostic note even if the region cannot be printed.<br><br>There are few cases where we can track the region, but cannot print the note,<br>which makes the testing limited. (Though, I’ve tested this manually by making<br>all regions non-printable.) Even though the applicability is limited now, the enhancement<br>will be more relevant as we start tracking more regions.<br><br>Modified:<br> cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp<br> cfe/trunk/test/Analysis/inlining/path-notes.cpp<br><br>Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=179396&r1=179395&r2=179396&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=179396&r1=179395&r2=179396&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)<br>+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Fri Apr 12 13:40:27 2013<br>@@ -511,9 +511,6 @@ PathDiagnosticPiece *FindLastStoreBRVisi<br> }<br> }<br><br>- if (!R->canPrintPretty())<br>- return 0;<br>-<br> // Okay, we've found the binding. Emit an appropriate message.<br> SmallString<256> sbuf;<br> llvm::raw_svector_ostream os(sbuf);<br>@@ -525,9 +522,11 @@ PathDiagnosticPiece *FindLastStoreBRVisi<br> const VarRegion *VR = dyn_cast<VarRegion>(R);<br><br> if (DS) {<br>- action = "initialized to ";<br>+ action = R->canPrintPretty() ? "initialized to " :<br>+ "Initializing to ";<br> } else if (isa<BlockExpr>(S)) {<br>- action = "captured by block as ";<br>+ action = R->canPrintPretty() ? "captured by block as " :<br>+ "Capturing by block as ";<br></div></blockquote><div><br></div><div>For some reason this isn't quite grammatical; in this case "Captured by block as " doesn't seem so bad. This will be very rare in practice, though, because blocks can only capture variables, meaning this will only affect blocks capturing null references. (I don't think we'll have garbage references.)</div><div><br></div><div><br></div><br><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"> if (VR) {<br> // See if we can get the BlockVarRegion.<br> ProgramStateRef State = StoreSite->getState();<br>@@ -545,9 +544,11 @@ PathDiagnosticPiece *FindLastStoreBRVisi<br> }<br><br> if (action) {<br>- R->printPretty(os);<br>- os << " ";<br>- <br>+ if (R->canPrintPretty()) {<br>+ R->printPretty(os);<br>+ os << " ";<br>+ }<br>+<br> if (V.getAs<loc::ConcreteInt>()) {<br> bool b = false;<br> if (R->isBoundable()) {<br>@@ -569,14 +570,18 @@ PathDiagnosticPiece *FindLastStoreBRVisi<br> if (V.isUndef()) {<br> if (isa<VarRegion>(R)) {<br> const VarDecl *VD = cast<VarDecl>(DS->getSingleDecl());<br>- if (VD->getInit())<br>- os << "initialized to a garbage value";<br>- else<br>- os << "declared without an initial value";<br>+ if (VD->getInit()) {<br>+ os << (R->canPrintPretty() ? "initialized" : "Initializing")<br>+ << " to a garbage value";<br></div></blockquote><div><br></div><div>We could use 'action' here, which already has "Initializing to" at this point.</div><div><br></div><br><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">+ } else {<br>+ os << (R->canPrintPretty() ? "declared" : "Declaring")<br>+ << " without an initial value";<br>+ }<br> }<br> }<br> else {<br>- os << "initialized here";<br>+ os << (R->canPrintPretty() ? "initialized" : "Initializing")<br>+ << " here";<br></div></blockquote><div><br></div><div>"Initialized here" sounds better to me, though neither of them are great.</div><div><br></div><br><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"> }<br> }<br> }<br>@@ -602,8 +607,11 @@ PathDiagnosticPiece *FindLastStoreBRVisi<br><br> // Printed parameter indexes are 1-based, not 0-based.<br> unsigned Idx = Param->getFunctionScopeIndex() + 1;<br>- os << " via " << Idx << llvm::getOrdinalSuffix(Idx) << " parameter ";<br>- R->printPretty(os);<br>+ os << " via " << Idx << llvm::getOrdinalSuffix(Idx) << " parameter";<br>+ if (R->canPrintPretty()) {<br>+ os << " ";<br>+ R->printPretty(os);<br>+ }<br></div></blockquote><div><br></div><div>This is much nicer than giving up. :-)</div><div><br></div><br><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"> }<br> }<br><br>@@ -613,25 +621,28 @@ PathDiagnosticPiece *FindLastStoreBRVisi<br> if (R->isBoundable()) {<br> if (const TypedValueRegion *TR = dyn_cast<TypedValueRegion>(R)) {<br> if (TR->getValueType()->isObjCObjectPointerType()) {<br>- os << "nil object reference stored to ";<br>+ os << "nil object reference stored";</div></blockquote><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"> b = true;<br> }<br> }<br> }<br><br> if (!b)<br>- os << "Null pointer value stored to ";<br>+ os << "Null pointer value stored";<br> }<br> else if (V.isUndef()) {<br>- os << "Uninitialized value stored to ";<br>+ os << "Uninitialized value stored";<br> } else if (Optional<nonloc::ConcreteInt> CV =<br> V.getAs<nonloc::ConcreteInt>()) {<br>- os << "The value " << CV->getValue() << " is assigned to ";<br>+ os << "The value " << CV->getValue() << " is assigned";<br> }<br> else<br>- os << "Value assigned to ";<br>+ os << "Value assigned";<br><br>- R->printPretty(os);<br>+ if (R->canPrintPretty()) {<br>+ os << " to ";<br>+ R->printPretty(os);<br>+ }<br></div></blockquote><div><br></div><div><div>These are where "Storing" sounds <i>much</i> better to me. "Storing nil object reference", "Storing null pointer value", "Storing uninitialized value", "Storing <value>", "Assigning value".</div></div></div></body></html>