Charusso added a comment.

Here is an example of the new `MemberExpr::getBase()` based report:
F9736772: report-Driver.cpp-operator()-6-1.html 
<https://reviews.llvm.org/F9736772>



================
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);
----------------
NoQ wrote:
> 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 `->`.
Previously I have focused on `DeclRefExpr` and `MemberExpr` value-evaluation, 
so that there the expression must be one of them. Because we have no 
`getField()` method for obtaining the field of the `MemberExpr`, I have picked 
`getBase()`.


================
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())
----------------
NoQ wrote:
> 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.
Hm, it probably makes sense. At this level 'Cond' and 'ME' are equals as I saw, 
but may in crazy conditions they are not.


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

https://reviews.llvm.org/D65663



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

Reply via email to