Using Stratego’s map in a Spoofax plugin on a list of lists, annotations are removed.

The code was something like:

<map(some-strategy)> [[...]{[...]}]
some-strategy: x -> x where ...

Which resulted in , while normal Stratego (both strc and strj) still have the annotation on the terms inside of the list.

I tracked down the problem to somewhere around line 126 of WrappedAstNodeFactory.

Submitted by Nathan Bruning on 28 March 2010 at 19:30

On 29 March 2010 at 11:38 Lennart Kats commented:

Should be fixed in r20722.


On 2 April 2010 at 11:43 Nathan Bruning commented:

Not fixed.

Stack trace (in call order):

WrappedAstNodeFactory.replaceList([[]{[]}], [[]])
WrappedAstNodeFactory.replaceList([[]{[]}], 0, [[]], [])
WrappedAstNodeFactory.ensureLink([]{[]}, [])
WrappedAstNodeFactory.makeLink([]{[]}, [])
WrappedAstNodeFactory.replaceList([]{[]}, [])

At this point, EMPTY_LIST is returned without any annotations.


On 2 April 2010 at 12:07 Nathan Bruning commented:

I’m using this patch for the spoofax imp runtime:

    Index: src/org/strategoxt/imp/runtime/stratego/adapter/WrappedAstNodeFactory.java
===================================================================
--- src/org/strategoxt/imp/runtime/stratego/adapter/WrappedAstNodeFactory.java	(revision 20748)
%2B%2B%2B src/org/strategoxt/imp/runtime/stratego/adapter/WrappedAstNodeFactory.java	(working copy)
@@ -125,7 %2B125,12 @@
 	private IStrategoList replaceList(IStrategoList terms, IStrategoList old) {
 		if (terms.isEmpty()) {
 			assert old.isEmpty();
-			return EMPTY_LIST; // we don't bother linking empty lists	 
%2B			// we don't bother linking empty lists
%2B			if (terms.getAnnotations().isEmpty()) {
%2B				return EMPTY_LIST;
%2B			} else {
%2B				return (IStrategoList)this.annotateTerm(EMPTY_LIST, terms.getAnnotations());
%2B			} 	 
 		} else {
 			IStrategoTerm head = ensureLink(terms.head(), old.head());
 			IStrategoList tail = replaceList(terms.tail(), old.tail());

On 6 April 2010 at 08:48 Lennart Kats commented:

Fixed, again, in 0.4.3, using Nathan’s patch.


On 7 April 2010 at 21:55 Nathan Bruning commented:

The patch works for empty lists, but not for non-empty lists.

New patch (might however be smarter to look whether there are any annotations before calling annotateTerm):

Index: src/org/strategoxt/imp/runtime/stratego/adapter/WrappedAstNodeFactory.java
===================================================================
--- src/org/strategoxt/imp/runtime/stratego/adapter/WrappedAstNodeFactory.java	(revision 20752)
+++ src/org/strategoxt/imp/runtime/stratego/adapter/WrappedAstNodeFactory.java	(working copy)
@@ -137,7 +137,8 @@
		IStrategoList tail = replaceList(terms.tail(), old.tail());
		if (old instanceof WrappedAstNodeList) {
			WrappedAstNodeList oldList = (WrappedAstNodeList) old;
-				return new WrappedAstNodeList(oldList.getNode(), oldList.getOffset(), head, tail);
+				WrappedAstNodeList newList = new WrappedAstNodeList(oldList.getNode(), oldList.getOffset(), head, tail);
+				return (IStrategoList)this.annotateTerm(newList, terms.getAnnotations());
		} else {
			// UNDONE: assert !(old instanceof IWrappedAstNode);
			return makeListCons(head, tail);

On 8 April 2010 at 09:15 Lennart Kats commented:

Hm, I see. The code is a bit confusing since there are two methods called replaceList(). They’re very similar in implementation, but they have different functions. The first one has very weak annotation preservation guarantees, corresponding to the Stratego semantics of most map-like operations on lists. The second one, listed here, must preserve all annotatons. I changed the name of the second one to makeListLink() to make this distinction clear.

Anyway, this is fixed, once again, in 0.4.3.3.


On 9 April 2010 at 10:55 Nathan Bruning commented:

I’m not completely aware of the Stratego-like semantics, but anyway the current SVN revision does not fix my case.

makeListLink does not preserve the annotations on the terms variable.
Got it fixed by replacing the last line of makeListLink with:

return makeListCons(newHead, newTail, terms.getAnnotations());

Stacktrace:
WrappedAstNodeFactory.makeListLink(IStrategoList, IStrategoList) line: 147
WrappedAstNodeFactory.makeLink(IStrategoTerm, IWrappedAstNode) line: 102
WrappedAstNodeFactory.ensureLink(IStrategoTerm, IStrategoTerm) line: 185
WrappedAstNodeFactory.replaceList(IStrategoTerm[], int, IStrategoList, IStrategoList) line: 120
WrappedAstNodeFactory.replaceList(IStrategoTerm[], IStrategoList) line: 90
SRTS_all.mapIgnoreAnnos(Context, IStrategoTerm, IStrategoList, Strategy) line: 93
SRTS_all.map(Context, IStrategoList, Strategy) line: 76
SRTS_all.invoke(Context, IStrategoTerm, Strategy) line: 21
map_1_0_override.invoke(Context, IStrategoTerm, Strategy) line: 27


On 10 April 2010 at 10:15 Lennart Kats commented:

Oops. My mistake. It’s really too bad that we cannot (easily) apply the standard Stratego unit tests for this term factory, since it only creates these special terms for things parsed from a file. (And since it’s a separate project from STRJ.)

Log in to post comments