This fixes SARIF column locations to be end-exclusive and be counted in terms of Unicode characters.
You don't really need the whole preprocessor, just the language options, right? If so, why not thread the language options through rather than the PP?
Can you add some text to the assertion to explain what's gone wrong if the assert triggers? e.g. assert(!Loc.isInvalid() && "invalid location when adjusting column position"); or something (same for the others asserts you've added)?
Formatting of the curly brace looks wrong.
What file encoding is used by this file (and is there a BOM)? Have you tried this test on multiple platforms by any chance?
Mostly LG aside from a question about an assertion. Adding other reviewers outside of GrammaTech in case there are other concerns we've missed.
I don't think this API is able to return a null buffer, but it can return an invalid buffer under legitimate circumstances, I believe. For instance, if the source location is in the scratch space used for things like macros defined on the command line with -D. I think you should pass the optional invalid parameter to the call to getBuffer() and then do no adjustments if the buffer is invalid. WDYT?
Okay, I think that's reasonable, thanks!
Good catch that this can't return NULL.
I tested out defining things on the command line, but this code deals with expansion locations and wasn't affected by it. Unless you can think of a way to reproduce it, I think I'd rather pass in the invalid argument and assert that it isn't invalid.
LGTM! I think we've had sufficient time for other reviewers to lodge concerns and we can deal with any other issues post-commit. Do you need me to commit on your behalf?
That's fine by me -- I can't think of another way to reproduce it.