[PATCH] D74324: Tools emit the bug report URL on crash

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/include/llvm/Support/PrettyStackTrace.h:42
+  /// a crash.
+  void SetCustomBugReportMsg(const char *msg);
+
----------------
jhenderson wrote:
> `msg` -> `Msg`
> `SetCustomBugReportMsg` -> `setBugReportMsg`
The second half of this hasn't been properly addressed. Function names should start with a lower-case letter (see the LLVM coding standards). The other functions use an older style, and in general I think we should avoid adding more with the wrong style.


================
Comment at: llvm/lib/Support/PrettyStackTrace.cpp:145
 
+static const char* customBugReportMsg;
+
----------------
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 ]].


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

https://reviews.llvm.org/D74324