In SourceManager::isBeforeInTranslationUnit, if we are trying to compare two source locations with no common ancestor in the include stack, determine order by assuming memory buffers preceed files, and then that FileIDs are created in order.

The later assumption is patently false, but this was already broken -- this situation is conceptually impossible, my feeling is we should fix SourceManager and friends to make it impossible in practice as well. However, we need to fix PR5662 and perhaps some other things involving memory buffers first. In the short term I'm pretty sure this is reliable.

Chris, Argiris, is this going to break anything that wasn't already broken?

llvm-svn: 90280

1 files changed, 14 insertions, 24 deletions

diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index 394f493fa90..20b32da2fad 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -1087,30 +1087,20 @@ bool SourceManager::isBeforeInTranslationUnit(SourceLocation LHS,
       return LastResForBeforeTUCheck = LOffs.second < I->second;
   }
 
-  // No common ancestor.
-  // Now we are getting into murky waters. Most probably this is because one
-  // location is in the predefines buffer.
-
-  const FileEntry *LEntry =
-    getSLocEntry(LOffs.first).getFile().getContentCache()->Entry;
-  const FileEntry *REntry =
-    getSLocEntry(ROffs.first).getFile().getContentCache()->Entry;
-
-  // If the locations are in two memory buffers we give up, we can't answer
-  // which one should be considered first.
-  // FIXME: Should there be a way to "include" memory buffers in the translation
-  // unit ?
-  assert((LEntry != 0 || REntry != 0) && "Locations in memory buffers.");
-  (void) REntry;
-
-  // Consider the memory buffer as coming before the file in the translation
-  // unit.
-  if (LEntry == 0)
-    return LastResForBeforeTUCheck = true;
-  else {
-    assert(REntry == 0 && "Locations in not #included files ?");
-    return LastResForBeforeTUCheck = false;
-  }
+  // There is no common ancestor, most probably because one location is in the
+  // predefines buffer.
+  //
+  // FIXME: We should rearrange the external interface so this simply never
+  // happens; it can't conceptually happen. Also see PR5662.
+
+  // If exactly one location is a memory buffer, assume it preceeds the other.
+  bool LIsMB = !getSLocEntry(LOffs.first).getFile().getContentCache()->Entry;
+  bool RIsMB = !getSLocEntry(ROffs.first).getFile().getContentCache()->Entry;
+  if (LIsMB != RIsMB)
+    return LastResForBeforeTUCheck = LIsMB;
+
+  // Otherwise, just assume FileIDs were created in order.
+  return LastResForBeforeTUCheck = (LOffs.first < ROffs.first);
 }
 
 void SourceManager::truncateFileAt(const FileEntry *Entry, unsigned Line,