[PATCH] D153612: [clang][analyzer] Add and change NoteTags in StdLibraryFunctionsChecker.

[Balázs Benics via Phabricator via cfe-commits]

steakhal added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1305
+      std::string Note =
+          llvm::formatv(Case.getNote().str().c_str(),
+                        cast<NamedDecl>(Call.getDecl())->getDeclName());
----------------
donat.nagy wrote:
> Consider using the method `StringRef::data()` which directly converts a `StringRef` to a `const char *`. Your two-step conversion has the advantage that it adds a `\0` terminator even if the `StringRef` isn't null-terminated, but I cannot imagine a "natural" code change that would introduce references to non-null-terminated char arrays as note message templates.
I would prefer not to rely on that `StringRef`s (here) are expected to be null-terminated, unless benchmarks demonstrate that this is important. If that would turn out to be the case, then we would need to enforce this using some sort of `assert` expression.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1337
+    // if 'errno' is interesting.
+    if (const auto *D = dyn_cast_or_null<FunctionDecl>(Call.getDecl()))
+      if (const NoteTag *NT =
----------------
Previously, we had a `cast<>(Call.getDecl())`, thus we should only have a `dyn_cast` here.


================
Comment at: clang/test/Analysis/stream-note.c:61-62
   FILE *F1 = fopen("foo1.c", "r"); // expected-note {{Stream opened here}}
+  // stdargs-note at -1 {{'fopen' is successful}}
+  // stdargs-note at -2 {{'fopen' is successful}}
   if (!F1)
----------------
Why are these notes doubled?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153612