From e1a9cd9fb38bd36fb5baf00d0f9c9c410acdd79f Mon Sep 17 00:00:00 2001 From: Pieter Olivier Date: Mon, 11 Nov 2024 10:51:27 +0100 Subject: [PATCH 1/3] Removed automatic error disambiguation when allowRecovery=true and allowAmbiguity=false --- .../tests/concrete/recovery/BasicRecoveryTests.rsc | 9 --------- src/org/rascalmpl/library/util/ErrorRecovery.rsc | 3 +++ .../rascalmpl/values/RascalFunctionValueFactory.java | 10 +--------- 3 files changed, 4 insertions(+), 18 deletions(-) diff --git a/src/org/rascalmpl/library/lang/rascal/tests/concrete/recovery/BasicRecoveryTests.rsc b/src/org/rascalmpl/library/lang/rascal/tests/concrete/recovery/BasicRecoveryTests.rsc index 692bf97f308..f3cc13aacdd 100644 --- a/src/org/rascalmpl/library/lang/rascal/tests/concrete/recovery/BasicRecoveryTests.rsc +++ b/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; -} diff --git a/src/org/rascalmpl/library/util/ErrorRecovery.rsc b/src/org/rascalmpl/library/util/ErrorRecovery.rsc index 08673830a91..a570cdcf2e4 100644 --- a/src/org/rascalmpl/library/util/ErrorRecovery.rsc +++ b/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); }; diff --git a/src/org/rascalmpl/values/RascalFunctionValueFactory.java b/src/org/rascalmpl/values/RascalFunctionValueFactory.java index 3abb852ef1c..5009dcb0762 100644 --- a/src/org/rascalmpl/values/RascalFunctionValueFactory.java +++ b/src/org/rascalmpl/values/RascalFunctionValueFactory.java @@ -583,15 +583,7 @@ private ITree parseObject(String methodName, ISourceLocation location, char[] in if (allowRecovery) { recoverer = new ToTokenRecoverer(uri, parserInstance, new StackNodeIdDispenser(parserInstance)); } - 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 - RascalValueFactory valueFactory = (RascalValueFactory) ValueFactoryFactory.getValueFactory(); - parseForest = (ITree) new ErrorRecovery(valueFactory).disambiguateErrors(parseForest, valueFactory.bool(false)); - } - - return parseForest; + return (ITree) parserInstance.parse(methodName, uri, input, exec, new DefaultNodeFlattener<>(), new UPTRNodeFactory(allowRecovery || allowAmbiguity), recoverer, debugListener); } } From ae9aa7661b67d32fcd0cbacffe676cd9395d45ba Mon Sep 17 00:00:00 2001 From: Pieter Olivier Date: Mon, 11 Nov 2024 11:01:46 +0100 Subject: [PATCH 2/3] Added check for 'regular' ambiguities when allowAmbiguities=false but allowRecovery=true --- src/org/rascalmpl/library/util/ErrorRecovery.java | 4 ++++ .../rascalmpl/values/RascalFunctionValueFactory.java | 10 +++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/org/rascalmpl/library/util/ErrorRecovery.java b/src/org/rascalmpl/library/util/ErrorRecovery.java index d117a59d717..b1d3aa6783f 100644 --- a/src/org/rascalmpl/library/util/ErrorRecovery.java +++ b/src/org/rascalmpl/library/util/ErrorRecovery.java @@ -217,4 +217,8 @@ private void collectAmbErrors(ITree amb, IListWriter errors, Set p collectErrors((ITree) alt, errors, processedTrees); } } + + public void checkForRegularAmbiguities(IConstructor parseForest) { + disambiguate(parseForest, false, new HashMap<>()); + } } diff --git a/src/org/rascalmpl/values/RascalFunctionValueFactory.java b/src/org/rascalmpl/values/RascalFunctionValueFactory.java index 5009dcb0762..331545d5e66 100644 --- a/src/org/rascalmpl/values/RascalFunctionValueFactory.java +++ b/src/org/rascalmpl/values/RascalFunctionValueFactory.java @@ -583,7 +583,15 @@ private ITree parseObject(String methodName, ISourceLocation location, char[] in if (allowRecovery) { recoverer = new ToTokenRecoverer(uri, parserInstance, new StackNodeIdDispenser(parserInstance)); } - return (ITree) parserInstance.parse(methodName, uri, input, exec, new DefaultNodeFlattener<>(), new UPTRNodeFactory(allowRecovery || allowAmbiguity), recoverer, debugListener); + ITree parseForest = (ITree) parserInstance.parse(methodName, uri, input, exec, new DefaultNodeFlattener<>(), new UPTRNodeFactory(allowRecovery || allowAmbiguity), recoverer, debugListener); + + if (!allowAmbiguity && allowRecovery) { + // Check for 'regular' (non-error) ambiguities + RascalValueFactory valueFactory = (RascalValueFactory) ValueFactoryFactory.getValueFactory(); + new ErrorRecovery(valueFactory).checkForRegularAmbiguities(parseForest); + } + + return parseForest; } } From 40b12267dc655b89b08ed93d3bcf34937d8b8d63 Mon Sep 17 00:00:00 2001 From: Pieter Olivier Date: Mon, 11 Nov 2024 13:30:27 +0100 Subject: [PATCH 3/3] No longer building the result tree when just checking for regular ambiguities --- .../rascalmpl/library/util/ErrorRecovery.java | 53 ++++++++++--------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/src/org/rascalmpl/library/util/ErrorRecovery.java b/src/org/rascalmpl/library/util/ErrorRecovery.java index b1d3aa6783f..5d116f0e70e 100644 --- a/src/org/rascalmpl/library/util/ErrorRecovery.java +++ b/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 processedTrees) { + private ScoredTree disambiguate(IConstructor tree, boolean allowAmbiguity, boolean buildTree, Map 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 processedTrees) { + private ScoredTree disambiguateAppl(ITree appl, boolean allowAmbiguity, boolean buildTree, Map processedTrees) { ScoredTree result = processedTrees.get(appl); if (result != null) { return result; @@ -83,9 +83,9 @@ private ScoredTree disambiguateAppl(ITree appl, boolean allowAmbiguity, Map processedTrees) { + private ScoredTree disambiguateAmb(ITree amb, boolean allowAmbiguity, boolean buildTree, Map processedTrees) { ScoredTree result = processedTrees.get(amb); if (result != null) { return result; @@ -124,9 +124,10 @@ private ScoredTree disambiguateAmb(ITree amb, boolean allowAmbiguity, Map 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); } } @@ -219,6 +224,6 @@ private void collectAmbErrors(ITree amb, IListWriter errors, Set p } public void checkForRegularAmbiguities(IConstructor parseForest) { - disambiguate(parseForest, false, new HashMap<>()); + disambiguate(parseForest, false, false, new HashMap<>()); } }