NoQ added a comment.

Y'all writing really good patches lately.

The updates on the C++ tests look spot on to me. I'm not sure if these tests 
contain any actual UB but the checker was anyway designed to be a bit more 
aggressive than that and that's exactly the kind of stuff we wanted it to warn 
us about. And even if it turns out to have been a bad idea from the start, 
that's a separate story.

I'll have some bikeshedding about warning and note text but overall I really 
like it.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:397-398
+                               Ctx.getLocationContext());
+      Report->addNote("The temporary object gets destroyed at the end of the "
+                      "full expression",
+                      L);
----------------
This is part of the path, right? We're reporting these bugs at 
`checkEndFunction` rather than at the store site so the destruction has already 
happened by the time we report it. In this case it should be a path note, not 
an extra blue-bubble note.

As for wording, I suggest `full-expression` with a dash (that's what other 
clang diagnostics use) and probably drop the initial "The"(?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107078

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

Reply via email to