improving handling of extend cycles and reloading (extended) modules with (temporary) errors

src/org/rascalmpl/ideservices/BasicIDEServices.java

@@ -26,7 +26,6 @@
 import org.rascalmpl.library.Messages;
 import org.rascalmpl.uri.URIResolverRegistry;
 import org.rascalmpl.uri.URIUtil;
-
 import io.usethesource.vallang.IInteger;
 import io.usethesource.vallang.IList;
 import io.usethesource.vallang.ISourceLocation;
@@ -153,7 +152,7 @@ public void jobTodo(String name, int work) {
 
   @Override
   public void warning(String message, ISourceLocation src) {
-    monitor.warning(message,  src);
+      monitor.warning(message,  src);
   }
 
   @Override

src/org/rascalmpl/interpreter/Evaluator.java

@@ -1120,6 +1120,7 @@ public void doImport(IRascalMonitor monitor, String... string) {
             monitor.jobEnd(LOADING_JOB_CONSTANT, true);
             setMonitor(old);
             setCurrentAST(null);
+            heap.writeLoadMessages(getErrorPrinter());
         }
     }
 
@@ -1210,7 +1211,6 @@ private void reloadModules(IRascalMonitor monitor, Set<String> names, ISourceLoc
                                 affectedModules.add(mod);
                             }
                             else {
-                                errStream.println("Could not reimport" + imp + " at " + errorLocation);
                                 warning("could not reimport " + imp, errorLocation);
                             }
                         }
@@ -1229,7 +1229,6 @@ private void reloadModules(IRascalMonitor monitor, Set<String> names, ISourceLoc
                                 env.addExtend(ext);
                             }
                             else {
-                                errStream.println("Could not re-extend" + ext + " at " + errorLocation);
                                 warning("could not re-extend " + ext, errorLocation);
                             }
                         }

src/org/rascalmpl/interpreter/env/GlobalEnvironment.java

@@ -14,9 +14,9 @@
 *******************************************************************************/
 package org.rascalmpl.interpreter.env;
 
+import java.io.PrintWriter;
 import java.net.URI;
 import java.util.ArrayDeque;
-import java.util.Collections;
 import java.util.Deque;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -31,6 +31,11 @@
 import org.rascalmpl.interpreter.result.ICallableValue;
 import org.rascalmpl.interpreter.staticErrors.UndeclaredModule;
 import org.rascalmpl.interpreter.utils.Names;
+import org.rascalmpl.values.IRascalValueFactory;
+
+import io.usethesource.capsule.SetMultimap;
+import io.usethesource.vallang.IList;
+import io.usethesource.vallang.IString;
 
 
 /**
@@ -40,44 +45,57 @@
  * 
  */
 public class GlobalEnvironment {
+	private final Deque<String> loadStack = new ArrayDeque<>();
+
 	/** The heap of Rascal */
 	private final HashMap<String, ModuleEnvironment> moduleEnvironment = new HashMap<String, ModuleEnvironment>();
 
 	/** Keeping track of module locations */
 	private final HashMap<String, URI> moduleLocations = new HashMap<String,URI>();
 	private final HashMap<URI, String> locationModules = new HashMap<URI,String>();
 
-	/**
-	 * For extend cycle detector
-	 */
-	private final Deque<String> extendStack = new ArrayDeque<>();
-
 	/**
 	 * Source location resolvers map user defined schemes to primitive schemes
 	 */
 	private final HashMap<String, ICallableValue> sourceResolvers = new HashMap<String, ICallableValue>();
 
 	private boolean bootstrapper;
-
-	public void pushExtend(String module) {
-	    extendStack.push(module);
-	}
-
-	public void popExtend() {
-	    extendStack.pop();
-	}
-
-	public boolean isCyclicExtend(String module) {
-	    return extendStack.contains(module);
-	}
-
+
 	public void clear() {
 		moduleEnvironment.clear();
 		moduleLocations.clear();
 		locationModules.clear();
 		sourceResolvers.clear();
 	}
 
+	/**
+	 * Push a module on the load stack and
+	 * return true iff its already on the stack.
+	 * @param name
+	 * @return
+	 */
+	public boolean pushModuleLoading(String name) {
+		var preExists = loadStack.contains(name);
+		loadStack.push(name);
+		return preExists;
+	}
+
+	public String popModuleLoading() {
+		return loadStack.pop();
+	}
+
+	public boolean onLoadingStack(String name) {
+		return loadStack.contains(name);
+	}
+
+	public void writeLoadMessages(PrintWriter out) {
+		moduleEnvironment.values().stream().forEach(m -> m.writeLoadMessages(out));
+	}
+
+	public List<String> getLoadStack() {
+		return loadStack.stream().collect(Collectors.toList());
+	}
+
 	/**
 	 * Register a source resolver for a specific scheme. Will overwrite the previously
 	 * registered source resolver for this scheme.
@@ -197,7 +215,11 @@ public Set<String> getImportingModules(String mod) {
 
 		return result;
 	}
-
+
+	public SetMultimap.Transient<String, String> getExtendGraphFrom(String mod) {
+		return getModule(mod).collectExtendsGraph();
+	}
+
 	public Set<String> getExtendingModules(String mod) {
 		Set<String> result = new HashSet<>();
 		Deque<String> todo = new ArrayDeque<>();
@@ -240,8 +262,63 @@ public boolean isBootstrapper() {
 	    return bootstrapper;
 	}
 
-	public List<String> getExtendCycle() {
-        return Collections.unmodifiableList(extendStack.stream().collect(Collectors.toList()));
+	/**
+	 * This starts with an edge that is not yet in the graph because we are loading these
+	 * modules. Then the rest of graph is loaded from what we already have in memory.
+	 * @param parent the first and last node of a detected cycle.
+	 * @return
+	 */
+    public List<String> findCyclicExtendPathFrom(String parent, String child) {
+        SetMultimap.Transient<String, String> graph = getExtendGraphFrom(child);
+		graph.__put(parent, child); // add the last edge that was not yet registered
+		IRascalValueFactory vf = IRascalValueFactory.getInstance();
+		return depthFirstCycleSearch(parent, new HashSet<>(), vf.list(vf.string(parent)), graph)
+			.stream()
+			.map(IString.class::cast)
+			.map(IString::getValue)
+			.collect(Collectors.toList());
+    }
+
+	/*
+	 * dfs uses an immutable IList for the path such that we don't have to maintain a stack, next to the 
+	 * recursion.
+	 * otherwise this is a standard depth-first search for a directed graph
+	 */
+	public IList depthFirstCycleSearch(String parent, Set<String> visited, IList path,  SetMultimap.Transient<String, String> graph) {
+		visited.add(parent);
+		var vf = IRascalValueFactory.getInstance();
+
+		for (String child : graph.get(parent)) {
+			if (!visited.contains(child)) {
+				// a new node but not cyclic (yet) 
+				IList result = depthFirstCycleSearch(child, visited, path.append(vf.string(child)), graph);
+				if (!result.isEmpty()) {
+					// then this is the first cycle we found, so we're done
+					return result;
+				}
+				else {
+					continue; // searching with the other nodes
+				}
+			}
+			else if (path.contains(vf.string(child))) {
+				// cycle detected, drop the prefix and report it!
+				return path.stream().dropWhile(e -> !e.equals(vf.string(child))).collect(vf.listWriter());
+			}
+			else {
+				// already visited but not cyclic
+				continue; // with the neighbors
+			}
+		}
+
+		// no cycle found here
+		return vf.list();
+	}
+
+	public List<String> nonInitializedModules() {
+		return moduleEnvironment.entrySet().stream()
+			.filter(e -> !e.getValue().isInitialized())
+			.map(e -> e.getKey())
+			.collect(Collectors.toList());
 	}
 
 	public void clearLookupChaches() {

src/org/rascalmpl/interpreter/env/ModuleEnvironment.java

@@ -17,6 +17,7 @@
 *******************************************************************************/
 package org.rascalmpl.interpreter.env;
 
+import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -43,16 +44,21 @@
 import org.rascalmpl.interpreter.result.Result;
 import org.rascalmpl.interpreter.staticErrors.UndeclaredModule;
 import org.rascalmpl.interpreter.utils.Names;
+import org.rascalmpl.library.Messages;
 import org.rascalmpl.types.NonTerminalType;
 import org.rascalmpl.types.RascalTypeFactory;
 import org.rascalmpl.uri.URIUtil;
+import org.rascalmpl.values.IRascalValueFactory;
 import org.rascalmpl.values.RascalValueFactory;
 import org.rascalmpl.values.ValueFactoryFactory;
 
+import io.usethesource.capsule.SetMultimap;
+import io.usethesource.capsule.core.PersistentTrieSetMultimap;
 import io.usethesource.vallang.IConstructor;
 import io.usethesource.vallang.IMap;
 import io.usethesource.vallang.IMapWriter;
 import io.usethesource.vallang.ISetWriter;
+import io.usethesource.vallang.ISourceLocation;
 import io.usethesource.vallang.ITuple;
 import io.usethesource.vallang.IValue;
 import io.usethesource.vallang.IValueFactory;
@@ -65,9 +71,6 @@
  * A module environment represents a module object (i.e. a running module).
  * It manages imported modules and visibility of the
  * functions and variables it declares. 
- * 
- * TODO: add management of locally declared types and constructors
- * 
  */
 public class ModuleEnvironment extends Environment {
 	private final GlobalEnvironment heap;
@@ -82,6 +85,7 @@ public class ModuleEnvironment extends Environment {
 	private boolean syntaxDefined;
 	private boolean bootstrap;
 	private String deprecated;
+	private List<IConstructor> loadMessages = new LinkedList<>();
 	private Map<String, AbstractFunction> resourceImporters;
 	private Map<Type, Set<GenericKeywordParameters>> cachedGeneralKeywordParameters;
 	private Map<String, List<AbstractFunction>> cachedPublicFunctions;
@@ -118,6 +122,7 @@ public void reset() {
 		this.bootstrap = false;
 		this.extended = new HashSet<String>();
 		this.deprecated = null;
+		this.loadMessages.clear();
 		this.generalKeywordParameters = new HashMap<>();
 		this.cachedGeneralKeywordParameters = null;
 		this.cachedPublicFunctions = null;
@@ -233,6 +238,22 @@ public void clearProductions() {
 			productions.clear();
 		}
 	}
+
+	public void addLoadError(String message, ISourceLocation loc, String trace) {
+		loadMessages.add(Messages.addCause(Messages.error(message, loc), trace, loc));
+	}
+
+	public void addLoadWarning(String message, ISourceLocation loc) {
+		loadMessages.add(Messages.warning(message, loc));
+	}
+
+	public void addLoadInfo(String message, ISourceLocation loc) {
+		loadMessages.add(Messages.info(message, loc));
+	}
+
+	public void writeLoadMessages(PrintWriter out) {
+		Messages.write(loadMessages.stream().collect(IRascalValueFactory.getInstance().listWriter()), out);
+	}
 
 	public boolean definesSyntax() {
 		if (!productions.isEmpty()) {
@@ -409,18 +430,22 @@ public Set<String> getImportsTransitive() {
 	}
 
 	public void unImport(String moduleName) {
-		var old = importedModules.remove(moduleName);
-		if (old != null && old.isPresent()) {
-			typeStore.unimportStores(old.get().getStore());
+		if (importedModules != null) {
+			var old = importedModules.remove(moduleName);
+			if (old != null && old.isPresent()) {
+				typeStore.unimportStores(old.get().getStore());
+			}
 		}
 		cachedGeneralKeywordParameters = null;
 		cachedPublicFunctions = null;
 	}
 
 	public void unExtend(String moduleName) {
-		extended.remove(moduleName);
-		cachedGeneralKeywordParameters = null;
-		cachedPublicFunctions = null;
+		if (extended != null) {
+			extended.remove(moduleName);
+		}
+
+		clearLookupCaches();
 	}
 
 	@Override
@@ -475,12 +500,7 @@ public Result<IValue> getVariable(QualifiedName name) {
 
 
 	@Override
-	public void storeVariable(String name, Result<IValue> value) {
-//		if (value instanceof AbstractFunction) {
-//			storeFunction(name, (AbstractFunction) value);
-//			return;
-//		}
-
+	public void storeVariable(String name, Result<IValue> value) {		
 		Result<IValue> result = super.getFrameVariable(name);
 
 		if (result != null) {
@@ -1118,6 +1138,34 @@ public Set<String> getExtends() {
 		return Collections.<String>emptySet();
 	}
 
+	/**
+	 * This collects only "extends" edges starting from the current module
+	 * @return
+	 */
+	public SetMultimap.Transient<String, String> collectExtendsGraph() {
+		List<String> todo = new LinkedList<String>();
+		Set<String> done = new HashSet<String>();
+		SetMultimap.Transient<String, String> result = PersistentTrieSetMultimap.transientOf();
+		todo.add(this.getName());
+		GlobalEnvironment heap = getHeap();
+
+		while (!todo.isEmpty()) {
+		   String mod = todo.remove(0);	
+		   done.add(mod);
+		   ModuleEnvironment env = heap.getModule(mod);
+		   if (env != null) {
+			  for (String e : env.getExtends()) {
+				  result.__put(mod, e);
+				  if (!done.contains(e)) {
+					  todo.add(e);
+				  }
+			  }
+		   }
+		}
+
+		return result;
+	}
+
 	public Set<String> getExtendsTransitive() {
 		List<String> todo = new LinkedList<String>();
 		Set<String> done = new HashSet<String>();

src/org/rascalmpl/interpreter/staticErrors/CyclicExtend.java

@@ -19,8 +19,14 @@
 
 public class CyclicExtend extends StaticError {
     private static final long serialVersionUID = -7673780054774870139L;
+    private final List<ISourceLocation> cycle;
 
-    public CyclicExtend(String name, List<String> cycle, ISourceLocation loc) {
-		super("Extend cycle detected: " + cycle.stream().collect(Collectors.joining(", ")), loc);
-	}
+    public CyclicExtend(String name, List<ISourceLocation> cycle, ISourceLocation loc) {
+        super("Extend cycle detected (" + name + "):\n* " + cycle.stream().map(Object::toString).collect(Collectors.joining("\n* ")), loc);
+        this.cycle = cycle;
+    }
+
+    public List<ISourceLocation> getCycle() {
+        return cycle;
+    }
 }