[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

2017-07-12 Thread don hinton via Phabricator via cfe-commits
hintonda added a comment. Great, thanks... https://reviews.llvm.org/D35175 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

2017-07-12 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment. I was impatient, so I already started on a patch for diagtool. I'll post it soon. https://reviews.llvm.org/D35175 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

2017-07-12 Thread don hinton via Phabricator via cfe-commits
hintonda added a comment. I'd be happy to do that if it would help. If so, should I do it here create a new diff? Perhaps we might even make sense add the ability to pass in a message and find the matching name/index. https://reviews.llvm.org/D35175 ___

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

2017-07-12 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment. Right. I was aware of the `diagtool` before, but didn't really look into what it did. TIL! It would make sense to add this kind of mapping functionality to that tool then. https://reviews.llvm.org/D35175 ___ cfe-commits m

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

2017-07-12 Thread don hinton via Phabricator via cfe-commits
hintonda added a comment. Not sure how to do this all in a script, but perhaps we could enhance diagtool to generate this mapping for you. It currently only lists warnings, but I don't think it would be difficult to extend it and generate a mapping you could use in your script. https://revi

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

2017-07-12 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment. My script relies on a hack to map the name of the diagnostic to the enum value. We need to come up with a better to map the diagnostic name to the enum value. I propose a new utility tool that would take the name of the diagnostic and map it back to the enum value.

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

2017-07-12 Thread don hinton via Phabricator via cfe-commits
hintonda abandoned this revision. hintonda added a comment. Okay, sounds good. Look forward to seeing Alex's script... https://reviews.llvm.org/D35175 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

2017-07-12 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. To me, features that only serve to help compiler development need to meet a higher bar than this. This just seems really marginal. Like Alex said, you should be able to pretty easily write a debugger script that breaks when it sees a specific diagnostic, or a diagnost

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

2017-07-11 Thread don hinton via Phabricator via cfe-commits
hintonda added a comment. It's just an effort to make the code a bit more accessible, especially for new users (or ones not used to running find/grep). Steve had suggested adding an option that took the entire message and matched it when it was produced. However, that won't work very well sinc

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

2017-07-11 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. This is a cute hack, but... I'm not sure I accept the premise that it's a noteworthy obstacle to Clang development to do two greps instead of one. And I don't think I've ever had to debug a diagnostic where __FILE__ and __LINE__ information would've been helpful. Als

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

2017-07-11 Thread don hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 106094. hintonda added a comment. - Only maintain enum names in debug builds. Current cost of maintaining enum names is 176k. https://reviews.llvm.org/D35175 Files: include/clang/Basic/DiagnosticIDs.h include/clang/Basic/DiagnosticOptions.def inclu

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

2017-07-11 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment. > Currently looks like around 200k (4534 @ 33 byte avg length + ptr). If this > is too much, we could make it conditional based on NDEBUG or some other macro > at compile time. I think this is mostly useful during development, so some conditional mechanism would make

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

2017-07-10 Thread don hinton via Phabricator via cfe-commits
hintonda updated this revision to Diff 105961. hintonda added a comment. - Make option cc1 only. Rename function, and add test. https://reviews.llvm.org/D35175 Files: include/clang/Basic/DiagnosticIDs.h include/clang/Basic/DiagnosticOptions.def include/clang/Driver/CC1Options.td lib/Ba

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

2017-07-10 Thread don hinton via Phabricator via cfe-commits
hintonda added a comment. In https://reviews.llvm.org/D35175#803496, @arphaman wrote: > Thanks, that's pretty cool! > > How bigger did the clang binary get after you've added all these strings? Currently looks like around 200k (4534 @ 33 byte avg length + ptr). If this is too much, we could m

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

2017-07-10 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment. Thanks, that's pretty cool! How bigger did the clang binary get after you've added all these strings? I feel like this is more of a CC1 option as well. I have some feedback for your additional ideas: > add another option to pass in the index (or enum) to force an asser

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

2017-07-09 Thread don hinton via Phabricator via cfe-commits
hintonda created this revision. This option helps locate the origin of a diagnostic message by outputing the enum name and index associated with a specific DiagID, allowing users to grep the code for the enum name directly without having to find it in the td files first. Additional ideas: 1. add