Skip to content

Commit b078b5b

Browse files
Mi-LaMi-La
committed
[#421] Fix cyclic template instantiation
1 parent ff8afca commit b078b5b

14 files changed

+149
-18
lines changed

‎compiler/core/src/zserio/ast/Package.java

-2
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,6 @@ PackageName getTopLevelPackageName()
140140
/**
141141
* Gets a symbol if it's visible in this package.
142142
*
143-
* This method does not throw exception in case of ambiguous symbol. It just returns null in this case.
144-
*
145143
* @param ownerNode AST node which holds symbol to resolve (used for ParserException).
146144
* @param packageName Package name where the symbol is defined.
147145
* @param symbolName Symbol name to resolve.

‎compiler/core/src/zserio/ast/TypeInstantiation.java

-1
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,6 @@ protected void fillInstantiationStack(ParserStackedException exception)
147147
exception.pushMessage(resolvingType.getLocation(),
148148
" See subtype '" +
149149
ZserioTypeUtil.getReferencedFullName(resolvingTypeReference) + "' definition here:");
150-
151150
resolvingTypeReference = ((Subtype)resolvingType).getTypeReference();
152151
}
153152
else if (resolvingType instanceof InstantiateType)

‎compiler/core/src/zserio/ast/ZserioAstTypeResolver.java

+26-12
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,7 @@ public class ZserioAstTypeResolver extends ZserioAstWalker
1111
@Override
1212
public void visitSubtype(Subtype subtype)
1313
{
14-
if (!subtypesOnStack.isEmpty() && subtypesOnStack.get(0).equals(subtype))
15-
{
16-
final ParserStackedException stackedException = new ParserStackedException(subtype.getLocation(),
17-
"Cyclic dependency detected in subtype '" + subtype.getName() + "' definition!");
18-
for (int i = 1; i < subtypesOnStack.size(); ++i)
19-
{
20-
final Subtype subtypeOnStack = subtypesOnStack.get(i);
21-
stackedException.pushMessage(subtypeOnStack.getLocation(),
22-
" Through subtype '" + subtypeOnStack.getName() + "' here");
23-
}
24-
throw stackedException;
25-
}
14+
checkCycle(subtypesOnStack, subtype, "subtype");
2615

2716
subtypesOnStack.add(subtype);
2817

@@ -63,6 +52,7 @@ public void visitTypeReference(TypeReference typeReference)
6352
typeReference.resolve(templateParameters);
6453

6554
// make sure that typeReference.getBaseTypeReference() can be called after this resolve
55+
// note: this can cause cycles which are guarded by subtypesOnStack and instantiateTypesOnStack
6656
final ZserioType type = typeReference.getType();
6757
if (type instanceof Subtype || type instanceof InstantiateType)
6858
type.accept(this);
@@ -78,8 +68,15 @@ public void visitTypeInstantiation(TypeInstantiation typeInstantiation)
7868
@Override
7969
public void visitInstantiateType(InstantiateType instantiateType)
8070
{
71+
// cannot be checked in resolve since the cycle happens via visitChildren
72+
checkCycle(instantiateTypesOnStack, instantiateType, "template instantiation");
73+
74+
instantiateTypesOnStack.add(instantiateType);
75+
8176
instantiateType.visitChildren(this);
8277
instantiateType.resolve();
78+
79+
instantiateTypesOnStack.remove(instantiateTypesOnStack.size() - 1);
8380
}
8481

8582
private void visitType(TemplatableType templatableType)
@@ -91,6 +88,23 @@ private void visitType(TemplatableType templatableType)
9188
templateParameters = null;
9289
}
9390

91+
private <T extends ZserioType> void checkCycle(List<T> typesOnStack, T type, String typeName)
92+
{
93+
if (!typesOnStack.isEmpty() && typesOnStack.get(0) == type)
94+
{
95+
final ParserStackedException stackedException = new ParserStackedException(type.getLocation(),
96+
"Cyclic dependency detected in " + typeName + " '" + type.getName() + "'!");
97+
for (int i = 1; i < typesOnStack.size(); ++i)
98+
{
99+
final T typeOnStack = typesOnStack.get(i);
100+
stackedException.pushMessage(typeOnStack.getLocation(),
101+
" Through " + typeName + " '" + typeOnStack.getName() + "' here");
102+
}
103+
throw stackedException;
104+
}
105+
}
106+
94107
private final List<Subtype> subtypesOnStack = new ArrayList<Subtype>();
108+
private final List<InstantiateType> instantiateTypesOnStack = new ArrayList<InstantiateType>();
95109
private List<TemplateParameter> templateParameters = null;
96110
}

‎test/errors/subtypes_error/java/subtypes_error/SubtypesErrorTest.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public void parameterizedSubtype()
3636
public void simpleCyclicDependency()
3737
{
3838
final String error = "simple_cyclic_dependency_error.zs:3:11: " +
39-
"Cyclic dependency detected in subtype 'X' definition!";
39+
"Cyclic dependency detected in subtype 'X'!";
4040
assertTrue(zserioErrors.isPresent(error));
4141
}
4242

@@ -47,8 +47,7 @@ public void transitiveCyclicDependency()
4747
{
4848
"transitive_cyclic_dependency_error.zs:4:11: Through subtype 'Z' here",
4949
"transitive_cyclic_dependency_error.zs:5:11: Through subtype 'X' here",
50-
"transitive_cyclic_dependency_error.zs:3:11: Cyclic dependency detected in " +
51-
"subtype 'Y' definition!"
50+
"transitive_cyclic_dependency_error.zs:3:11: Cyclic dependency detected in subtype 'Y'!"
5251
};
5352
assertTrue(zserioErrors.isPresent(errors));
5453
}

‎test/errors/templates_error/build.xml

+11
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,25 @@
2828
<testGen testName="templates_error" zsFile="instantiate_missing_template_arguments_error.zs"
2929
ignoreErrors="true"/>
3030
<testGen testName="templates_error" zsFile="instantiate_name_clash_error.zs" ignoreErrors="true"/>
31+
<testGen testName="templates_error" zsFile="instantiate_name_clash_with_single_import_error.zs"
32+
ignoreErrors="true"/>
33+
<testGen testName="templates_error" zsFile="instantiate_name_clash_with_template_error.zs"
34+
ignoreErrors="true"/>
3135
<testGen testName="templates_error" zsFile="instantiate_name_clash_with_type_error.zs"
3236
ignoreErrors="true"/>
3337
<testGen testName="templates_error" zsFile="instantiate_no_template_error.zs" ignoreErrors="true"/>
3438
<testGen testName="templates_error" zsFile="instantiate_subtype_error.zs" ignoreErrors="true"/>
39+
<testGen testName="templates_error" zsFile="instantiate_type_cycle_error.zs" ignoreErrors="true"/>
40+
<testGen testName="templates_error" zsFile="instantiate_type_full_name_cycle_error.zs"
41+
ignoreErrors="true"/>
42+
<testGen testName="templates_error" zsFile="instantiate_type_imported_cycle_error.zs"
43+
ignoreErrors="true"/>
3544
<testGen testName="templates_error" zsFile="instantiate_type_in_instantiate_error.zs"
3645
ignoreErrors="true"/>
3746
<testGen testName="templates_error" zsFile="instantiate_type_is_sql_table_error.zs"
3847
ignoreErrors="true"/>
48+
<testGen testName="templates_error" zsFile="instantiate_type_transitive_cycle_error.zs"
49+
ignoreErrors="true"/>
3950
<testGen testName="templates_error" zsFile="instantiation_via_subtype_error.zs" ignoreErrors="true"/>
4051
<testGen testName="templates_error" zsFile="missing_template_arguments_error.zs" ignoreErrors="true"/>
4152
<testGen testName="templates_error" zsFile="missing_type_parameters_error.zs" ignoreErrors="true"/>

‎test/errors/templates_error/java/templates_error/TemplatesErrorTest.java

+61
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,29 @@ public void instantiateNameClash()
154154
assertTrue(zserioErrors.isPresent(errors));
155155
}
156156

157+
@Test
158+
public void instantiateNameClashWithSingleImport()
159+
{
160+
final String errors[] =
161+
{
162+
"instantiate_name_clash_with_single_import_error/pkg.zs:3:8: Found here",
163+
"instantiate_name_clash_with_single_import_error.zs:5:30: Found here",
164+
"instantiate_name_clash_with_single_import_error.zs:5:13: Ambiguous symbol 'Template'"
165+
};
166+
assertTrue(zserioErrors.isPresent(errors));
167+
}
168+
169+
@Test
170+
public void instantiateNameClashWithTemplate()
171+
{
172+
final String errors[] =
173+
{
174+
"instantiate_name_clash_with_template_error.zs:3:8: First defined here",
175+
"instantiate_name_clash_with_template_error.zs:8:30: 'Template' is already defined in this package!"
176+
};
177+
assertTrue(zserioErrors.isPresent(errors));
178+
}
179+
157180
@Test
158181
public void instantiateNameClashWithType()
159182
{
@@ -179,6 +202,30 @@ public void instantiateSubtype()
179202
assertTrue(zserioErrors.isPresent(error));
180203
}
181204

205+
@Test
206+
public void instantiateTypeCycle()
207+
{
208+
final String error = "instantiate_type_cycle_error.zs:3:30: " +
209+
"Cyclic dependency detected in template instantiation 'Template'!";
210+
assertTrue(zserioErrors.isPresent(error));
211+
}
212+
213+
@Test
214+
public void instantiateTypeFullNameCycle()
215+
{
216+
final String error = "instantiate_type_full_name_cycle_error.zs:3:69: " +
217+
"Cyclic dependency detected in template instantiation 'Template'!";
218+
assertTrue(zserioErrors.isPresent(error));
219+
}
220+
221+
@Test
222+
public void instantiateTypeImportedCycle()
223+
{
224+
final String error = "instantiate_type_imported_cycle_error.zs:8:30: " +
225+
"Cyclic dependency detected in template instantiation 'Template'!";
226+
assertTrue(zserioErrors.isPresent(error));
227+
}
228+
182229
@Test
183230
public void instantiateTypeInInstantiate()
184231
{
@@ -194,6 +241,20 @@ public void instantiateTypeIsSqlTable()
194241
assertTrue(zserioErrors.isPresent(error));
195242
}
196243

244+
@Test
245+
public void instantiateTypeTransitiveCycle()
246+
{
247+
final String[] errors = {
248+
"instantiate_type_transitive_cycle_error.zs:4:27: " +
249+
" Through template instantiation 'Some' here",
250+
"instantiate_type_transitive_cycle_error.zs:5:26: " +
251+
" Through template instantiation 'Template' here",
252+
"instantiate_type_transitive_cycle_error.zs:3:30: " +
253+
"Cyclic dependency detected in template instantiation 'Other'!"
254+
};
255+
assertTrue(zserioErrors.isPresent(errors));
256+
}
257+
197258
@Test
198259
public void instantiationViaSubtype()
199260
{

‎test/errors/templates_error/zs/instantiate_name_clash_with_single_import_error.zs

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package instantiate_name_clash_with_single_import_error;
2+
3+
import instantiate_name_clash_with_single_import_error.pkg.Template;
4+
5+
instantiate Template<uint32> Template;

‎test/errors/templates_error/zs/instantiate_name_clash_with_single_import_error/pkg.zs

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package instantiate_name_clash_with_single_import_error.pkg;
2+
3+
struct Template<T>
4+
{
5+
T field;
6+
};

‎test/errors/templates_error/zs/instantiate_name_clash_with_template_error.zs

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package instantiate_name_clash_with_template_error;
2+
3+
struct Template<T>
4+
{
5+
T field;
6+
};
7+
8+
instantiate Template<uint32> Template;

‎test/errors/templates_error/zs/instantiate_type_cycle_error.zs

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
package instantiate_type_cycle_error;
2+
3+
instantiate Template<uint32> Template;

‎test/errors/templates_error/zs/instantiate_type_full_name_cycle_error.zs

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
package instantiate_type_full_name_cycle_error;
2+
3+
instantiate instantiate_type_full_name_cycle_error.Template<uint32> Template;

‎test/errors/templates_error/zs/instantiate_type_imported_cycle_error.zs

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package instantiate_type_imported_cycle_error;
2+
3+
import instantiate_type_imported_cycle_error.pkg.*;
4+
5+
// ok via full name
6+
instantiate instantiate_type_imported_cycle_error.pkg.OtherTemplate<int32> OtherTemplate;
7+
8+
instantiate Template<uint32> Template;

‎test/errors/templates_error/zs/instantiate_type_imported_cycle_error/pkg.zs

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package instantiate_type_imported_cycle_error.pkg;
2+
3+
struct Template<T>
4+
{
5+
T field;
6+
};
7+
8+
struct OtherTemplate<T>
9+
{
10+
T field;
11+
};

‎test/errors/templates_error/zs/instantiate_type_transitive_cycle_error.zs

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package instantiate_type_transitive_cycle_error;
2+
3+
instantiate Template<uint32> Other;
4+
instantiate Other<uint32> Some;
5+
instantiate Some<uint32> Template;

0 commit comments

Comments
 (0)