dblaikie added a subscriber: scott.linder.
dblaikie added a comment.

Thanks!

So I can see where/how this fails now - the LLVM backend seems to require that 
if any file has embedded source, they all do. Would you be able to/would you 
mind adding a debug info verifier check for this? That'd help make this fail 
sooner. (@scott.linder - perhaps you'd be able to/interested in doing this - 
looks like you wrote the initial implementation?)

Beyond that, then the question is whether this fix is the right way to satisfy 
that requirement (I'm assuming the requirement is reasonable - though I've not 
looked at the embedded source support & have no idea if there's a way/it's 
reasonable to support some embedded source and some not). My big question & 
I've tried to do some debugging to better understand this - but don't have the 
time to dive much further into it (& haven't really figured it out as yet): Why 
is this source not accessible through this API at this point? is the fallback 
to the main file always accurate?

(as far as I got was figuring out that SourceManager::getBufferData had an SLoc 
with isFile == true for the header, but isFile was false for the SLocs related 
to the .cpp file. Can't say I know why that is/what that means & would like to 
understand that (or someone else more familiar with this stuff who already 
knows can review this code) before approving this patch - if you could help 
explain this, that'd be great)


Repository:
  rC Clang

https://reviews.llvm.org/D53329



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to