This translates diagnostics correctly between 0-based LSP columns, and 1-based clang columns.
Also, use TextEdit as early as possible.
Sorry for the delay getting this reviewed.
LG, thanks for fixing this! Just style nits.
nit: functions are lowerCamelCase.
only used once and private - inline this for now?
It seems like this could be decomposed into
The first probably belongs (or exists) elsewhere in clang, though I can't find it - fine to keep it here.
Incidentally, these tests seem to be wrong! The ranges shouldn't be empty (at least this one).
Unrelated to your patch though, I'll look into it separately.
Hmm, I looked into the empty diagnostic ranges, and it overlaps here a lot.
Feel free to go ahead and land this and I can merge, otherwise I can include this fix there.
Regardless of how it gets landed, thanks for finding and fixing this, this explains a lot of weird behavior!
nit: consider just toPosition, with the location as the first argument
And similarly below. The shorter names and putting the main param first make the callsites easier to read I think.
Actually I think the DiagnosticConsumer interface should pass you the language options itself, if you override the lifecycle methods.
D41118 adds real (non-empty) ranges for diagnostics.
I've copied all the changes you've made here with some slight tweaks, as they were required for that too.
If you like it as-is, probably easiest to land that instead.
But I'm also happy for you to land this and I'll merge. Up to you.