Merge pull request #2075 from usethesource/feat/remove-auto-disambigu…

…ation

Feat/remove auto disambiguation

src/org/rascalmpl/library/lang/rascal/tests/concrete/recovery/BasicRecoveryTests.rsc

@@ -32,12 +32,3 @@ test bool basicOk() = checkRecovery(#S, "a b c $", []);
 test bool abx() = checkRecovery(#S, "a b x $", ["x "]);
 
 test bool axc() = checkRecovery(#S, "a x c $", ["x c"]);
-
-test bool autoDisambiguation() {
-    str input = "a x $";
-
-    assert checkRecovery(#S, input, ["x "]);
-
-    Tree autoDisambiguated = parser(#S, allowRecovery=true, allowAmbiguity=false)(input, |unknown:///|);
-    return size(findAllErrors(autoDisambiguated)) == 1;
-}

src/org/rascalmpl/library/util/ErrorRecovery.java

@@ -48,17 +48,17 @@ public ScoredTree(IConstructor tree, int score) {
     */
 
     public IConstructor disambiguateErrors(IConstructor arg, IBool allowAmbiguity) {
-        return disambiguate(arg, allowAmbiguity.getValue(), new HashMap<>()).tree;
+        return disambiguate(arg, allowAmbiguity.getValue(), true, new HashMap<>()).tree;
     }
 
-    private ScoredTree disambiguate(IConstructor tree, boolean allowAmbiguity, Map<IConstructor, ScoredTree> processedTrees) {
+    private ScoredTree disambiguate(IConstructor tree, boolean allowAmbiguity, boolean buildTree, Map<IConstructor, ScoredTree> processedTrees) {
         Type type = tree.getConstructorType();
         ScoredTree result;
 
         if (type == RascalValueFactory.Tree_Appl) {
-            result = disambiguateAppl((ITree) tree, allowAmbiguity, processedTrees);
+            result = disambiguateAppl((ITree) tree, allowAmbiguity, buildTree, processedTrees);
         } else if (type == RascalValueFactory.Tree_Amb) {
-            result = disambiguateAmb((ITree) tree, allowAmbiguity, processedTrees);
+            result = disambiguateAmb((ITree) tree, allowAmbiguity, buildTree, processedTrees);
         } else {
             // Other trees (cycle, char) do not have subtrees so they have a score of 0
             result = new ScoredTree(tree, 0);
@@ -67,7 +67,7 @@ private ScoredTree disambiguate(IConstructor tree, boolean allowAmbiguity, Map<I
         return result;
     }
 
-    private ScoredTree disambiguateAppl(ITree appl, boolean allowAmbiguity, Map<IConstructor, ScoredTree> processedTrees) {
+    private ScoredTree disambiguateAppl(ITree appl, boolean allowAmbiguity, boolean buildTree, Map<IConstructor, ScoredTree> processedTrees) {
         ScoredTree result = processedTrees.get(appl);
         if (result != null) {
             return result;
@@ -83,9 +83,9 @@ private ScoredTree disambiguateAppl(ITree appl, boolean allowAmbiguity, Map<ICon
             // Disambiguate and score all children
             for (int i=0; i<args.size(); i++) {
                 IValue arg = args.get(i);
-                ScoredTree disambiguatedArg = disambiguate((IConstructor) arg, allowAmbiguity, processedTrees);
+                ScoredTree disambiguatedArg = disambiguate((IConstructor) arg, allowAmbiguity, buildTree, processedTrees);
                 totalScore += disambiguatedArg.score;
-                if (disambiguatedArg.tree != arg && disambiguatedArgs == null) {
+                if (buildTree && disambiguatedArg.tree != arg && disambiguatedArgs == null) {
                     disambiguatedArgs = rascalValues.listWriter();
                     for (int j=0; j<i; j++) {
                         disambiguatedArgs.append(args.get(j));
@@ -98,11 +98,11 @@ private ScoredTree disambiguateAppl(ITree appl, boolean allowAmbiguity, Map<ICon
             }
 
             // Only build a new tree if at least one of the arguments has changed
-            ITree resultTree;
+            ITree resultTree = null;
             if (disambiguatedArgs != null) {
                 // Some arguments have changed
                 resultTree = TreeAdapter.setArgs(appl, disambiguatedArgs.done());
-            } else {
+            } else if (buildTree) {
                 // None of the arguments have changed
                 resultTree = appl;
             }
@@ -115,7 +115,7 @@ private ScoredTree disambiguateAppl(ITree appl, boolean allowAmbiguity, Map<ICon
         return result;
     }
 
-    private ScoredTree disambiguateAmb(ITree amb, boolean allowAmbiguity, Map<IConstructor, ScoredTree> processedTrees) {
+    private ScoredTree disambiguateAmb(ITree amb, boolean allowAmbiguity, boolean buildTree, Map<IConstructor, ScoredTree> processedTrees) {
         ScoredTree result = processedTrees.get(amb);
         if (result != null) {
             return result;
@@ -124,9 +124,10 @@ private ScoredTree disambiguateAmb(ITree amb, boolean allowAmbiguity, Map<IConst
         ISet originalAlts = (ISet) amb.get(0);
 
         ISetWriter alternativesWithoutErrors = null;
+
         ScoredTree errorAltWithBestScore = null;
         for (IValue alt : originalAlts) {
-            ScoredTree disambiguatedAlt = disambiguate((IConstructor) alt, allowAmbiguity, processedTrees);
+            ScoredTree disambiguatedAlt = disambiguate((IConstructor) alt, allowAmbiguity, buildTree, processedTrees);
             if (disambiguatedAlt.score == 0) {
                 // Non-error tree
                 if (alternativesWithoutErrors == null) {
@@ -149,20 +150,24 @@ private ScoredTree disambiguateAmb(ITree amb, boolean allowAmbiguity, Map<IConst
 
         ISet remainingAlts = alternativesWithoutErrors.done();
 
-        ITree resultTree;
-        if (remainingAlts.size() == originalAlts.size()) {
-            // All children are without errors, return the original tree
-            resultTree = amb;
-        } else if (remainingAlts.size() == 1) {
-            // One child without errors remains, dissolve the amb tree
-            resultTree = (ITree) remainingAlts.iterator().next();
-        } else {
-            // Create a new amb tree with the remaining non-error trees
-            resultTree = rascalValues.amb(remainingAlts);
+        ITree resultTree = null;
 
+        if (remainingAlts.size() > 1 && !allowAmbiguity) {
             // We have an ambiguity between non-error trees
-            if (!allowAmbiguity) {
-                throw new Ambiguous(resultTree);
+            resultTree = rascalValues.amb(remainingAlts);
+            throw new Ambiguous(resultTree);
+        }
+
+        if (buildTree) {
+            if (remainingAlts.size() == originalAlts.size()) {
+                // All children are without errors, return the original tree
+                resultTree = amb;
+            } else if (remainingAlts.size() == 1) {
+                // One child without errors remains, dissolve the amb tree
+                resultTree = (ITree) remainingAlts.iterator().next();
+            } else {
+                // Create a new amb tree with the remaining non-error trees
+                resultTree = rascalValues.amb(remainingAlts);
             }
         }
 
@@ -217,4 +222,8 @@ private void collectAmbErrors(ITree amb, IListWriter errors, Set<IConstructor> p
             collectErrors((ITree) alt, errors, processedTrees);
         }
     }
+
+    public void checkForRegularAmbiguities(IConstructor parseForest) {
+        disambiguate(parseForest, false, false, new HashMap<>());
+    }
 }

src/org/rascalmpl/library/util/ErrorRecovery.rsc

@@ -51,3 +51,6 @@ This filter removes error trees until no ambiguities caused by error recovery ar
 Note that regular ambiguous trees remain in the parse forest unless `allowAmbiguity` is set to false in which case an error is thrown.
 }
 java Tree disambiguateErrors(Tree t, bool allowAmbiguity=true);
+
+Tree(Tree) createErrorFilter(bool allowAmbiguity) =
+    Tree(Tree t)  { return disambiguateErrors(t, allowAmbiguity=allowAmbiguity); };

src/org/rascalmpl/values/RascalFunctionValueFactory.java

@@ -585,10 +585,10 @@ private ITree parseObject(String methodName, ISourceLocation location, char[] in
             }
             ITree parseForest = (ITree) parserInstance.parse(methodName, uri, input, exec, new DefaultNodeFlattener<>(), new UPTRNodeFactory(allowRecovery || allowAmbiguity), recoverer, debugListener);
 
-            if (!allowAmbiguity && allowRecovery && filters.isEmpty()) {
-                // Filter error-induced ambiguities
+            if (!allowAmbiguity && allowRecovery) {
+                // Check for 'regular' (non-error) ambiguities
                 RascalValueFactory valueFactory = (RascalValueFactory) ValueFactoryFactory.getValueFactory();
-                parseForest = (ITree) new ErrorRecovery(valueFactory).disambiguateErrors(parseForest, valueFactory.bool(false));
+                new ErrorRecovery(valueFactory).checkForRegularAmbiguities(parseForest);
             }
 
             return parseForest;