Bug fix for humane source location computation. (#346)

Fixes #345

A brief refresher: a `SourceLoc` in the Slang implementation is just an integer (more or less an absolute byte index into all of the source compiled so far). We convert that integer to a "humane" source location (a file name and line/column numbers) by finding the file and line that match the integer via binary search. The data structures used for that search are owned by a `SourceManager`.

In order to avoid running out of source locations when used in a long-running application (that might reload shaders many times), the implementation creates one `SourceManager` per `CompileRequest`, along with a single shared `SourceManager` that is used for locations in the builtin libraries.

The root of the bug here was that some code was using the `SourceManager` for a compile request when it should have been using the one for the builtins. This happened because one source manager was asked to translate a `SourceLoc` into a humane location, which first involves "expanding" that location (figuring out which file it belongs to, and which source manager owns that file), and failed to realize that the expanded location might use a different source manager (either the current one or one of its "parents").

I fixed this by reworking the API so that the mapping from an expanded location to a humane one is no longer a member of a source manager (since the correct source manager can be looked up in the associated expanded location). Hopefully this will prevent this class of error in the future.

3 files changed, 15 insertions, 7 deletions

diff --git a/source/slang/diagnostics.cpp b/source/slang/diagnostics.cpp
index 18e5f9a4d..4f4a33e60 100644
--- a/source/slang/diagnostics.cpp
+++ b/source/slang/diagnostics.cpp
@@ -155,7 +155,7 @@ static void formatDiagnostic(
     auto sourceManager = sink->sourceManager;
 
     auto expandedLoc = sourceManager->expandSourceLoc(diagnostic.loc);
-    auto humaneLoc = expandedLoc.sourceManager->getHumaneLoc(expandedLoc);
+    auto humaneLoc = getHumaneLoc(expandedLoc);
 
     sb << humaneLoc.getPath();
     sb << "(";
diff --git a/source/slang/source-loc.cpp b/source/slang/source-loc.cpp
index cea8441e6..80556ac37 100644
--- a/source/slang/source-loc.cpp
+++ b/source/slang/source-loc.cpp
@@ -197,14 +197,16 @@ ExpandedSourceLoc SourceManager::expandSourceLoc(SourceLoc const& loc)
     return Slang::expandSourceLoc(this, loc);
 }
 
-HumaneSourceLoc SourceManager::getHumaneLoc(ExpandedSourceLoc const& loc)
+HumaneSourceLoc getHumaneLoc(ExpandedSourceLoc const& loc)
 {
     // First check if this location maps to an actual file.
     SourceFile* sourceFile = loc.getSourceFile();
     if(!sourceFile)
         return HumaneSourceLoc();
 
-    auto& entry = sourceFiles[loc.entryIndex];
+    auto sourceManager = loc.sourceManager;
+
+    auto& entry = sourceManager->sourceFiles[loc.entryIndex];
     UInt offset = loc.getRaw() - entry.startLoc.getRaw();
 
     // We now have a raw input file that we can search for line breaks.
@@ -297,10 +299,14 @@ HumaneSourceLoc SourceManager::getHumaneLoc(ExpandedSourceLoc const& loc)
     return humaneLoc;
 }
 
-HumaneSourceLoc SourceManager::getHumaneLoc(SourceLoc const& loc)
+HumaneSourceLoc ExpandedSourceLoc::getHumaneLoc()
 {
-    return getHumaneLoc(expandSourceLoc(loc));
+    return Slang::getHumaneLoc(*this);
+}
 
+HumaneSourceLoc SourceManager::getHumaneLoc(SourceLoc const& loc)
+{
+    return expandSourceLoc(loc).getHumaneLoc();
 }
 
 SourceLoc SourceManager::getSpellingLoc(ExpandedSourceLoc const& loc)
diff --git a/source/slang/source-loc.h b/source/slang/source-loc.h
index 8bb89c645..83ad87633 100644
--- a/source/slang/source-loc.h
+++ b/source/slang/source-loc.h
@@ -117,9 +117,12 @@ struct ExpandedSourceLoc : public SourceLoc
 
     // Get the original source file that holds this location
     SourceFile* getSourceFile() const;
-};
 
+    // Get a "humane" version of a source location
+    HumaneSourceLoc getHumaneLoc();
+};
 
+HumaneSourceLoc getHumaneLoc(ExpandedSourceLoc const& loc);
 
 struct SourceManager
 {
@@ -142,7 +145,6 @@ struct SourceManager
     ExpandedSourceLoc expandSourceLoc(SourceLoc const& loc);
 
     // Get a "humane" version of a source location
-    HumaneSourceLoc getHumaneLoc(ExpandedSourceLoc const& loc);
     HumaneSourceLoc getHumaneLoc(SourceLoc const& loc);