donat.nagy added a comment. (Just added some side remarks about string handling annoyances.)
@balazske Please (1) handle the `dyn_cast` request of @steakhal and also (2) do something with this "how to convert `StringRef` to `char *`" question that we're bikeshedding. I hope that after those we could finally merge this commit sequence. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1305 + std::string Note = + llvm::formatv(Case.getNote().str().c_str(), + cast<NamedDecl>(Call.getDecl())->getDeclName()); ---------------- steakhal wrote: > 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. I think the way of expressing this concrete conversion is a very low priority question (a.k.a. bikeshedding), so returning to the previously used two-step conversion is also fine for me. The primary goal of this ".data()" shortcut was not performance, but clarity: those chained conversions triggered my "detect suspicious code" instincts and this was the best alternative that I could find. In general I feel that the LLVM codebase has way too many string types, and the back-and-forth conversions between them add lots of unnecessary noise to the code. In this particular case I think it's the fault of `llvm::formatv` that it doesn't accept `StringRef` (the most commonly used non-owning string) and in general it's problematic design that there is no "good" conversion between `StringRef` and C-style strings in either direction. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153612/new/ https://reviews.llvm.org/D153612 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
