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

[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 sense IMO. I think that it makes sense to avoid the growth of our release binaries if such growth can be avoided.

> 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.

I can definitely see the usefulness of `__FILE__` and `__LINE__` markers. However, I think that should be a development only feature as well. I agree about the source to source transformation, a refactoring tool should handle it.

> 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.

That makes sense, thanks for pointing it out. I agree, It would be useful if we had such "coverage" reports for diagnostics.


https://reviews.llvm.org/D35175