Skip to content

Commit 877f2f7

Browse files
committed
Remove duplicate Symbols caused by a Label
**The Problem:** In `NormGrammarReader.processSymbol`, a cache is maintained for symbols that are already processed. However, this cache did not take into account that Symbols with and without a Label should be treated as equal (e.g. `tail:Stm` and `Stm`, see org.spoofax.jsglr2.integration/src/main/resources/grammars/layout-sensitive.sdf3). These labels are only used to reference the symbols from the layout constraints, but do not actually contribute to the meaning of a production. **Why is this a problem?** For SLR parse table generation (see metaborg#27), all grammar symbols are assumed to be unique. However, because Symbols with or without a Label were not treated as equal, the "graph" used to calculate the First- and Follow-sets became disconnected. This in turn caused some symbols to have empty First-/Follow-sets. The productions belonging to these symbols would not get any reduce actions in the parse table, because if the Follow-set is empty, no lookahead is allowed. **Current solution:** When processing symbols in the `NormGrammarReader`, any Label constructor is removed from the symbol term. **Discussion:** I have tried looking around where these labels are used (I am assuming layout constraints, but I couldn't find how they are processed in NormGrammarReader). Depending on their usage, it might be a good idea to apply this filter in Stratego already, e.g. in the strategy `module-to-normal-form`. @udesou any feedback is appreciated, as always 🙂
1 parent c31b4af commit 877f2f7

File tree

1 file changed

+33
-47
lines changed

1 file changed

+33
-47
lines changed

org.metaborg.sdf2table/src/main/java/org/metaborg/sdf2table/io/NormGrammarReader.java

+33-47
Original file line numberDiff line numberDiff line change
@@ -3,50 +3,14 @@
33
import java.io.File;
44
import java.io.FileReader;
55
import java.io.IOException;
6-
import java.util.Collection;
7-
import java.util.Collections;
8-
import java.util.LinkedList;
9-
import java.util.List;
10-
import java.util.Map;
11-
import java.util.Set;
6+
import java.util.*;
127

138
import org.metaborg.parsetable.characterclasses.CharacterClassFactory;
149
import org.metaborg.parsetable.characterclasses.ICharacterClass;
1510
import org.metaborg.sdf2table.exceptions.ModuleNotFoundException;
1611
import org.metaborg.sdf2table.exceptions.UnexpectedTermException;
17-
import org.metaborg.sdf2table.grammar.AltSymbol;
18-
import org.metaborg.sdf2table.grammar.CharacterClassSymbol;
19-
import org.metaborg.sdf2table.grammar.ConstructorAttribute;
20-
import org.metaborg.sdf2table.grammar.ContextFreeSymbol;
21-
import org.metaborg.sdf2table.grammar.DeprecatedAttribute;
22-
import org.metaborg.sdf2table.grammar.FileStartSymbol;
23-
import org.metaborg.sdf2table.grammar.GeneralAttribute;
24-
import org.metaborg.sdf2table.grammar.IAttribute;
25-
import org.metaborg.sdf2table.grammar.ISymbol;
26-
import org.metaborg.sdf2table.grammar.IterSepSymbol;
27-
import org.metaborg.sdf2table.grammar.IterStarSepSymbol;
28-
import org.metaborg.sdf2table.grammar.IterStarSymbol;
29-
import org.metaborg.sdf2table.grammar.IterSymbol;
30-
import org.metaborg.sdf2table.grammar.Layout;
31-
import org.metaborg.sdf2table.grammar.LayoutConstraintAttribute;
32-
import org.metaborg.sdf2table.grammar.LexicalSymbol;
33-
import org.metaborg.sdf2table.grammar.LiteralType;
34-
import org.metaborg.sdf2table.grammar.NormGrammar;
35-
import org.metaborg.sdf2table.grammar.OptionalSymbol;
36-
import org.metaborg.sdf2table.grammar.Priority;
37-
import org.metaborg.sdf2table.grammar.Production;
38-
import org.metaborg.sdf2table.grammar.ProductionReference;
39-
import org.metaborg.sdf2table.grammar.SequenceSymbol;
40-
import org.metaborg.sdf2table.grammar.Sort;
41-
import org.metaborg.sdf2table.grammar.StartSymbol;
42-
import org.metaborg.sdf2table.grammar.Symbol;
43-
import org.metaborg.sdf2table.grammar.TermAttribute;
44-
import org.metaborg.sdf2table.grammar.UniqueProduction;
45-
import org.spoofax.interpreter.terms.IStrategoAppl;
46-
import org.spoofax.interpreter.terms.IStrategoList;
47-
import org.spoofax.interpreter.terms.IStrategoString;
48-
import org.spoofax.interpreter.terms.IStrategoTerm;
49-
import org.spoofax.interpreter.terms.ITermFactory;
12+
import org.metaborg.sdf2table.grammar.*;
13+
import org.spoofax.interpreter.terms.*;
5014
import org.spoofax.terms.StrategoAppl;
5115
import org.spoofax.terms.StrategoList;
5216
import org.spoofax.terms.StrategoString;
@@ -115,7 +79,7 @@ private void normalizeIndexedPriorities() {
11579
for(Integer arg : grammar.priorities().get(p)) {
11680
if(arg != -1 && arg != Integer.MIN_VALUE && arg != Integer.MAX_VALUE) {
11781
grammar.getIndexedPriorities().put(p, arg);
118-
}
82+
}
11983
}
12084
}
12185

@@ -389,6 +353,7 @@ private Symbol processSymbol(IStrategoTerm term) {
389353
Symbol symbol = null;
390354
String enquoted;
391355

356+
term = removeLabelFromSymbolTerm(term);
392357
symbol = grammar.getCacheSymbolsRead().get(term.toString());
393358

394359
if(symbol != null) {
@@ -453,9 +418,6 @@ private Symbol processSymbol(IStrategoTerm term) {
453418
case "FileStart":
454419
symbol = new FileStartSymbol();
455420
break;
456-
case "Label":
457-
symbol = processSymbol(app.getSubterm(1));
458-
break;
459421
default:
460422
System.err.println("Unknown symbol type `" + app.getName() + "'. Is that normalized SDF3?");
461423
return null;
@@ -465,14 +427,38 @@ private Symbol processSymbol(IStrategoTerm term) {
465427
return null;
466428
}
467429

468-
if(grammar != null && symbol != null) {
469-
grammar.getCacheSymbolsRead().put(term.toString(), symbol);
470-
}
471-
430+
grammar.getCacheSymbolsRead().put(term.toString(), symbol);
472431
grammar.getSymbols().add(symbol);
473432
return symbol;
474433
}
475434

435+
private IStrategoTerm removeLabelFromSymbolTerm(IStrategoTerm term) {
436+
if(!(term instanceof StrategoAppl))
437+
return term;
438+
StrategoAppl app = (StrategoAppl) term;
439+
440+
if(app.getName().equals("Label")) {
441+
return removeLabelFromSymbolTerm(app.getSubterm(1));
442+
}
443+
444+
IStrategoTerm[] children = app.getAllSubterms();
445+
boolean changed = false;
446+
for(int i = 0; i < children.length; i++) {
447+
IStrategoTerm newChild = removeLabelFromSymbolTerm(children[i]);
448+
if(newChild != children[i]) {
449+
changed = true;
450+
children[i] = newChild;
451+
}
452+
}
453+
454+
// Only create a new Stratego term if the children have changed.
455+
if(changed) {
456+
return new StrategoAppl(app.getConstructor(), children, app.getAnnotations(), app.getStorageType());
457+
}
458+
// If there is no `Label` subterm, the full term is reused.
459+
return term;
460+
}
461+
476462
public List<Symbol> processSymbolList(IStrategoTerm term) {
477463
List<Symbol> list = Lists.newLinkedList();
478464

0 commit comments

Comments
 (0)