Fix handling of errors in imported modules (shader-slang#387)

* Fix handling of errors in imported modules

- If a semantic error is detected in an imported module, then don't try to generate IR code for it

- Also, if a module (transitively) imports itself, then report that as an error
  - The way I'm checking for this is a bit hacky (I'm adding the module to the map of loaded modules, but in an "unfinished" state, and then using that unfinished state to detect the import of a module already being imported).

This isn't a 100% complete solution for any of the related problems, but it improves the user experience for the common case.

* Remove #import test.

The feature is slated to be removed, so it isn't worth fixing up this test case.

Author: Tim Foley

source/slang/diagnostic-defs.h

@@ -257,6 +257,8 @@ DIAGNOSTIC(38102, Error, accessorMustBeInsideSubscriptOrProperty, "an accessor d
 DIAGNOSTIC(38020, Error, mismatchEntryPointTypeArgument, "expecting $0 entry-point type arguments, provided $1.")
 DIAGNOSTIC(38021, Error, typeArgumentDoesNotConformToInterface, "type argument `$1` for generic parameter `$0` does not conform to interface `$1`.")
 
+DIAGNOSTIC(38200, Error, recursiveModuleImport, "module `$0` recursively imports itself")
+
 //
 // 4xxxx - IL code generation.
 //

source/slang/slang.cpp

@@ -499,16 +499,31 @@ void CompileRequest::loadParsedModule(
     Name*                                   name,
     String const&                           path)
 {
+    // Note: we add the loaded module to our name->module listing
+    // before doing semantic checking, so that if it tries to
+    // recursively `import` itself, we can detect it.
+    RefPtr<LoadedModule> loadedModule = new LoadedModule();
+    mapPathToLoadedModule.Add(path, loadedModule);
+    mapNameToLoadedModules.Add(name, loadedModule);
+
+    int errorCountBefore = mSink.GetErrorCount();
     checkTranslationUnit(translationUnit.Ptr());
+    int errorCountAfter = mSink.GetErrorCount();
 
     RefPtr<ModuleDecl> moduleDecl = translationUnit->SyntaxNode;
-
-    RefPtr<LoadedModule> loadedModule = new LoadedModule();
     loadedModule->moduleDecl = moduleDecl;
-    loadedModule->irModule = generateIRForTranslationUnit(translationUnit);
 
-    mapPathToLoadedModule.Add(path, loadedModule);
-    mapNameToLoadedModules.Add(name, loadedModule);
+    if (errorCountAfter != errorCountBefore)
+    {
+        // There must have been an error in the loaded module.
+    }
+    else
+    {
+        // If we didn't run into any errors, then try to generate
+        // IR code for the imported module.
+        loadedModule->irModule = generateIRForTranslationUnit(translationUnit);
+    }
+
     loadedModulesList.Add(loadedModule);
 }
 
@@ -591,7 +606,16 @@ RefPtr<ModuleDecl> CompileRequest::findOrImportModule(
     // If so, return it.
     RefPtr<LoadedModule> loadedModule;
     if (mapNameToLoadedModules.TryGetValue(name, loadedModule))
-        return loadedModule->moduleDecl;
+    {
+        if (!loadedModule)
+            return nullptr;
+
+        if (!loadedModule->moduleDecl)
+        {
+            // We seem to be in the middle of loading this module
+            mSink.diagnose(loc, Diagnostics::recursiveModuleImport, name);
+        }
+    }
 
     // Derive a file name for the module, by taking the given
     // identifier, replacing all occurences of `_` with `-`,

tests/bugs/import-with-error-extra.slang

@@ -0,0 +1,11 @@
+//TEST_IGNORE_FILE:
+
+// This file exists to be imported.
+//
+// It contains a semantic error that should lead to compilation failure
+// in the module doing the importing.
+
+void broken()
+{
+	int a = b;
+}

tests/bugs/import-with-error.slang

@@ -0,0 +1,11 @@
+//TEST:SIMPLE:-use-ir
+
+// Confirm that we correctly issue a diagnostic when
+// we `import` a module that has some errors in it.
+
+import import_with_error_extra;
+
+void main()
+{
+	broken();
+}

tests/bugs/import-with-error.slang.expected

@@ -0,0 +1,6 @@
+result code = -1
+standard error = {
+tests/bugs/import-with-error-extra.slang(10): error 30015: undefined identifier 'b'.
+}
+standard output = {
+}

tests/diagnostics/recursive-import-extra.slang

@@ -0,0 +1,6 @@
+//TEST_IGNORE_FILE:
+
+// This file creates an `import` loop with
+// `recursive-import.slang`
+
+import recursive_import;

tests/diagnostics/recursive-import.slang

@@ -0,0 +1,6 @@
+//TEST:SIMPLE:
+
+// A file that recursively imports itself
+// (including transitive cases) should be diagnosed.
+
+import recursive_import_extra;

tests/diagnostics/recursive-import.slang.expected

@@ -0,0 +1,6 @@
+result code = -1
+standard error = {
+tests/diagnostics/recursive-import.slang(6): error 38200: module `recursive_import_extra` recursively imports itself
+}
+standard output = {
+}