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

[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 make it conditional based on NDEBUG or some other macro at compile time.

> I feel like this is more of a CC1 option as well.

Sure, I can do that.

> I have some feedback for your additional ideas:
> 
>> add another option to pass in the index (or enum) to force an assert or backtrace when a specific DiagID is seen.
> 
> That sounds quite useful, but could it be something that's more suited for an external debugging script? I have a personal script that computes the enum value for a particular diagnostic, launches clang in lldb, sets a breakpoint for that particular diagnostic enum and runs clang. I could work on upstreaming it into clang/utils if people are interested.

This type of behavior (either an assert/bt or coupled with debugger) could be useful as a quick and easy solution.  However, capturing `__FILE__` and `__LINE__` when a diagnostic is reported, would be my preference.  However, that change would be very invasive and should probably be handled by a source to source transformation -- I did some of this by hand as a proof of concept, but doing all of clang manually would take quite a while, not to mention various tools that also use diagnostics.

>> capture FILE and LINE when a diagnostic is created and output it. This would make it easier to find the specific instance, and verify all instances are actually tested. Currently, it's almost impossible to determine if all instances are actually tested.
> 
> I reckon the first part (find the specific instance) could be useful, but I think that if you can force a backtrace on a specific DiagID then it becomes less useful. I disagree with the second part, can't you use our coverage bots and see if the all places where the diagnostic is emitted are covered to see if they are tested? It might be tedious to find these places, but maybe we can add a search for our coverage viewer so you quickly find the lines that have the name of diagnostic?

Agreed, but the problem with relying exclusively on coverage is that it can't cover the various permutations, e.g., "%select{A|B|C}0".  It's pretty difficult to tell if A, B, and C were actually tested -- or if that makes a difference.

If we included enum name (and permutation) with `__FILE__` and `__LINE__` in the output, then we could quickly analyze the test output and produce a simple report.  I think this would also be helpful in allowing test writers to see exactly which diagnostic report was triggered for each test.


https://reviews.llvm.org/D35175