[clang] buildFixItInsertionLine should use Hints of the same FID and LineNo

[chh]

Summary

Fix bug https://bugs.llvm.org/show_bug.cgi?id=33734

[chh]

See https://reviews.llvm.org/D35225 for a test case.

[alexfh]

lib/Frontend/TextDiagnostic.cpp

1109–1110 ↗

(On Diff #105937)

Did you figure out why the old code used to give up here? Why does your code just continue?

[chh]

lib/Frontend/TextDiagnostic.cpp

1109–1110 ↗

(On Diff #105937)

I don't know. It does not make sense to me for the use cases I think should work.
Hints can contain suggested changes for different files and lines.
If those hints can be in any order, resetting FixItInsertLine and break from the loop would lose the collected line.

[chh]

lib/Frontend/TextDiagnostic.cpp

1109–1110 ↗

(On Diff #105937)

I found the old code introduced in https://reviews.llvm.org/rL70656
to fix the problem in https://bugs.llvm.org/show_bug.cgi?id=4084
This change does not have the problem in PR4084.

$ cat /tmp/t.c
int f0(char *a, char *b) {
  return ("a" !=
          "b");
}

$ clang -Wall -fsyntax-only /tmp/t.c
/tmp/t.c:2:15: warning: result of comparison against a string literal is unspecified (use strncmp instead) [-Wstring-compare]
  return ("a" !=
          ~~~ ^
1 warning generated.

[alexfh]

LG. Thank you for the fix!

cfe/trunk/lib/Frontend/TextDiagnostic.cpp

static void highlightRange(const CharSourceRange &R,
   EndColNo = map.byteToContainingColumn(EndColNo);
 
   assert(StartColNo <= EndColNo && "Invalid range!");
   if (CaretLine.size() < EndColNo)
     CaretLine.resize(EndColNo,' ');
   std::fill(CaretLine.begin()+StartColNo,CaretLine.begin()+EndColNo,'~');
 }
 
-static std::string buildFixItInsertionLine(unsigned LineNo,
+static std::string buildFixItInsertionLine(FileID FID,
+                                           unsigned LineNo,
                                            const SourceColumnMap &map,
                                            ArrayRef<FixItHint> Hints,
                                            const SourceManager &SM,
                                            const DiagnosticOptions *DiagOpts) {
   std::string FixItInsertionLine;
   if (Hints.empty() || !DiagOpts->ShowFixits)
     return FixItInsertionLine;
   unsigned PrevHintEndCol = 0;
 
   for (ArrayRef<FixItHint>::iterator I = Hints.begin(), E = Hints.end();
        I != E; ++I) {
     if (!I->CodeToInsert.empty()) {
       // We have an insertion hint. Determine whether the inserted
       // code contains no newlines and is on the same line as the caret.
       std::pair<FileID, unsigned> HintLocInfo
         = SM.getDecomposedExpansionLoc(I->RemoveRange.getBegin());
-      if (LineNo == SM.getLineNumber(HintLocInfo.first, HintLocInfo.second) &&
+      if (FID == HintLocInfo.first &&
+          LineNo == SM.getLineNumber(HintLocInfo.first, HintLocInfo.second) &&
           StringRef(I->CodeToInsert).find_first_of("\n\r") == StringRef::npos) {
         // Insert the new code into the line just below the code
         // that the user wrote.
         // Note: When modifying this function, be very careful about what is a
         // "column" (printed width, platform-dependent) and what is a
         // "byte offset" (SourceManager "column").
         unsigned HintByteOffset
           = SM.getColumnNumber(HintLocInfo.first, HintLocInfo.second) - 1;
if (!I->CodeToInsert.empty()) {
         if (NewFixItLineSize > FixItInsertionLine.size())
           FixItInsertionLine.resize(NewFixItLineSize, ' ');
 
         std::copy(I->CodeToInsert.begin(), I->CodeToInsert.end(),
                   FixItInsertionLine.end() - I->CodeToInsert.size());
 
         PrevHintEndCol =
           HintCol + llvm::sys::locale::columnWidth(I->CodeToInsert);
-      } else {
-        FixItInsertionLine.clear();
-        break;
       }
     }
   }
 
   expandTabs(FixItInsertionLine, DiagOpts->TabStop);
 
   return FixItInsertionLine;
 }
for (unsigned LineNo = Lines.first; LineNo != Lines.second + 1; ++LineNo) {
     if (CaretLineNo == LineNo) {
       CaretColNo = sourceColMap.byteToContainingColumn(CaretColNo - 1);
       if (CaretLine.size() < CaretColNo + 1)
         CaretLine.resize(CaretColNo + 1, ' ');
       CaretLine[CaretColNo] = '^';
     }
 
     std::string FixItInsertionLine = buildFixItInsertionLine(
-        LineNo, sourceColMap, Hints, SM, DiagOpts.get());
+        FID, LineNo, sourceColMap, Hints, SM, DiagOpts.get());
 
     // If the source line is too long for our terminal, select only the
     // "interesting" source region within that line.
     unsigned Columns = DiagOpts->MessageLength;
     if (Columns)
       selectInterestingSourceRegion(SourceLine, CaretLine, FixItInsertionLine,
                                     Columns, sourceColMap);