MaskRay added inline comments.
================ Comment at: llvm/lib/Support/PrettyStackTrace.cpp:145 +static const char* customBugReportMsg; + ---------------- MaskRay wrote: > jhenderson wrote: > > jhenderson wrote: > > > `static const char *BugReportMsg;` > > > > > > Why not just have this set to the default in the first place, and > > > overwrite it if somebody needs to? That'll remove the need for the new > > > `if` in the `CrashHandler` function. > > Again, this hasn't been properly addresssed. This should use an upper-case > > letter for the first letter of variable names. Please see [[ > > https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly > > | the coding standards ]]. > `static const char BugReportMsg[] = ...` > > `static const char *BugReportMsg` defines a writable variable. > > In IR, the former does not have the unnamed_addr attribute while the latter > has. The latter can lower to .rodata.str1.1 (SHF_MERGE | SHF_STRINGS) which > can be optimized by the linker. This is nuance and the optimization probably > weighs nothing (it is hardly to think there will be another thing having the > same byte sequence.) The former just seems more correct. Nevermind. I did not see `setBugReportMsg` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74324/new/ https://reviews.llvm.org/D74324 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
