[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Aha, aha, looks reasonable. If we're describing a variable, we should only highlight that variable.



================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2420
+  if (!IsAssuming) {
+    PathDiagnosticLocation Loc(BExpr->getLHS(), BRC.getSourceManager(), LCtx);
     return std::make_shared<PathDiagnosticPopUpPiece>(Loc, Message);
----------------
Just curious, can `BExpr->getLHS()` potentially still be a multi-line expression? Or are we making sure it's always a `DeclRefExpr`/`MemberExpr`?

In case of `MemberExpr` i'm pretty sure you can fit a newline before/after `.` or `->`.


================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2512
   const LocationContext *LCtx = N->getLocationContext();
-  PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx);
+  PathDiagnosticLocation Loc(ME, BRC.getSourceManager(), LCtx);
   if (!Loc.isValid() || !Loc.asLocation().isValid())
----------------
It looks like you forgot to make this change popup-piece-specific. I think we should add our note to the whole condition in case of event pieces.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65663