From dbf5f413cd7a7b0448312a6f198b2a544087ac58 Mon Sep 17 00:00:00 2001
From: jsmall-nvidia <jsmall@nvidia.com>
Date: Thu, 10 Jan 2019 16:01:05 -0500
Subject: Improvements around review of debug serialization info (#769)

* * Make SourceView and SourceFile no longer derive from RefObject
* Both have life time now managed by SourceManager
* Tidied up a little around the serialization test code - just create the IRModule once

* Simplified code around deleting SourceView/File.

* Looked into generateIRForTranslationUnit - seems reasonable to just call it once, because it has side effects.
---
 source/slang/slang.cpp | 43 +++++++++++++++----------------------------
 1 file changed, 15 insertions(+), 28 deletions(-)

(limited to 'source/slang/slang.cpp')

diff --git a/source/slang/slang.cpp b/source/slang/slang.cpp
index 89b56e409..be6edc381 100644
--- a/source/slang/slang.cpp
+++ b/source/slang/slang.cpp
@@ -420,7 +420,7 @@ RefPtr<Expr> CompileRequest::parseTypeString(TranslationUnitRequest * translatio
     SourceManager localSourceManager;
     localSourceManager.initialize(sourceManager);
         
-    Slang::RefPtr<Slang::SourceFile> srcFile(localSourceManager.createSourceFileWithString(PathInfo::makeTypeParse(), typeStr));
+    Slang::SourceFile* srcFile = localSourceManager.createSourceFileWithString(PathInfo::makeTypeParse(), typeStr);
     
     // We'll use a temporary diagnostic sink  
     DiagnosticSink sink;
@@ -561,17 +561,15 @@ void CompileRequest::generateIR()
     // in isolation.
     for( auto& translationUnit : translationUnits )
     {
-        // TODO JS:
-        // This is a bit of HACK. Apparently if we call generateIRForTranslationUnit(translationUnit) twice
-        // we get a different result (!).
-        // So here, we only create once even if we run verification.
-        RefPtr<IRModule> irModule;
+        // We want to only run generateIRForTranslationUnit once here. This is for two side effects:
+        // * it can dump ir 
+        // * it can generate diagnostics
+
+        /// Generate IR for translation unit
+        RefPtr<IRModule> irModule(generateIRForTranslationUnit(translationUnit));
 
         if (verifyDebugSerialization)
         {
-            /// Generate IR for translation unit
-            irModule = generateIRForTranslationUnit(translationUnit);
-
             // Verify debug information
             if (SLANG_FAILED(IRSerialUtil::verifySerialize(irModule, mSession, sourceManager, IRSerialBinary::CompressionType::None, IRSerialWriter::OptionFlag::DebugInfo)))
             {
@@ -583,16 +581,11 @@ void CompileRequest::generateIR()
         {              
             IRSerialData serialData;
             {
-                /// Generate IR for translation unit
-                if (!irModule)
-                {
-                    irModule = generateIRForTranslationUnit(translationUnit);
-                }
-
                 // Write IR out to serialData - copying over SourceLoc information directly
                 IRSerialWriter writer;
                 writer.write(irModule, sourceManager, IRSerialWriter::OptionFlag::RawSourceLocation, &serialData);
 
+                // Destroy irModule such that memory can be used for newly constructed read irReadModule  
                 irModule = nullptr;
             }
             RefPtr<IRModule> irReadModule;
@@ -602,18 +595,12 @@ void CompileRequest::generateIR()
                 reader.read(serialData, mSession, nullptr, irReadModule);
             }
 
-            // Use the serialized irModule
-            translationUnit->irModule = irReadModule;
+            // Set irModule to the read module
+            irModule = irReadModule;
         }
-        else
-        {
-            if (!irModule)
-            {
-                irModule = generateIRForTranslationUnit(translationUnit);
-            }
 
-            translationUnit->irModule = irModule; 
-        }
+        // Set the module on the translation unit
+        translationUnit->irModule = irModule;
     }
 }
 
@@ -779,7 +766,7 @@ void CompileRequest::addTranslationUnitSourceBlob(
     ISlangBlob*     sourceBlob)
 {
     PathInfo pathInfo = PathInfo::makePath(path);
-    RefPtr<SourceFile> sourceFile = getSourceManager()->createSourceFileWithBlob(pathInfo, sourceBlob);
+    SourceFile* sourceFile = getSourceManager()->createSourceFileWithBlob(pathInfo, sourceBlob);
     
     addTranslationUnitSourceFile(translationUnitIndex, sourceFile);
 }
@@ -790,7 +777,7 @@ void CompileRequest::addTranslationUnitSourceString(
     String const&   source)
 {
     PathInfo pathInfo = PathInfo::makePath(path);
-    RefPtr<SourceFile> sourceFile = getSourceManager()->createSourceFileWithString(pathInfo, source);
+    SourceFile* sourceFile = getSourceManager()->createSourceFileWithString(pathInfo, source);
     
     addTranslationUnitSourceFile(translationUnitIndex, sourceFile);
 }
@@ -915,7 +902,7 @@ RefPtr<ModuleDecl> CompileRequest::loadModule(
     translationUnit->compileFlags = 0;
 
     // Create with the 'friendly' name
-    RefPtr<SourceFile> sourceFile = getSourceManager()->createSourceFileWithBlob(filePathInfo, sourceBlob);
+    SourceFile* sourceFile = getSourceManager()->createSourceFileWithBlob(filePathInfo, sourceBlob);
     
     translationUnit->sourceFiles.Add(sourceFile);
 
-- 
cgit v1.2.3