[clang][Diagnostics] Fix wrong line number display

[hazohelet]

When the caret location is lower than the lowest source range, clang is printing wrong line numbers. The first line number should consider caret location line even when there are source ranges provided.

Current wrong line example: https://godbolt.org/z/aj4qEjzs4

[cor3ntin]

This looks good. My only question is, can Loc ever be invalid there, and what happens if it is?

[tbaederr]

In 8554a55, this was added to fix a range with a lower begin source location than Loc is, so I think this change would break that?

[hazohelet]

When the Loc is higher than the lowest begin source location, DisplayLineNo still points to the lowest begin location line because we have

  DisplayLineNo =
        std::min(DisplayLineNo, SM.getPresumedLineNumber(I.getBegin()));

in L1169, correct?
This does not break the test added in the commit at least locally.

I don't think we need to ignore Loc.getPresumedLoc().getLine() when Ranges is not empty.

[cor3ntin]

Ouch the inability to comment outside of modified code is going to be painful.

The changed code being

  unsigned DisplayLineNo = Loc.getPresumedLoc().getLine();
  for (const auto &I : Ranges) {
  DisplayLineNo = 
            std::min(DisplayLineNo, SM.getPresumedLineNumber(I.getBegin()));
  }

If the range starts before DisplayLineNo, it still works. Am I missing something?

[hazohelet]

Ouch the inability to comment outside of modified code is going to be painful.

The changed code being

  unsigned DisplayLineNo = Loc.getPresumedLoc().getLine();
  for (const auto &I : Ranges) {
  DisplayLineNo = 
            std::min(DisplayLineNo, SM.getPresumedLineNumber(I.getBegin()));
  }

If the range starts before DisplayLineNo, it still works. Am I missing something?

Thanks, that's my point.

When I provided an invalid source location, the diagnostics looked like the following. Filename, line/column number and code snippet are not printed.

source:7:3: error: no matching function for call to 'func'
    7 |   func(3, 4, 5);
      |   ^~~~
note: candidate function not viable: expects an lvalue for 3rd argument
1 error generated.

When Loc is invalid, calling Loc.getPresumedLoc().getLine() should cause validness assertion failure in PresumedLoc::getLine, so invalid Loc seems to be handled elsewhere.

[tbaederr]

Okay, makes sense to me. Should probably also be backported to 17.

[cor3ntin]

could you upload a dummy change to trigger the build? Thanks!

[cor3ntin]

LGTM, thanks