Improve circular import error message (#450)

spicelang · Feb 4, 2024 · 46fdc70 · 46fdc70
1 parent 8b5b8a1
commit 46fdc70
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 7 deletions.
diff --git a/src/SourceFile.cpp b/src/SourceFile.cpp
@@ -534,8 +534,13 @@ void SourceFile::runBackEnd() { // NOLINT(misc-no-recursion)
 void SourceFile::addDependency(SourceFile *sourceFile, const ASTNode *declNode, const std::string &dependencyName,
                                const std::string &path) {
   // Check if this would cause a circular dependency
-  if (isAlreadyImported(path))
-    throw SemanticError(declNode, CIRCULAR_DEPENDENCY, "Circular import detected while importing '" + sourceFile->fileName + "'");
+  std::vector<const SourceFile *> dependencyCircle;
+  if (isAlreadyImported(path, dependencyCircle)) {
+    std::stringstream errorMessage;
+    errorMessage << "Circular import detected while importing '" << sourceFile->fileName << "':\n\n";
+    errorMessage << CommonUtil::getCircularImportMessage(dependencyCircle);
+    throw SemanticError(declNode, CIRCULAR_DEPENDENCY, errorMessage.str());
+  }
 
   // Add the dependency
   sourceFile->mainFile = false;
@@ -549,12 +554,14 @@ bool SourceFile::imports(const SourceFile *sourceFile) const {
   return std::ranges::any_of(dependencies, [=](const auto &dependency) { return dependency.second == sourceFile; });
 }
 
-bool SourceFile::isAlreadyImported(const std::string &filePathSearch) const { // NOLINT(misc-no-recursion)
+bool SourceFile::isAlreadyImported(const std::string &filePathSearch,
+                                   std::vector<const SourceFile *> &circle) const { // NOLINT(misc-no-recursion)
+  circle.push_back(this);
   // Check if the current source file corresponds to the path to search
   if (std::filesystem::equivalent(filePath, filePathSearch))
     return true;
   // Check parent recursively
-  return parent != nullptr && parent->isAlreadyImported(filePathSearch);
+  return parent != nullptr && parent->isAlreadyImported(filePathSearch, circle);
 }
 
 SourceFile *SourceFile::requestRuntimeModule(RuntimeModule runtimeModule) {

diff --git a/src/SourceFile.h b/src/SourceFile.h
@@ -140,7 +140,7 @@ class SourceFile {
   // Public methods
   void addDependency(SourceFile *sourceFile, const ASTNode *declNode, const std::string &dependencyName, const std::string &path);
   [[nodiscard]] bool imports(const SourceFile *sourceFile) const;
-  [[nodiscard]] bool isAlreadyImported(const std::string &filePathSearch) const;
+  [[nodiscard]] bool isAlreadyImported(const std::string &filePathSearch, std::vector<const SourceFile *> &circle) const;
   SourceFile *requestRuntimeModule(RuntimeModule runtimeModule);
   bool isRuntimeModuleAvailable(RuntimeModule runtimeModule) const;
   void addNameRegistryEntry(const std::string &symbolName, uint64_t typeId, SymbolTableEntry *entry, Scope *scope,

diff --git a/src/typechecker/InterfaceManager.cpp b/src/typechecker/InterfaceManager.cpp
@@ -26,7 +26,7 @@ Interface *InterfaceManager::insertSubstantiation(Scope *insertScope, Interface
   const std::string signature = newManifestation.getSignature();
 
   // Make sure that the manifestation does not exist already
-  for (const auto &manifestations : insertScope->interfaces)
+  for ([[maybe_unused]] const auto &manifestations : insertScope->interfaces)
     assert(!manifestations.second.contains(newManifestation.getSignature()));
 
   // Retrieve the matching manifestation list of the scope

diff --git a/src/util/CommonUtil.cpp b/src/util/CommonUtil.cpp
@@ -4,6 +4,8 @@
 
 #include <cxxabi.h>
 
+#include <SourceFile.h>
+
 #ifdef OS_WINDOWS
 #include <windows.h>
 #elif OS_UNIX
@@ -101,6 +103,26 @@ bool CommonUtil::isValidMangledName(const std::string &mangledName) {
   return status == 0;
 }
 
+/**
+ * Generate a circular import message from the given source files
+ *
+ * @param sourceFiles Source files building the circular dependency chain
+ * @return Error message
+ */
+std::string CommonUtil::getCircularImportMessage(const std::vector<const SourceFile *> &sourceFiles) {
+  std::stringstream message;
+  message << "*-----*\n";
+  message << "|     |\n";
+  for (size_t i = 0; i < sourceFiles.size(); i++) {
+    if (i != 0)
+      message << "|     |\n";
+    message << "|  " << sourceFiles.at(i)->fileName.c_str() << "\n";
+  }
+  message << "|     |\n";
+  message << "*-----*";
+  return message.str();
+}
+
 /**
  * Generate the version info string for the Spice driver
  *

diff --git a/src/util/CommonUtil.h b/src/util/CommonUtil.h
@@ -8,6 +8,9 @@
 
 namespace spice::compiler {
 
+// Forward declarations
+class SourceFile;
+
 /**
  * Util for general simplification of tasks
  */
@@ -19,6 +22,7 @@ class CommonUtil {
   static std::vector<std::string> split(const std::string &input);
   static size_t getSystemPageSize();
   static bool isValidMangledName(const std::string &mangledName);
+  static std::string getCircularImportMessage(const std::vector<const SourceFile *> &sourceFiles);
   static std::string getVersionInfo();
 };
 

diff --git a/test/test-files/typechecker/imports/error-circular-import/exception.out b/test/test-files/typechecker/imports/error-circular-import/exception.out
@@ -1,5 +1,15 @@
 [Error|Semantic] ../source2.spice:1:1:
-Circular import detected: Circular import detected while importing 'source.spice'
+Circular import detected: Circular import detected while importing 'source.spice':
+
+*-----*
+|     |
+|  source2.spice
+|     |
+|  source1.spice
+|     |
+|  source.spice
+|     |
+*-----*
 
 1  import "source" as s;
    ^^^^^^^^^^^^^^^^^^^^^