Permalink
Browse files

For ide bug 79 : To avoid working with the TokenStream

  • Loading branch information...
David Festal
David Festal committed Nov 29, 2011
1 parent 603d9fc commit 821741e80fda185d5bfd2d9a00e3d858e4676d62
@@ -1,9 +1,10 @@
package com.redhat.ceylon.compiler.typechecker.context;
import java.util.HashSet;
+import java.util.List;
import java.util.Set;
-import org.antlr.runtime.CommonTokenStream;
+import org.antlr.runtime.CommonToken;
import com.redhat.ceylon.compiler.typechecker.analyzer.ControlFlowVisitor;
import com.redhat.ceylon.compiler.typechecker.analyzer.DeclarationVisitor;
@@ -46,18 +47,18 @@
private final String pathRelativeToSrcDir;
private VirtualFile unitFile;
private final Set<PhasedUnit> dependentsOf = new HashSet<PhasedUnit>();
- private CommonTokenStream tokenStream;
+ private List<CommonToken> tokens;
private boolean fullyTyped;
public PhasedUnit(VirtualFile unitFile, VirtualFile srcDir, Tree.CompilationUnit cu,
- Package p, ModuleBuilder moduleBuilder, Context context, CommonTokenStream tokenStream) {
+ Package p, ModuleBuilder moduleBuilder, Context context, List<CommonToken> tokenStream) {
this.compilationUnit = cu;
this.pkg = p;
this.unitFile = unitFile;
this.fileName = unitFile.getName();
this.pathRelativeToSrcDir = computeRelativePath(unitFile, srcDir);
this.moduleBuilder = moduleBuilder;
- this.tokenStream = tokenStream;
+ this.tokens = tokenStream;
}
@Deprecated
@@ -194,8 +195,8 @@ public String toString() {
return dependentsOf;
}
- public CommonTokenStream getTokenStream() {
- return tokenStream;
+ public List<CommonToken> getTokens() {
+ return tokens;
}
}
@@ -7,6 +7,7 @@
import java.util.Map;
import org.antlr.runtime.ANTLRInputStream;
+import org.antlr.runtime.CommonToken;
import org.antlr.runtime.CommonTokenStream;
import com.redhat.ceylon.compiler.typechecker.analyzer.ModuleBuilder;
@@ -112,9 +113,11 @@ private void parseFile(VirtualFile file, VirtualFile srcDir) throws Exception {
CommonTokenStream tokenStream = new CommonTokenStream(lexer);
CeylonParser parser = new CeylonParser(tokenStream);
Tree.CompilationUnit cu = parser.compilationUnit();
+ List<CommonToken> tokens = new ArrayList<CommonToken>(tokenStream.getTokens().size());
+ tokens.addAll(tokenStream.getTokens());
PhasedUnit phasedUnit = new PhasedUnit(file, srcDir, cu,
moduleBuilder.getCurrentPackage(), moduleBuilder,
- context, tokenStream);
+ context, tokens);
addPhasedUnit(file, phasedUnit);
List<LexError> lexerErrors = lexer.getErrors();

6 comments on commit 821741e

Owner

FroMage replied Nov 29, 2011

Mmm, I think this will need a similar fix on the compiler.

Owner

quintesse replied Nov 29, 2011

What does this fix? Does this not cause excessive memory use possibly?

Member

davidfestal replied Nov 29, 2011

In fact it just avoids storing an Antlr TokenStream by reference.
The problem in doing so was that TokenStream is a statefull class. And modification operations still can be applied on it.
This was clearly the cause of the annoying multi-threading issues in the editor.

Yes, there might be places where I've taken unnecessary precautions, but only the collection is duplicated (maybe not really because the stream itself is not necessarily kept as a reference), but the copied list still references the same CommonToken objects. So I'm not very much worried about memory. But maybe I'm wrong :)

As for the compiler, I'm not sure whether you need to do the change also : I've not looked precisely at how you use the TokenStream once stored.

Owner

FroMage replied Nov 29, 2011

It's more that we instantiate PhasedUnits and PhasedUnit and their constructor changed.

Member

davidfestal replied Nov 29, 2011

Of course yes :) Sorry for not paying attention to this !

Member

davidfestal replied Nov 29, 2011

In fact no :) it seems in the compiler you only use the deprecated PhasedUnit constructor (where ex-tokenStream parameter is null) from the CeylonPhasedUnit sub-class constructuor.
So my change shouldn't impact compiler code, right ?

Please sign in to comment.