Handle public imports MUCH more efficiently. Fixes #160

makefile

@@ -2,6 +2,7 @@
 
 all: dmd
 dmd: dmdserver dmdclient
+debug: dmdclient debugserver
 gdc: gdcserver gdcclient
 ldc: ldcserver ldcclient
 
@@ -83,6 +84,13 @@ DMD_SERVER_FLAGS = -Icontainers/src\
 	-inline\
 	-ofbin/dcd-server
 
+DEBUG_SERVER_FLAGS = -Icontainers/src\
+	-Imsgpack-d/src\
+	-Ilibdparse/src\
+	-wi\
+	-g\
+	-ofbin/dcd-server
+
 GDC_SERVER_FLAGS =  -Icontainers/src\
 	-Imsgpack-d/src\
 	-Ilibdparse/src\
@@ -108,6 +116,12 @@ dmdserver:
 	rm -f containers/src/std/allocator.d
 	${DMD} ${SERVER_SRC} ${DMD_SERVER_FLAGS}
 
+debugserver:
+	mkdir -p bin
+	rm -f containers/src/std/allocator.d
+	${DMD} ${SERVER_SRC} ${DEBUG_SERVER_FLAGS}
+
+
 gdcclient:
 	mkdir -p bin
 	rm -f containers/src/std/allocator.d

sonar-project.properties

@@ -2,4 +2,4 @@ sonar.projectKey=dcd
 sonar.projectName=DCD
 sonar.projectVersion=1.0
 sonar.sourceEncoding=UTF-8
-sonar.sources=.
+sonar.sources=src

src/actypes.d

@@ -21,7 +21,6 @@ module actypes;
 import std.algorithm;
 import std.array;
 import std.container;
-//import std.stdio;
 import std.typecons;
 import std.allocator;
 
@@ -55,6 +54,9 @@ struct ACSymbol
 {
 public:
 
+	/**
+	 * Copying is disabled.
+	 */
 	@disable this();
 
 	/**
@@ -115,12 +117,18 @@ public:
 	 */
 	ACSymbol*[] getPartsByName(string name)
 	{
+		import std.range : chain;
 		ACSymbol s = ACSymbol(name);
-		auto er = parts.equalRange(&s);
+		ACSymbol p = ACSymbol(IMPORT_SYMBOL_NAME);
-		if (er.empty)
+		auto app = appender!(ACSymbol*[])();
-			return array(aliasThisParts.equalRange(&s));
+		foreach (part; parts.equalRange(&s))
-		else
+			app.put(part);
-			return array(er);
+		foreach (extra; extraSymbols.equalRange(&s))
+			app.put(extra.getPartsByName(name));
+		foreach (im; parts.equalRange(&p))
+			foreach (part; im.type.parts.equalRange(&s))
+				app.put(part);
+		return app.data();
 	}
 
 	/**
@@ -231,11 +239,16 @@ struct Scope
 		if (s is null)
 			return [];
 		UnrolledList!(ACSymbol*) symbols;
-		symbols.insert(s.symbols[]);
+		Scope* sc = s;
-		Scope* sc = s.parent;
 		while (sc !is null)
 		{
-			symbols.insert(sc.symbols[]);
+			foreach (item; sc.symbols[])
+			{
+				if (item.kind == CompletionKind.importSymbol) foreach (i; item.type.parts[])
+					symbols.insert(i);
+				else
+					symbols.insert(item);
+			}
 			sc = sc.parent;
 		}
 		return array(symbols[]);
@@ -253,7 +266,18 @@ struct Scope
 		ACSymbol s = ACSymbol(name);
 		auto er = symbols.equalRange(&s);
 		if (!er.empty)
-			return cast(typeof(return)) array(er);
+			return array(er);
+		ACSymbol ir = ACSymbol(IMPORT_SYMBOL_NAME);
+		auto r = symbols.equalRange(&ir);
+		if (!r.empty)
+		{
+			auto app = appender!(ACSymbol*[])();
+			foreach (e; r)
+				foreach (importedSymbol; e.type.parts.equalRange(&s))
+					app.put(importedSymbol);
+			if (app.data.length > 0)
+				return app.data;
+		}
 		if (parent is null)
 			return [];
 		return parent.getSymbolsByName(name);
@@ -275,6 +299,9 @@ struct Scope
 		return s.getSymbolsByName(name);
 	}
 
+	/**
+	 * Returns an array of symbols that are present at global scope
+	 */
 	ACSymbol*[] getSymbolsAtGlobalScope(string name)
 	{
 		if (parent !is null)
@@ -344,6 +371,16 @@ TTree!(ACSymbol*, true, "a < b", false) classSymbols;
 
 private immutable(string[24]) builtinTypeNames;
 
+/**
+ * Name given to the public import symbol. Its value is "public" because this
+ * is not a valid identifier. This is initialized to an interned string during
+ * static construction.
+ */
+immutable string IMPORT_SYMBOL_NAME;
+
+/**
+ * Translates the IDs for built-in types into an interned string.
+ */
 string getBuiltinTypeName(IdType id)
 {
 	switch (id)
@@ -407,6 +444,8 @@ static this()
 	builtinTypeNames[22] = internString("cfloat");
 	builtinTypeNames[23] = internString("creal");
 
+	IMPORT_SYMBOL_NAME = internString("public");
+
 
 	auto bool_ = allocate!ACSymbol(Mallocator.it, "bool", CompletionKind.keyword);
 	auto int_ = allocate!ACSymbol(Mallocator.it, "int", CompletionKind.keyword);

src/autocomplete.d

@@ -295,6 +295,7 @@ AutocompleteResponse parenCompletion(T)(T beforeTokens,
 
 bool isSelectiveImport(T)(T tokens)
 {
+	assert (tokens.length > 1);
 	size_t i = tokens.length - 1;
 	if (!(tokens[i] == tok!":" || tokens[i] == tok!","))
 		return false;
@@ -391,7 +392,7 @@ body
 	auto symbols = ModuleCache.getSymbolsInModule(ModuleCache.resolveImportLoctation(path));
 	import containers.hashset;
 	HashSet!string h;
-	foreach (s; symbols)
+	foreach (s; symbols.parts[])
 	{
 		auto a = ACSymbol(s.name);
 		if (!builtinSymbols.contains(&a) && !h.contains(s.name))
@@ -630,7 +631,8 @@ void setCompletions(T)(ref AutocompleteResponse response,
 		}
 		TTree!(ACSymbol*, true, "a < b", false) parts;
 		parts.insert(symbols[0].parts[]);
-		parts.insert(symbols[0].aliasThisParts[]);
+		foreach (s; symbols[0].extraSymbols[])
+			parts.insert(s.parts[]);
 		foreach (s; parts[].filter!(a => a.name !is null
 			&& a.name.length > 0 && a.name[0] != '*'
 			&& (partial is null ? true : a.name.toUpper().startsWith(partial.toUpper()))

src/conversion/first.d

@@ -292,12 +292,11 @@ final class FirstPass : ASTVisitor
 		rootSymbol = currentSymbol;
 		currentScope = allocate!Scope(semanticAllocator, 0, size_t.max);
 		auto i = allocate!ImportInformation(semanticAllocator);
-		i.modulePath = "object";
+		i.modulePath = internString("object");
-		i.importParts.insert("object");
+		i.importParts.insert(i.modulePath);
 		currentScope.importInformation.insert(i);
 		moduleScope = currentScope;
 		mod.accept(this);
-		assert (currentSymbol.acSymbol.name is null);
 	}
 
 	override void visit(const EnumDeclaration dec)
@@ -328,10 +327,7 @@ final class FirstPass : ASTVisitor
 	override void visit(const ModuleDeclaration moduleDeclaration)
 	{
 //		Log.trace(__FUNCTION__, " ", typeof(dec).stringof);
-		foreach (identifier; moduleDeclaration.moduleName.identifiers)
+		rootSymbol.acSymbol.name = internString(moduleDeclaration.moduleName.identifiers[$ - 1].text);
-		{
-			moduleName.insert(internString(identifier.text));
-		}
 	}
 
 	override void visit(const StructBody structBody)
@@ -616,9 +612,6 @@ private:
 	/// Current protection type
 	IdType protection;
 
-	/// Package and module name
-	UnrolledList!string moduleName;
-
 	/// Current scope
 	Scope* currentScope;
 

src/conversion/second.d

@@ -24,6 +24,7 @@ import semantic;
 import messages;
 import std.allocator;
 import stupidlog;
+import string_interning;
 
 /**
  * Second pass handles the following:
@@ -68,32 +69,35 @@ private:
 		}
 	}
 
+	/**
+	 * Creates package symbols as necessary to contain the given module symbol
+	 */
 	ACSymbol* createImportSymbols(ImportInformation* info, Scope* currentScope,
-		ACSymbol*[] moduleSymbols)
+		ACSymbol* moduleSymbol)
 	in
 	{
 		assert (info !is null);
-		foreach (s; moduleSymbols)
+		assert (moduleSymbol !is null);
-			assert (s !is null);
 	}
 	body
 	{
 		immutable string firstPart = info.importParts[].front;
-//		Log.trace("firstPart = ", firstPart);
 		ACSymbol*[] symbols = currentScope.getSymbolsByName(firstPart);
-		immutable bool found = symbols.length > 0;
+		ACSymbol* firstSymbol = void;
-		ACSymbol* firstSymbol = found
+		if (symbols.length > 0)
-			? symbols[0] : allocate!ACSymbol(symbolAllocator, firstPart,
+			firstSymbol = symbols[0];
+		else
+			firstSymbol = allocate!ACSymbol(symbolAllocator, firstPart,
 				CompletionKind.packageName);
-		if (!found)
 		currentScope.symbols.insert(firstSymbol);
-//		Log.trace(firstSymbol.name);
 		ACSymbol* currentSymbol = firstSymbol;
 		size_t i = 0;
 		foreach (string importPart; info.importParts[])
 		{
 			if (i++ == 0)
 				continue;
+			if (i + 2 >= info.importParts.length) // Skip the last item as it's the module name
+				break;
 			symbols = currentSymbol.getPartsByName(importPart);
 			ACSymbol* s = symbols.length > 0
 				? cast(ACSymbol*) symbols[0] : allocate!ACSymbol(symbolAllocator,
@@ -101,9 +105,7 @@ private:
 			currentSymbol.parts.insert(s);
 			currentSymbol = s;
 		}
-		currentSymbol.kind = CompletionKind.moduleName;
+		currentSymbol.parts.insert(moduleSymbol);
-		currentSymbol.parts.insert(moduleSymbols);
-//		Log.trace(currentSymbol.name);
 		return currentSymbol;
 	}
 
@@ -114,19 +116,21 @@ private:
 		foreach (importInfo; currentScope.importInformation[])
 		{
 			string location = ModuleCache.resolveImportLoctation(importInfo.modulePath);
-			ACSymbol*[] symbols = location is null ? [] : ModuleCache.getSymbolsInModule(location);
+			ACSymbol* symbol = location is null ? null : ModuleCache.getSymbolsInModule(location);
-			ACSymbol* moduleSymbol = createImportSymbols(importInfo, currentScope, symbols);
+			if (symbol is null)
-			currentScope.symbols.insert(moduleSymbol);
+				continue;
-			currentScope.symbols.insert(symbols);
+			ACSymbol* moduleSymbol = createImportSymbols(importInfo, currentScope, symbol);
+			currentScope.symbols.insert(symbol.parts[]);
 			if (importInfo.importedSymbols.length == 0)
 			{
 				if (importInfo.isPublic && currentScope.parent is null)
 				{
-					rootSymbol.acSymbol.parts.insert(symbols);
+					rootSymbol.acSymbol.parts.insert(allocate!ACSymbol(symbolAllocator,
+						IMPORT_SYMBOL_NAME, CompletionKind.importSymbol, symbol));
 				}
 				continue;
 			}
-			symbolLoop: foreach (symbol; symbols)
+			symbolLoop: foreach (sym; symbol.parts[])
 			{
 				foreach (tup; importInfo.importedSymbols[])
 				{
@@ -135,15 +139,13 @@ private:
 					if (tup[1] !is null)
 					{
 						ACSymbol* s = allocate!ACSymbol(symbolAllocator, tup[1],
-							symbol.kind, symbol.type);
+							sym.kind, sym.type);
-						// TODO: Compiler gets confused here, so cast the types.
+						s.parts.insert(sym.parts[]);
-						s.parts.insert(symbol.parts[]);
+						s.callTip = sym.callTip;
-						// TODO: Re-format callTip with new name?
+						s.doc = sym.doc;
-						s.callTip = symbol.callTip;
+						s.qualifier = sym.qualifier;
-						s.doc = symbol.doc;
+						s.location = sym.location;
-						s.qualifier = symbol.qualifier;
+						s.symbolFile = sym.symbolFile;
-						s.location = symbol.location;
-						s.symbolFile = symbol.symbolFile;
 						currentScope.symbols.insert(s);
 						moduleSymbol.parts.insert(s);
 						if (importInfo.isPublic && currentScope.parent is null)
@@ -151,10 +153,10 @@ private:
 					}
 					else
 					{
-						moduleSymbol.parts.insert(symbol);
+						moduleSymbol.parts.insert(sym);
-						currentScope.symbols.insert(symbol);
+						currentScope.symbols.insert(sym);
 						if (importInfo.isPublic && currentScope.parent is null)
-							rootSymbol.acSymbol.parts.insert(symbol);
+							rootSymbol.acSymbol.parts.insert(sym);
 					}
 				}
 			}

src/conversion/third.d

@@ -92,6 +92,7 @@ private:
 		case assocArray:
 		case templateName:
 		case mixinTemplateName:
+		case importSymbol:
 			break;
 		}
 
@@ -145,7 +146,7 @@ private:
 			auto parts = currentSymbol.acSymbol.getPartsByName(aliasThis);
 			if (parts.length == 0 || parts[0].type is null)
 				continue;
-			currentSymbol.acSymbol.aliasThisParts.insert(parts[0].type.parts[]);
+			currentSymbol.acSymbol.aliasThisParts.insert(parts[0].type);
 		}
 	}
 

src/dscanner-report.json

@@ -0,0 +1,54 @@
+{
+  "issues": [
+    {
+      "key": "dscanner.suspicious.incomplete_operator_overloading",
+      "fileName": "src/modulecache.d",
+      "line": 46,
+      "column": 16,
+      "message": "'CacheEntry' has method 'opCmp', but not 'opEquals'."
+    },
+    {
+      "key": "dscanner.suspicious.length_subtraction",
+      "fileName": "src/autocomplete.d",
+      "line": 298,
+      "column": 20,
+      "message": "Avoid subtracting from '.length' as it may be unsigned."
+    },
+    {
+      "key": "dscanner.suspicious.length_subtraction",
+      "fileName": "src/autocomplete.d",
+      "line": 357,
+      "column": 26,
+      "message": "Avoid subtracting from '.length' as it may be unsigned."
+    },
+    {
+      "key": "dscanner.suspicious.length_subtraction",
+      "fileName": "src/autocomplete.d",
+      "line": 442,
+      "column": 68,
+      "message": "Avoid subtracting from '.length' as it may be unsigned."
+    },
+    {
+      "key": "dscanner.suspicious.length_subtraction",
+      "fileName": "src/autocomplete.d",
+      "line": 515,
+      "column": 12,
+      "message": "Avoid subtracting from '.length' as it may be unsigned."
+    },
+    {
+      "key": "dscanner.suspicious.length_subtraction",
+      "fileName": "src/autocomplete.d",
+      "line": 736,
+      "column": 26,
+      "message": "Avoid subtracting from '.length' as it may be unsigned."
+    }
+  ],
+  "interfaceCount": 0,
+  "classCount": 3,
+  "functionCount": 120,
+  "templateCount": 0,
+  "structCount": 11,
+  "statementCount": 1718,
+  "lineOfCodeCount": 2180,
+  "undocumentedPublicSymbols": 0
+}

src/messages.d

@@ -27,6 +27,10 @@ enum CompletionKind : char
 	/// be returned in a completion response.
 	dummy = '?',
 
+	/// Import symbol. This is used internally and will never
+	/// be returned in a completion response.
+	importSymbol = '*',
+
 	/// class names
 	className = 'c',
 

src/modulecache.d

@@ -45,17 +45,27 @@ import messages;
 
 private struct CacheEntry
 {
-	ACSymbol*[] symbols;
+	ACSymbol* symbol;
 	SysTime modificationTime;
 	string path;
 
 	int opCmp(ref const CacheEntry other) const
 	{
+		int r = path > other.path;
 		if (path < other.path)
 			return -1;
-		if (path > other.path)
+		return r;
-			return 1;
+	}
-		return 0;
+
+	bool opEquals(ref const CacheEntry other) const
+	{
+		return path == other.path;
+	}
+
+	size_t toHash() const
+	{
+		import core.internal.hash : hashOf;
+		return hashOf(path);
 	}
 
 	void opAssign(ref const CacheEntry other)
@@ -64,6 +74,9 @@ private struct CacheEntry
 	}
 }
 
+/**
+ * Returns: true if a file exists at the given path.
+ */
 bool existanceCheck(A)(A path)
 {
 	if (path.exists())
@@ -84,10 +97,6 @@ struct ModuleCache
 {
 	@disable this();
 
-	static void clear()
-	{
-	}
-
 	/**
 	 * Adds the given path to the list of directories checked for imports
 	 */
@@ -108,13 +117,16 @@ struct ModuleCache
 		}
 	}
 
+	/// TODO: Implement
+	static void clear() {}
+
 	/**
 	 * Params:
 	 *     moduleName = the name of the module in "a/b/c" form
 	 * Returns:
 	 *     The symbols defined in the given module
 	 */
-	static ACSymbol*[] getSymbolsInModule(string location)
+	static ACSymbol* getSymbolsInModule(string location)
 	{
 		import string_interning;
 		assert (location !is null);
@@ -124,8 +136,8 @@ struct ModuleCache
 			e.path = location;
 			auto r = cache.equalRange(&e);
 			if (!r.empty)
-				return r.front.symbols;
+				return r.front.symbol;
-			return [];
+			return null;
 		}
 
 		string cachedLocation = internString(location);
@@ -134,9 +146,7 @@ struct ModuleCache
 
 		recursionGuard.insert(cachedLocation);
 
-
+		ACSymbol* symbol;
-
-		ACSymbol*[] symbols;
 //		try
 //		{
 			import std.stdio;
@@ -144,7 +154,7 @@ struct ModuleCache
 			File f = File(cachedLocation);
 			immutable fileSize = cast(size_t)f.size;
 			if (fileSize == 0)
-				return symbols;
+				return null;
 			ubyte[] source = cast(ubyte[]) Mallocator.it.allocate(fileSize);
 			f.rawRead(source);
 			LexerConfig config;
@@ -156,6 +166,8 @@ struct ModuleCache
 				config, &parseStringCache);
 			Mallocator.it.deallocate(source);
 
+//			StopWatch sw;
+//			sw.start();
 			Module m = parseModuleSimple(tokens[], cachedLocation, semanticAllocator);
 
 			assert (symbolAllocator);
@@ -163,20 +175,17 @@ struct ModuleCache
 				semanticAllocator);
 			first.run();
 
+//			Log.trace(location, " finished in ", sw.peek.msecs, "msecs");
+
 			SecondPass second = SecondPass(first);
 			second.run();
 
 			ThirdPass third = ThirdPass(second, cachedLocation);
 			third.run();
 
-			symbols = cast(ACSymbol*[]) Mallocator.it.allocate(
+			symbol = third.rootSymbol.acSymbol;
-				third.rootSymbol.acSymbol.parts.length * (ACSymbol*).sizeof);
-			size_t i = 0;
-			foreach (part; third.rootSymbol.acSymbol.parts[])
-				symbols[i++] = part;
 
 			typeid(Scope).destroy(third.moduleScope);
-			typeid(SemanticSymbol).destroy(third.rootSymbol);
 			symbolsAllocated += first.symbolsAllocated;
 //		}
 //		catch (Exception ex)
@@ -187,11 +196,11 @@ struct ModuleCache
 		SysTime access;
 		SysTime modification;
 		getTimes(cachedLocation, access, modification);
-		CacheEntry* c = allocate!CacheEntry(Mallocator.it, symbols,
+		CacheEntry* c = allocate!CacheEntry(Mallocator.it, symbol,
 			modification, cachedLocation);
 		cache.insert(c);
 		recursionGuard.remove(cachedLocation);
-		return symbols;
+		return symbol;
 	}
 
 	/**