[PATCH] D40746: Correctly handle line directives without filenames that come first in the file

[Hans Wennborg via Phabricator via cfe-commits]

hans created this revision.

The comment in LineTableInfo::AddLineNote says "An unspecified
FilenameID means use the last filename if available, or the main source
file otherwise.", but the second part of that sentence was never
actually implemented. This lead to asserts when writing the line table
to a PCH file.

(Chromium runs into this when building with the latest MS SDK.)


https://reviews.llvm.org/D40746

Files:
  include/clang/Basic/SourceManagerInternals.h
  lib/Basic/SourceManager.cpp
  test/PCH/line-directive-nofilename.c
  test/PCH/line-directive-nofilename.h


Index: test/PCH/line-directive-nofilename.h
===================================================================
--- /dev/null
+++ test/PCH/line-directive-nofilename.h
@@ -0,0 +1,5 @@
+#line 42
+int foo; // This should appear as at line-directive-nofilename.h:42
+
+#line 100 "foobar.h"
+int bar; // This should appear as at foobar.h:100
Index: test/PCH/line-directive-nofilename.c
===================================================================
--- /dev/null
+++ test/PCH/line-directive-nofilename.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -emit-pch -o %t %S/line-directive-nofilename.h
+// RUN: not %clang_cc1 -include-pch %t -fsyntax-only %s 2>&1 | FileCheck %s
+
+// This causes an "error: redefinition" diagnostic. The notes will have the
+// locations of the declarations from the PCH file.
+double foo, bar;
+
+// CHECK: line-directive-nofilename.h:42:5: note: previous definition is here
+// CHECK: foobar.h:100:5: note: previous definition is here
Index: lib/Basic/SourceManager.cpp
===================================================================
--- lib/Basic/SourceManager.cpp
+++ lib/Basic/SourceManager.cpp
@@ -207,13 +207,19 @@
 /// system header or extern C system header.
 void LineTableInfo::AddLineNote(FileID FID, unsigned Offset, unsigned LineNo,
                                 int FilenameID, unsigned EntryExit,
-                                SrcMgr::CharacteristicKind FileKind) {
+                                SrcMgr::CharacteristicKind FileKind,
+                                const SourceManager &SM) {
   std::vector<LineEntry> &Entries = LineEntries[FID];
 
   // An unspecified FilenameID means use the last filename if available, or the
   // main source file otherwise.
-  if (FilenameID == -1 && !Entries.empty())
-    FilenameID = Entries.back().FilenameID;
+  if (FilenameID == -1) {
+    if (!Entries.empty())
+      FilenameID = Entries.back().FilenameID;
+    else
+      FilenameID = getLineTableFilenameID(SM.getFileEntryForID(FID)->getName());
+  }
+  assert(FilenameID != -1);
 
   assert((Entries.empty() || Entries.back().FileOffset < Offset) &&
          "Adding line entries out of order!");
@@ -297,7 +303,7 @@
     EntryExit = 2;
 
   LineTable->AddLineNote(LocInfo.first, LocInfo.second, LineNo, FilenameID,
-                         EntryExit, FileKind);
+                         EntryExit, FileKind, *this);
 }
 
 LineTableInfo &SourceManager::getLineTable() {
Index: include/clang/Basic/SourceManagerInternals.h
===================================================================
--- include/clang/Basic/SourceManagerInternals.h
+++ include/clang/Basic/SourceManagerInternals.h
@@ -108,10 +108,9 @@
 
   unsigned getNumFilenames() const { return FilenamesByID.size(); }
 
-  void AddLineNote(FileID FID, unsigned Offset,
-                   unsigned LineNo, int FilenameID,
-                   unsigned EntryExit, SrcMgr::CharacteristicKind FileKind);
-
+  void AddLineNote(FileID FID, unsigned Offset, unsigned LineNo, int FilenameID,
+                   unsigned EntryExit, SrcMgr::CharacteristicKind FileKind,
+                   const SourceManager &SM);
 
   /// \brief Find the line entry nearest to FID that is before it.
   ///


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D40746.125199.patch
Type: text/x-patch
Size: 3200 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171201/54310614/attachment-0001.bin>