Page MenuHomePhabricator

Do not use CppHashInfo if reporting an error after asm file parsed
Needs ReviewPublic

Authored by pmatos on Oct 28 2016, 8:37 AM.

Details

Summary

If parsing has finished, do not allow DiagHandler to rely on last parsed CppHashInfo
Instead recalculate the hash line before the error location.

Fixes PR#30817

Diff Detail

Event Timeline

pmatos updated this revision to Diff 76194.Oct 28 2016, 8:37 AM
pmatos retitled this revision from to Do not use CppHashInfo if reporting an error after asm file parsed.
pmatos updated this object.
pmatos added reviewers: enderby, t.p.northover, grosbach.
pmatos added a subscriber: llvm-commits.

To the reviewers:
I believe there's a better way to handle this. However, I would like to know if this approach is a good one or the reviewers find that there's a much better way to do this and I took it in the wrong direction.

For example:

  1. The re-parsing of the hash line in FindCppHashLoc is not great. It would be useful if we could reuse the asm lexer/parser but as far as I know, we can't;
  2. I am slightly concerned that this fix won't work if the hash line doesn't fit the usual '# <int> ...', or worse, there's no hash line in the file; While in normal operation mode this doesn't happen it is possible to create a situation when the fix can be broken;

Can someone please take a look at this one?

enderby edited edge metadata.Dec 15 2016, 10:06 AM

Seem like a step forward. The initial implementation for the search backwards in SourceMgr::FindCppHashLoc() is not great but I suspect it will work well enough.

Apologies for leaving this for so long. Are there any further comments?
How do we get this upstream given I don't have commit rights?