Feature/source loc review (#672)

* Fixes/improvements based around review comments.

* SourceUnit -> SourceView

* * Removed the HumaneSourceLoc as it's POD-like ness seemed to make that unnecessary
* Made exposed member variables in SourceManager protected - so make clear where/how can be accesed
* Improved description about SourceLoc and associated structures

* Changed SourceLocType to 'Actual' and 'Nominal'.

* Improved a comment.

8 files changed, 190 insertions, 152 deletions

diff --git a/source/slang/diagnostics.cpp b/source/slang/diagnostics.cpp
index 24019e4aa..ff664e81b 100644
--- a/source/slang/diagnostics.cpp
+++ b/source/slang/diagnostics.cpp
@@ -171,7 +171,7 @@ static void formatDiagnostic(
 
     auto humaneLoc = sourceManager->getHumaneLoc(diagnostic.loc);
 
-    sb << humaneLoc.getPath();
+    sb << humaneLoc.path;
     sb << "(";
     sb << humaneLoc.line;
     sb << "): ";
diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp
index 9411c6fdf..00bc2556b 100644
--- a/source/slang/emit.cpp
+++ b/source/slang/emit.cpp
@@ -578,8 +578,8 @@ struct EmitVisitor
 
             if(shouldUseGLSLStyleLineDirective)
             {
-                auto path = sourceLocation.getPath();
-
+                auto path = sourceLocation.path;
+                
                 // GLSL doesn't support the traditional form of a `#line` directive without
                 // an extension. Rather than depend on that extension we will output
                 // a directive in the traditional GLSL fashion.
@@ -609,7 +609,7 @@ struct EmitVisitor
                 // in a module that tracks source files.
 
                 emitRawText("\"");
-                for(auto c : sourceLocation.getPath())
+                for(auto c : sourceLocation.path)
                 {
                     char charBuffer[] = { c, 0 };
                     switch(c)
diff --git a/source/slang/lexer.cpp b/source/slang/lexer.cpp
index 339d98d49..918668296 100644
--- a/source/slang/lexer.cpp
+++ b/source/slang/lexer.cpp
@@ -84,22 +84,22 @@ namespace Slang
     // Lexer
 
     void Lexer::initialize(
-        SourceUnit*     inSourceUnit,
+        SourceView*     inSourceView,
         DiagnosticSink* inSink,
         NamePool*       inNamePool)
     {
-        sourceUnit  = inSourceUnit;
+        sourceView  = inSourceView;
         sink        = inSink;
         namePool    = inNamePool;
 
-        auto content = inSourceUnit->getSourceFile()->content;
-
+        auto content = inSourceView->getContent();
+        
         begin   = content.begin();
         cursor  = content.begin();
         end     = content.end();
 
         // Set the start location
-        startLoc = inSourceUnit->getRange().begin;
+        startLoc = inSourceView->getRange().begin;
 
         tokenFlags = TokenFlag::AtStartOfLine | TokenFlag::AfterWhitespace;
         lexerFlags = 0;
diff --git a/source/slang/lexer.h b/source/slang/lexer.h
index 46d2aa227..85aca77dd 100644
--- a/source/slang/lexer.h
+++ b/source/slang/lexer.h
@@ -73,7 +73,7 @@ namespace Slang
     struct Lexer
     {
         void initialize(
-            SourceUnit*     sourceUnit,
+            SourceView*     sourceView,
             DiagnosticSink* sink,
             NamePool*       namePool);
 
@@ -83,7 +83,7 @@ namespace Slang
 
         TokenList lexAllTokens();
 
-        SourceUnit*     sourceUnit;
+        SourceView*     sourceView;
         DiagnosticSink* sink;
         NamePool*       namePool;
 
@@ -92,17 +92,9 @@ namespace Slang
         char const*     begin;
         char const*     end;
 
-        /// The starting sourceLoc (same as first location of SourceUnit)
+        /// The starting sourceLoc (same as first location of SourceView)
         SourceLoc       startLoc;           
 
-        // The starting source location for the code as written,
-        // which cannot be overridden.
-        //SourceLoc       spellingStartLoc;
-
-        // The nominal starting location for the file, taking
-        // any active `#line` directive into account.
-        //SourceLoc       presumedStartLoc;
-
         TokenFlags      tokenFlags;
         LexerFlags      lexerFlags;
     };
diff --git a/source/slang/preprocessor.cpp b/source/slang/preprocessor.cpp
index 2986b275b..98347bd0c 100644
--- a/source/slang/preprocessor.cpp
+++ b/source/slang/preprocessor.cpp
@@ -265,13 +265,13 @@ static NamePool* getNamePool(Preprocessor* preprocessor)
 // TODO(tfoley): pre-tokenizing files isn't going to work in the long run.
 static PreprocessorInputStream* CreateInputStreamForSource(
     Preprocessor*   preprocessor,
-    SourceUnit*     sourceUnit)
+    SourceView*     sourceView)
 {
     PrimaryInputStream* inputStream = new PrimaryInputStream();
     initializePrimaryInputStream(preprocessor, inputStream);
 
     // initialize the embedded lexer so that it can generate a token stream
-    inputStream->lexer.initialize(sourceUnit, GetSink(preprocessor), getNamePool(preprocessor));
+    inputStream->lexer.initialize(sourceView, GetSink(preprocessor), getNamePool(preprocessor));
     inputStream->token = inputStream->lexer.lexToken();
 
     return inputStream;
@@ -838,12 +838,12 @@ top:
         SourceManager* sourceManager = preprocessor->getCompileRequest()->getSourceManager();
 
         // We create a dummy file to represent the token-paste operation
-        SourceFile* sourceFile = sourceManager->newSourceFile("token paste", sb.ProduceString());
+        SourceFile* sourceFile = sourceManager->createSourceFile("token paste", sb.ProduceString());
 
-        SourceUnit* sourceUnit = sourceManager->newSourceUnit(sourceFile);
+        SourceView* sourceView = sourceManager->createSourceView(sourceFile);
 
         Lexer lexer;
-        lexer.initialize(sourceUnit, GetSink(preprocessor), getNamePool(preprocessor));
+        lexer.initialize(sourceView, GetSink(preprocessor), getNamePool(preprocessor));
 
         SimpleTokenInputStream* inputStream = new SimpleTokenInputStream();
         initializeInputStream(preprocessor, inputStream);
@@ -1589,7 +1589,7 @@ static void HandleIncludeDirective(PreprocessorDirectiveContext* context)
 
     auto directiveLoc = GetDirectiveLoc(context);
     
-    String pathIncludedFrom = context->preprocessor->translationUnit->compileRequest->getSourceManager()->getPath(directiveLoc, SourceLocType::Original);
+    String pathIncludedFrom = context->preprocessor->translationUnit->compileRequest->getSourceManager()->getPath(directiveLoc, SourceLocType::Actual);
     String foundPath;
     ComPtr<ISlangBlob> foundSourceBlob;
 
@@ -1633,14 +1633,14 @@ static void HandleIncludeDirective(PreprocessorDirectiveContext* context)
     // If not create a new one, and add to the list of known source files
     if (!sourceFile)
     {
-        sourceFile = sourceManager->newSourceFile(foundPath, foundSourceBlob);
+        sourceFile = sourceManager->createSourceFile(foundPath, foundSourceBlob);
         sourceManager->addSourceFile(foundPath, sourceFile);
     }
 
     // This is a new parse (even if it's a pre-existing source file), so create a new SourceUnit
-    SourceUnit* sourceUnit = sourceManager->newSourceUnit(sourceFile);
+    SourceView* sourceView = sourceManager->createSourceView(sourceFile);
 
-    PreprocessorInputStream* inputStream = CreateInputStreamForSource(context->preprocessor, sourceUnit);
+    PreprocessorInputStream* inputStream = CreateInputStreamForSource(context->preprocessor, sourceView);
     inputStream->parent = context->preprocessor->inputStream;
     context->preprocessor->inputStream = inputStream;
 }
@@ -1803,8 +1803,8 @@ static void HandleLineDirective(PreprocessorDirectiveContext* context)
         AdvanceToken(context);
 
         // Stop overriding source locations.
-        auto sourceUnit = inputStream->primaryStream->lexer.sourceUnit;
-        sourceUnit->addDefaultLineDirective(directiveLoc);
+        auto sourceView = inputStream->primaryStream->lexer.sourceView;
+        sourceView->addDefaultLineDirective(directiveLoc);
         return;
     }
     else
@@ -1840,8 +1840,8 @@ static void HandleLineDirective(PreprocessorDirectiveContext* context)
         return;
     }
 
-    auto sourceUnit = inputStream->primaryStream->lexer.sourceUnit;
-    sourceUnit->addLineDirective(directiveLoc, file, line);
+    auto sourceView = inputStream->primaryStream->lexer.sourceView;
+    sourceView->addLineDirective(directiveLoc, file, line);
 }
 
 #define SLANG_PRAGMA_DIRECTIVE_CALLBACK(NAME) \
@@ -1875,7 +1875,7 @@ SLANG_PRAGMA_DIRECTIVE_CALLBACK(handlePragmaOnceDirective)
     // trivial cases of the "same" path.
     //
     auto directiveLoc = GetDirectiveLoc(context);
-    auto pathIssuedFrom = context->preprocessor->translationUnit->compileRequest->getSourceManager()->getPath(directiveLoc, SourceLocType::Original);
+    auto pathIssuedFrom = context->preprocessor->translationUnit->compileRequest->getSourceManager()->getPath(directiveLoc, SourceLocType::Actual);
 
     context->preprocessor->pragmaOncePaths.Add(pathIssuedFrom);
 }
@@ -2248,21 +2248,21 @@ static void DefineMacro(
 
     auto sourceManager = preprocessor->translationUnit->compileRequest->getSourceManager();
 
-    SourceFile* keyFile = sourceManager->newSourceFile(fileName, key);
-    SourceFile* valueFile = sourceManager->newSourceFile(fileName, value);
+    SourceFile* keyFile = sourceManager->createSourceFile(fileName, key);
+    SourceFile* valueFile = sourceManager->createSourceFile(fileName, value);
 
-    SourceUnit* keyUnit = sourceManager->newSourceUnit(keyFile);
-    SourceUnit* valueUnit = sourceManager->newSourceUnit(valueFile);
+    SourceView* keyView = sourceManager->createSourceView(keyFile);
+    SourceView* valueView = sourceManager->createSourceView(valueFile);
 
     // Use existing `Lexer` to generate a token stream.
     Lexer lexer;
-    lexer.initialize(valueUnit, GetSink(preprocessor), getNamePool(preprocessor));
+    lexer.initialize(valueView, GetSink(preprocessor), getNamePool(preprocessor));
     macro->tokens = lexer.lexAllTokens();
 
     Name* keyName = preprocessor->translationUnit->compileRequest->getNamePool()->getName(key);
 
     macro->nameAndLoc.name = keyName;
-    macro->nameAndLoc.loc = keyUnit->getRange().begin;
+    macro->nameAndLoc.loc = keyView->getRange().begin;
     
     PreprocessorMacro* oldMacro = NULL;
     if (preprocessor->globalEnv.macros.TryGetValue(keyName, oldMacro))
@@ -2311,10 +2311,10 @@ TokenList preprocessSource(
 
     SourceManager* sourceManager = translationUnit->compileRequest->getSourceManager();
 
-    SourceUnit* sourceUnit = sourceManager->newSourceUnit(file);
+    SourceView* sourceView = sourceManager->createSourceView(file);
 
     // create an initial input stream based on the provided buffer
-    preprocessor.inputStream = CreateInputStreamForSource(&preprocessor, sourceUnit);
+    preprocessor.inputStream = CreateInputStreamForSource(&preprocessor, sourceView);
 
     TokenList tokens = ReadAllTokens(&preprocessor);
 
diff --git a/source/slang/slang.cpp b/source/slang/slang.cpp
index eab9401b4..7ec448ab1 100644
--- a/source/slang/slang.cpp
+++ b/source/slang/slang.cpp
@@ -372,7 +372,7 @@ SlangResult CompileRequest::loadFile(String const& path, ISlangBlob** outBlob)
 
 RefPtr<Expr> CompileRequest::parseTypeString(TranslationUnitRequest * translationUnit, String typeStr, RefPtr<Scope> scope)
 {
-    Slang::RefPtr<Slang::SourceFile> srcFile = sourceManager->newSourceFile(String("type string"), typeStr);
+    Slang::RefPtr<Slang::SourceFile> srcFile = sourceManager->createSourceFile(String("type string"), typeStr);
     
     DiagnosticSink sink;
     sink.sourceManager = sourceManager;
@@ -675,7 +675,7 @@ void CompileRequest::addTranslationUnitSourceBlob(
     String const&   path,
     ISlangBlob*     sourceBlob)
 {
-    RefPtr<SourceFile> sourceFile = getSourceManager()->newSourceFile(path, sourceBlob);
+    RefPtr<SourceFile> sourceFile = getSourceManager()->createSourceFile(path, sourceBlob);
 
     addTranslationUnitSourceFile(translationUnitIndex, sourceFile);
 }
@@ -685,7 +685,7 @@ void CompileRequest::addTranslationUnitSourceString(
     String const&   path,
     String const&   source)
 {
-    RefPtr<SourceFile> sourceFile = getSourceManager()->newSourceFile(path, source);
+    RefPtr<SourceFile> sourceFile = getSourceManager()->createSourceFile(path, source);
 
     addTranslationUnitSourceFile(translationUnitIndex, sourceFile);
 }
@@ -804,7 +804,7 @@ RefPtr<ModuleDecl> CompileRequest::loadModule(
     // TODO: decide which options, if any, should be inherited.
     translationUnit->compileFlags = 0;
 
-    RefPtr<SourceFile> sourceFile = getSourceManager()->newSourceFile(path, sourceBlob);
+    RefPtr<SourceFile> sourceFile = getSourceManager()->createSourceFile(path, sourceBlob);
 
     translationUnit->sourceFiles.Add(sourceFile);
 
@@ -884,7 +884,7 @@ RefPtr<ModuleDecl> CompileRequest::findOrImportModule(
     includeHandler.request = this;
 
     // Get the original path
-    String pathIncludedFrom= getSourceManager()->getPath(loc, SourceLocType::Original);
+    String pathIncludedFrom= getSourceManager()->getPath(loc, SourceLocType::Actual);
     
     String foundPath;
     ComPtr<ISlangBlob> foundSourceBlob;
diff --git a/source/slang/source-loc.cpp b/source/slang/source-loc.cpp
index eef7b1ecd..905cb46c6 100644
--- a/source/slang/source-loc.cpp
+++ b/source/slang/source-loc.cpp
@@ -7,7 +7,7 @@ namespace Slang {
 
 /* !!!!!!!!!!!!!!!!!!!!!!!!! SourceUnit !!!!!!!!!!!!!!!!!!!!!!!!!!!! */
 
-int SourceUnit::findEntryIndex(SourceLoc sourceLoc) const
+int SourceView::findEntryIndex(SourceLoc sourceLoc) const
 {
     if (!m_range.contains(sourceLoc))
     {
@@ -47,7 +47,7 @@ int SourceUnit::findEntryIndex(SourceLoc sourceLoc) const
     return lo;
 }
 
-void SourceUnit::addLineDirective(SourceLoc directiveLoc, StringSlicePool::Handle pathHandle, int line)
+void SourceView::addLineDirective(SourceLoc directiveLoc, StringSlicePool::Handle pathHandle, int line)
 {
     SLANG_ASSERT(pathHandle != StringSlicePool::Handle(0));
     SLANG_ASSERT(m_range.contains(directiveLoc));
@@ -74,13 +74,13 @@ void SourceUnit::addLineDirective(SourceLoc directiveLoc, StringSlicePool::Handl
     m_entries.Add(entry);
 }
 
-void SourceUnit::addLineDirective(SourceLoc directiveLoc, const String& path, int line)
+void SourceView::addLineDirective(SourceLoc directiveLoc, const String& path, int line)
 {
     StringSlicePool::Handle pathHandle = m_sourceManager->getStringSlicePool().add(path.getUnownedSlice());
     return addLineDirective(directiveLoc, pathHandle, line);
 }
 
-void SourceUnit::addDefaultLineDirective(SourceLoc directiveLoc)
+void SourceView::addDefaultLineDirective(SourceLoc directiveLoc)
 {
     SLANG_ASSERT(m_range.contains(directiveLoc));
     // Check that the directiveLoc values are always increasing
@@ -102,7 +102,7 @@ void SourceUnit::addDefaultLineDirective(SourceLoc directiveLoc)
     m_entries.Add(entry);
 }
 
-HumaneSourceLoc SourceUnit::getHumaneLoc(SourceLoc loc, SourceLocType type)
+HumaneSourceLoc SourceView::getHumaneLoc(SourceLoc loc, SourceLocType type)
 {
     const int offset = m_range.getOffset(loc);
 
@@ -127,7 +127,7 @@ HumaneSourceLoc SourceUnit::getHumaneLoc(SourceLoc loc, SourceLocType type)
     StringSlicePool::Handle pathHandle = StringSlicePool::Handle(0);
 
     // Only bother looking up the entry information if we want a 'Normal' lookup
-    const int entryIndex = (type == SourceLocType::Normal) ? findEntryIndex(loc) : -1;
+    const int entryIndex = (type == SourceLocType::Nominal) ? findEntryIndex(loc) : -1;
     if (entryIndex >= 0)
     {
         const Entry& entry = m_entries[entryIndex];
@@ -150,9 +150,9 @@ HumaneSourceLoc SourceUnit::getHumaneLoc(SourceLoc loc, SourceLocType type)
     return humaneLoc;
 }
 
-String SourceUnit::getPath(SourceLoc loc, SourceLocType type)
+String SourceView::getPath(SourceLoc loc, SourceLocType type)
 {
-    if (type == SourceLocType::Original)
+    if (type == SourceLocType::Actual)
     {
         return m_sourceFile->path;
     }
@@ -263,7 +263,7 @@ int SourceFile::calcColumnIndex(int lineIndex, int offset)
 void SourceManager::initialize(
     SourceManager*  p)
 {
-    parent = p;
+    m_parent = p;
 
     if( p )
     {
@@ -272,16 +272,16 @@ void SourceManager::initialize(
         // right after those from the parent.
         //
         // TODO: more clever allocation in cases where that might not be reasonable
-        startLoc = p->nextLoc;
+        m_startLoc = p->m_nextLoc;
     }
     else
     {
         // Location zero is reserved for an invalid location,
         // so we need to start reserving locations starting at 1.
-        startLoc = SourceLoc::fromRaw(1);
+        m_startLoc = SourceLoc::fromRaw(1);
     }
 
-    nextLoc = startLoc;
+    m_nextLoc = m_startLoc;
 }
 
 SourceRange SourceManager::allocateSourceRange(UInt size)
@@ -289,19 +289,19 @@ SourceRange SourceManager::allocateSourceRange(UInt size)
     // TODO: consider using atomics here
 
 
-    SourceLoc beginLoc  = nextLoc;
+    SourceLoc beginLoc  = m_nextLoc;
     SourceLoc endLoc    = beginLoc + size;
 
     // We need to be able to represent the location that is *at* the end of
     // the input source, so the next available location for a new file
     // must be placed one after the end of this one.
 
-    nextLoc = endLoc + 1;
+    m_nextLoc = endLoc + 1;
 
     return SourceRange(beginLoc, endLoc);
 }
 
-SourceFile* SourceManager::newSourceFile(
+SourceFile* SourceManager::createSourceFile(
     String const&   path,
     ISlangBlob*     contentBlob)
 {
@@ -317,47 +317,61 @@ SourceFile* SourceManager::newSourceFile(
     return sourceFile;
 }
 
-SourceFile* SourceManager::newSourceFile(
+SourceFile* SourceManager::createSourceFile(
     String const&   path,
     String const&   content)
 {
     ComPtr<ISlangBlob> contentBlob = createStringBlob(content);
-    return newSourceFile(path, contentBlob);
+    return createSourceFile(path, contentBlob);
 }
 
-SourceUnit* SourceManager::newSourceUnit(SourceFile* sourceFile)
+SourceView* SourceManager::createSourceView(SourceFile* sourceFile)
 {
     SourceRange range = allocateSourceRange(sourceFile->content.size());
-    SourceUnit* sourceUnit = new SourceUnit(this, sourceFile, range);
-    m_sourceUnits.Add(sourceUnit);
+    SourceView* sourceView = new SourceView(this, sourceFile, range);
+    m_sourceViews.Add(sourceView);
 
-    return sourceUnit;
+    return sourceView;
 }
 
-SourceUnit* SourceManager::findSourceUnit(SourceLoc loc)
+SourceView* SourceManager::findSourceView(SourceLoc loc) const
 {
-    SourceLoc::RawValue rawLoc = loc.getRaw();
-
-    int hi = int(m_sourceUnits.Count());
+    int hi = int(m_sourceViews.Count());
+    // It must be in the range of this manager and have associated views for it to possibly be a hit
+    if (!getSourceRange().contains(loc) || hi == 0)
+    {
+        return nullptr;
+    }
 
-    if (hi == 0)
+    // If we don't have very many, we may as well just linearly search
+    if (hi <= 8)
     {
+        for (int i = 0; i < hi; ++i)
+        {
+            SourceView* view = m_sourceViews[i];
+            if (view->getRange().contains(loc))
+            {
+                return view;
+            }
+        }
         return nullptr;
     }
 
+    const SourceLoc::RawValue rawLoc = loc.getRaw();
+
+    // Binary chop to see if we can find the associated SourceUnit
     int lo = 0;
     while (lo + 1 < hi)
     {
         int mid = (hi + lo) >> 1;
 
-        SourceUnit* midEntry = m_sourceUnits[mid];
-        if (midEntry->getRange().contains(loc))
+        SourceView* midView = m_sourceViews[mid];
+        if (midView->getRange().contains(loc))
         {
-            return midEntry;
+            return midView;
         }
 
-        SourceLoc::RawValue midValue = midEntry->getRange().begin.getRaw();
-
+        const SourceLoc::RawValue midValue = midView->getRange().begin.getRaw();
         if (midValue <= rawLoc)
         {
             // The location we seek is at or after this entry
@@ -370,17 +384,30 @@ SourceUnit* SourceManager::findSourceUnit(SourceLoc loc)
         }
     }
 
-    // Check if low is a hit
+    // Check if low is actually a hit
+    SourceView* view = m_sourceViews[lo];
+    return (view->getRange().contains(loc)) ? view : nullptr;
+}
+
+SourceView* SourceManager::findSourceViewRecursively(SourceLoc loc) const
+{
+    // Start with this manager
+    const SourceManager* manager = this;
+    do 
     {
-        SourceUnit* unit = m_sourceUnits[lo];
-        if (unit->getRange().contains(loc))
+        SourceView* sourceView = findSourceView(loc);
+        // If we found a hit we are done
+        if (sourceView)
         {
-            return unit;
+            return sourceView;
         }
+        
+        // Try the parent
+        manager = manager->m_parent;
     }
-
-    // Check the parent if there is a parent
-    return (parent) ? parent->findSourceUnit(loc) : nullptr;
+    while (manager);
+    // Didn't find it
+    return nullptr;
 }
 
 SourceFile* SourceManager::findSourceFile(const String& path)
@@ -390,7 +417,7 @@ SourceFile* SourceManager::findSourceFile(const String& path)
     {
         return filePtr->Ptr();
     }
-    return parent ? parent->findSourceFile(path) : nullptr;
+    return m_parent ? m_parent->findSourceFile(path) : nullptr;
 }
 
 void SourceManager::addSourceFile(const String& path, SourceFile* sourceFile)
@@ -401,10 +428,10 @@ void SourceManager::addSourceFile(const String& path, SourceFile* sourceFile)
 
 HumaneSourceLoc SourceManager::getHumaneLoc(SourceLoc loc, SourceLocType type)
 {
-    SourceUnit* sourceUnit = findSourceUnit(loc);
-    if (sourceUnit)
+    SourceView* sourceView = findSourceViewRecursively(loc);
+    if (sourceView)
     {
-        return sourceUnit->getHumaneLoc(loc, type);
+        return sourceView->getHumaneLoc(loc, type);
     }
     else
     {
@@ -414,10 +441,10 @@ HumaneSourceLoc SourceManager::getHumaneLoc(SourceLoc loc, SourceLocType type)
 
 String SourceManager::getPath(SourceLoc loc, SourceLocType type)
 {
-    SourceUnit* sourceUnit = findSourceUnit(loc);
-    if (sourceUnit)
+    SourceView* sourceView = findSourceViewRecursively(loc);
+    if (sourceView)
     {
-        return sourceUnit->getPath(loc, type);
+        return sourceView->getPath(loc, type);
     }
     else
     {
diff --git a/source/slang/source-loc.h b/source/slang/source-loc.h
index 45d99ce3e..48b7791c7 100644
--- a/source/slang/source-loc.h
+++ b/source/slang/source-loc.h
@@ -13,12 +13,29 @@ namespace Slang {
 
 /** Overview: 
 
-SourceFile - Is the immutable contents of a file (or perhaps some generated source - say from doing a macro substitution)
-SourceUnit - Tracks a single parse of a SourceFile. Each SourceUnit defines a range of source locations used. If a SourceFile is parsed twice, two 
-SourceUnits are used, with unique SourceRanges - such that it is possible to tell which specific parse a SourceLoc is from. Not only that but the SourceUnit 
-contains the modifications of the interpretations of source (say by #line) directives. It is necessary to have these different 'views' on the 
-source because if a file is included twice, it may be used in different ways and have different #line directives.
-
+There needs to be a mechanism where we can easily and quickly track a specific locations in any source file used during a compilation. 
+This is important because that original location is meaningful to the user as it relates to their original source. Thus SourceLoc are 
+used so we can display meaningful and accurate errors/warnings as well as being able to always map generated code locations back to their origins.
+
+A 'SourceLoc' along with associated structures (SourceView, SourceFile, SourceMangager) this can pinpoint the location down to the byte across the 
+compilation. This could be achieved by storing for every token and instruction the file, line and column number came from. The SourceLoc is used in 
+lots of places - every AST node, every Token from the lexer, every IRInst - so we really want to make it small. So for this reason we actually 
+encode SourceLoc as a single integer and then use the associated structures when needed to determine what the location actually refers to - 
+the source file, line and column number, or in effect the byte in the original file.  
+
+Unfortunately there is extra complications. When a source is parsed it's interpretation (in terms of how a piece of source maps to an 'original' file etc)
+can be overridden - for example by using #line directives. Moreover a single source file can be parsed multiple times. When it's parsed multiple times the 
+interpretation of the mapping (#line directives for example) can change. This is the purpose of the SourceView - it holds the interpretation of a source file 
+for a specific Lex/Parse. 
+
+Another complication is that not all 'source' comes from SourceFiles, a macro expansion, may generate new 'source' we need to handle this, but also be able 
+to have a SourceLoc map to the expansion unambiguously. This is handled by creating a SourceFile and SourceView that holds only the macro generated 
+specific information.  
+
+SourceFile - Is the immutable text contents of a file (or perhaps some generated source - say from doing a macro substitution)
+SourceView - Tracks a single parse of a SourceFile. Each SourceView defines a range of source locations used. If a SourceFile is parsed twice, two 
+SourceViews are created, with unique SourceRanges. This is so that it is possible to tell which specific parse a SourceLoc is from - and so know the right
+interpretation for that lex/parse. 
 */
 
 class SourceLoc
@@ -104,14 +121,9 @@ public:
         /// Calculate the offset for a line
     int calcColumnIndex(int line, int offset);
 
-    // The logical file path to report for locations inside this span.
-    String path;
-
-    /// A blob that owns the storage for the file contents
-    ComPtr<ISlangBlob> contentBlob;
-
-    /// The actual contents of the file.
-    UnownedStringSlice content;
+    String path;                        ///< The logical file path to report for locations inside this span.
+    ComPtr<ISlangBlob> contentBlob;     ///< A blob that owns the storage for the file contents
+    UnownedStringSlice content;         ///< The actual contents of the file.
 
     protected:
     // In order to speed up lookup of line number information,
@@ -120,12 +132,10 @@ public:
     List<uint32_t> m_lineBreakOffsets;
 };
 
-struct SourceManager;
-
 enum class SourceLocType
 {
-    Normal,                 ///< Takes into account #line directives
-    Original,               ///< Ignores #line directives - humane location as seen in the actual file
+    Nominal,                ///< The normal interpretation which takes into account #line directives 
+    Actual,                 ///< Ignores #line directives - and is the location as seen in the actual file
 };
 
 // A source location in a format a human might like to see
@@ -134,17 +144,16 @@ struct HumaneSourceLoc
     String  path;
     Int     line = 0;
     Int     column = 0;
-
-    String const& getPath() const { return path; }
-    Int getLine() const { return line; }
-    Int getColumn() const { return column; }
 };
 
-/* A SourceUnit maps to a single span of SourceLoc range and is equivalent to a single include or use of a source file. 
+// Pre-declare
+struct SourceManager;
+
+/* A SourceView maps to a single span of SourceLoc range and is equivalent to a single include or more precisely use of a source file. 
 It is distinct from a SourceFile - because a SourceFile may be included multiple times, with different interpretations (depending 
-on #defines for example).s
+on #defines for example).
 */ 
-class SourceUnit: public RefObject
+class SourceView: public RefObject
 {
     public:
 
@@ -152,7 +161,7 @@ class SourceUnit: public RefObject
     // all map to the same logical file.
     struct Entry
     {
-            /// True if this resets the line numbering. It is distinct from a m_lineAdjust from 0, because it also means the path returns to the default.
+            /// True if this resets the line numbering. It is distinct from a m_lineAdjust being 0, because it also means the path returns to the default.
         bool isDefault() const { return m_pathHandle == StringSlicePool::Handle(0); }
 
         SourceLoc m_startLoc;                       ///< Where does this entry begin?
@@ -165,34 +174,36 @@ class SourceUnit: public RefObject
         /// into the underlying sourceFile.
     int findEntryIndex(SourceLoc sourceLoc) const;
 
-        /// Add a line directive for this unit. The directiveLoc must of course be in this SourceUnit
-        /// The path handle, must have been constructed on the SourceManager associated with the unit
+        /// Add a line directive for this view. The directiveLoc must of course be in this SourceView
+        /// The path handle, must have been constructed on the SourceManager associated with the view
         /// NOTE! Directives are assumed to be added IN ORDER during parsing such that every directiveLoc > previous 
     void addLineDirective(SourceLoc directiveLoc, StringSlicePool::Handle pathHandle, int line);
-
     void addLineDirective(SourceLoc directiveLoc, const String& path, int line);
 
         /// Removes any corrections on line numbers and reverts to the source files path
     void addDefaultLineDirective(SourceLoc directiveLoc);
 
-        /// Get the range that this unit applies to
+        /// Get the range that this view applies to
     const SourceRange& getRange() const { return m_range; }
         /// Get the entries
     const List<Entry>& getEntries() const { return m_entries; }
-        /// Get the source file holds the contents this 'unit' 
+        /// Get the source file holds the contents this view 
     SourceFile* getSourceFile() const { return m_sourceFile; }
         /// Get the source manager
     SourceManager* getSourceManager() const { return m_sourceManager; }
 
+        /// Get the associated 'content' (the source text)
+    const UnownedStringSlice& getContent() const { return m_sourceFile->content; }
+
         /// Get the humane location 
         /// Type determines if the location wanted is the original, or the 'normal' (which modifys behavior based on #line directives)
-    HumaneSourceLoc getHumaneLoc(SourceLoc loc, SourceLocType type = SourceLocType::Normal);
+    HumaneSourceLoc getHumaneLoc(SourceLoc loc, SourceLocType type = SourceLocType::Nominal);
 
         /// Get the path associated with a location
-    String getPath(SourceLoc loc, SourceLocType type = SourceLocType::Normal);
+    String getPath(SourceLoc loc, SourceLocType type = SourceLocType::Nominal);
 
         /// Ctor
-    SourceUnit(SourceManager* sourceManager, SourceFile* sourceFile, SourceRange range):
+    SourceView(SourceManager* sourceManager, SourceFile* sourceFile, SourceRange range):
         m_sourceManager(sourceManager),
         m_range(range),
         m_sourceFile(sourceFile)
@@ -202,39 +213,41 @@ class SourceUnit: public RefObject
     protected:
     
     SourceManager* m_sourceManager;     /// Get the manager this belongs to 
-    SourceRange m_range;                ///< The range that this SourceUnit applies to
+    SourceRange m_range;                ///< The range that this SourceView applies to
     RefPtr<SourceFile> m_sourceFile;    ///< The source file can hold the line breaks
     List<Entry> m_entries;              ///< An array entries describing how we should interpret a range, starting from the start location. 
 };
 
 struct SourceManager
 {
-    // Initialize a source manager, with an optional parent
-    void initialize(
-        SourceManager*  parent);
+        // Initialize a source manager, with an optional parent
+    void initialize(SourceManager*  parent);
 
+        /// Allocate a range of SourceLoc locations, these can be used to identify a specific location in the source
     SourceRange allocateSourceRange(UInt size);
 
-    SourceFile* newSourceFile(
-        String const&   path,
-        ISlangBlob*     content);
-
-    SourceFile* newSourceFile(
-        String const&   path,
-        String const&   content);
+        /// Create a SourceFile defined with the specified path, and content held within a blob
+    SourceFile* createSourceFile(String const& path, ISlangBlob* content);
+        /// Create a SourceFile with specified path. Create a Blob that contains the content.
+    SourceFile* createSourceFile(String const& path, String const& content);
 
         /// Get the humane source location
-    HumaneSourceLoc getHumaneLoc(SourceLoc loc, SourceLocType type = SourceLocType::Normal);
+    HumaneSourceLoc getHumaneLoc(SourceLoc loc, SourceLocType type = SourceLocType::Nominal);
 
         /// Get the path associated with a location 
-    String getPath(SourceLoc loc, SourceLocType type = SourceLocType::Normal);
+    String getPath(SourceLoc loc, SourceLocType type = SourceLocType::Nominal);
+
+        /// Create a new source view from a file
+    SourceView* createSourceView(SourceFile* sourceFile);
 
-        /// Allocate a new source unit from a file
-    SourceUnit* newSourceUnit(SourceFile* sourceFile);
+        /// Find a view by a source file location. 
+        /// If not found in this manager will look in the parent SourceManager
+        /// Returns nullptr if not found.
+    SourceView* findSourceViewRecursively(SourceLoc loc) const;
 
-        /// Find a unit by a source file location. If not found in this will look in the parent/
-        /// Returns nullptr if not found
-    SourceUnit* findSourceUnit(SourceLoc loc);
+        /// Find the SourceView associated with this manager for a specified location
+        /// Returns nullptr if not found. 
+    SourceView* findSourceView(SourceLoc loc) const;
 
         /// Searches this manager, and then the parent to see if can find a match for path. 
         /// If not found returns nullptr.    
@@ -246,27 +259,33 @@ struct SourceManager
         /// Get the slice pool
     StringSlicePool& getStringSlicePool() { return m_slicePool; }
 
+        /// Get the source range for just this manager
+        /// Caution - the range will change if allocations are made to this manager.
+    SourceRange getSourceRange() const { return SourceRange(m_startLoc, m_nextLoc); } 
+    
+        /// Get the parent manager to this manager. Returns nullptr if there isn't any.
+    SourceManager* getParent() const { return m_parent; }
+
+    protected:
+
     // The first location available to this source manager
     // (may not be the first location of all, because we might
     // have a parent source manager)
-    SourceLoc startLoc;
+    SourceLoc m_startLoc;
 
     // The "parent" source manager that owns locations ahead of `startLoc`
-    SourceManager* parent = nullptr;
+    SourceManager* m_parent = nullptr;
 
     // The location to be used by the next source file to be loaded
-    SourceLoc nextLoc;
-
-    protected:
+    SourceLoc m_nextLoc;
 
-    // All of the source units. These are held in increasing order of range, so can find by doing a binary chop.
-    List<RefPtr<SourceUnit> > m_sourceUnits;                
+    // All of the SourceViews. These are held in increasing order of range, so can find by doing a binary chop.
+    List<RefPtr<SourceView> > m_sourceViews;                
     StringSlicePool m_slicePool;
 
     Dictionary<String, RefPtr<SourceFile> > m_sourceFiles;
 };
 
-
 } // namespace Slang
 
 #endif