Skip to content

Commit 6168995

Browse files
writeoncereadmanytom.johnson
writeoncereadmany
and
tom.johnson
authored
Replicate compiler errors in decoders on JDK17 for "yield()" aeron-io#910 (aeron-io#911)
* Test to replicate compiler errors in decoders on JDK17 when a field is called "yield" aeron-io#910 * aeron-io#910 Start fixing unqualified yield errors This now generates code that compiles in Java 17 for groups, composites and fields named "yield", and demonstrates the need for each of these. There are many paths through the JavaGenerator I have not yet tested (or implemented). * aeron-io#910 Finish(?) fixing unqualified yield errors This now generates code that compiles in Java 17 for all the paths through writeTokenDisplay I can find, and I think that's all that's needed here? * aeron-io#910 Add a test to ensure we handle both groups called yield and groups with a field called yield, ie that recursive rendering works the way I'd hope and expect. * aeron-io#910 Use a unit test to highlight compilation errors rather than an entry in the build file. Note: this only fails when running with a Java 17 toolchain, not on Java 8 or 11. * aeron-io#910 Restrict test to run only on JRE17, and generate updated IR codecs * aeron-io#910 Fix checkstyle * aeron-io#910 Remove unnecessary property from unit test * aeron-io#910 Remove unused import Co-authored-by: tom.johnson <tom.johnson@transficc.com>
1 parent c17d2bf commit 6168995

File tree

7 files changed

+164
-27
lines changed

7 files changed

+164
-27
lines changed

‎sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaGenerator.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3492,7 +3492,7 @@ private void appendDecoderDisplay(
34923492
sb, indent, "builder.append(\"" + groupName + Separator.KEY_VALUE + Separator.BEGIN_GROUP + "\");");
34933493
append(sb, indent, "final int " + groupName + "OriginalOffset = " + groupName + ".offset;");
34943494
append(sb, indent, "final int " + groupName + "OriginalIndex = " + groupName + ".index;");
3495-
append(sb, indent, "final " + groupDecoderName + " " + groupName + " = " + groupName + "();");
3495+
append(sb, indent, "final " + groupDecoderName + " " + groupName + " = this." + groupName + "();");
34963496

34973497
append(sb, indent, "if (" + groupName + ".count() > 0)");
34983498
append(sb, indent, "{");
@@ -3575,9 +3575,9 @@ private int writeTokenDisplay(
35753575
if (typeToken.encoding().primitiveType() == PrimitiveType.CHAR)
35763576
{
35773577
append(sb, indent,
3578-
"for (int i = 0; i < " + fieldName + "Length() && " + fieldName + "(i) > 0; i++)");
3578+
"for (int i = 0; i < " + fieldName + "Length() && this." + fieldName + "(i) > 0; i++)");
35793579
append(sb, indent, "{");
3580-
append(sb, indent, " builder.append((char)" + fieldName + "(i));");
3580+
append(sb, indent, " builder.append((char)this." + fieldName + "(i));");
35813581
append(sb, indent, "}");
35823582
}
35833583
else
@@ -3587,7 +3587,7 @@ private int writeTokenDisplay(
35873587
append(sb, indent, "{");
35883588
append(sb, indent, " for (int i = 0; i < " + fieldName + "Length(); i++)");
35893589
append(sb, indent, " {");
3590-
append(sb, indent, " builder.append(" + fieldName + "(i));");
3590+
append(sb, indent, " builder.append(this." + fieldName + "(i));");
35913591
Separator.ENTRY.appendToGeneratedBuilder(sb, indent + INDENT + INDENT);
35923592
append(sb, indent, " }");
35933593
append(sb, indent, " builder.setLength(builder.length() - 1);");
@@ -3598,22 +3598,22 @@ private int writeTokenDisplay(
35983598
else
35993599
{
36003600
// have to duplicate because of checkstyle :/
3601-
append(sb, indent, "builder.append(" + fieldName + "());");
3601+
append(sb, indent, "builder.append(this." + fieldName + "());");
36023602
}
36033603
break;
36043604

36053605
case BEGIN_ENUM:
3606-
append(sb, indent, "builder.append(" + fieldName + "());");
3606+
append(sb, indent, "builder.append(this." + fieldName + "());");
36073607
break;
36083608

36093609
case BEGIN_SET:
3610-
append(sb, indent, fieldName + "().appendTo(builder);");
3610+
append(sb, indent, "this." + fieldName + "().appendTo(builder);");
36113611
break;
36123612

36133613
case BEGIN_COMPOSITE:
36143614
{
36153615
final String typeName = formatClassName(decoderName(typeToken.applicableTypeName()));
3616-
append(sb, indent, "final " + typeName + " " + fieldName + " = " + fieldName + "();");
3616+
append(sb, indent, "final " + typeName + " " + fieldName + " = this." + fieldName + "();");
36173617
append(sb, indent, "if (" + fieldName + " != null)");
36183618
append(sb, indent, "{");
36193619
append(sb, indent, " " + fieldName + ".appendTo(builder);");
@@ -3694,7 +3694,7 @@ private void generateMessageLength(
36943694
final String groupName = formatPropertyName(groupToken.name());
36953695
final String groupDecoderName = decoderName(groupToken.name());
36963696

3697-
append(sb, bodyIndent, groupDecoderName + " " + groupName + " = " + groupName + "();");
3697+
append(sb, bodyIndent, groupDecoderName + " " + groupName + " = this." + groupName + "();");
36983698
append(sb, bodyIndent, "if (" + groupName + ".count() > 0)");
36993699
append(sb, bodyIndent, "{");
37003700
append(sb, bodyIndent, " while (" + groupName + ".hasNext())");

‎sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/FrameCodecDecoder.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -623,13 +623,13 @@ public StringBuilder appendTo(final StringBuilder builder)
623623
builder.append(BLOCK_LENGTH);
624624
builder.append("):");
625625
builder.append("irId=");
626-
builder.append(irId());
626+
builder.append(this.irId());
627627
builder.append('|');
628628
builder.append("irVersion=");
629-
builder.append(irVersion());
629+
builder.append(this.irVersion());
630630
builder.append('|');
631631
builder.append("schemaVersion=");
632-
builder.append(schemaVersion());
632+
builder.append(this.schemaVersion());
633633
builder.append('|');
634634
builder.append("packageName=");
635635
builder.append('\'').append(packageName()).append('\'');

‎sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/MessageHeaderDecoder.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -217,16 +217,16 @@ public StringBuilder appendTo(final StringBuilder builder)
217217

218218
builder.append('(');
219219
builder.append("blockLength=");
220-
builder.append(blockLength());
220+
builder.append(this.blockLength());
221221
builder.append('|');
222222
builder.append("templateId=");
223-
builder.append(templateId());
223+
builder.append(this.templateId());
224224
builder.append('|');
225225
builder.append("schemaId=");
226-
builder.append(schemaId());
226+
builder.append(this.schemaId());
227227
builder.append('|');
228228
builder.append("version=");
229-
builder.append(version());
229+
builder.append(this.version());
230230
builder.append(')');
231231

232232
return builder;

‎sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/TokenCodecDecoder.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1724,34 +1724,34 @@ public StringBuilder appendTo(final StringBuilder builder)
17241724
builder.append(BLOCK_LENGTH);
17251725
builder.append("):");
17261726
builder.append("tokenOffset=");
1727-
builder.append(tokenOffset());
1727+
builder.append(this.tokenOffset());
17281728
builder.append('|');
17291729
builder.append("tokenSize=");
1730-
builder.append(tokenSize());
1730+
builder.append(this.tokenSize());
17311731
builder.append('|');
17321732
builder.append("fieldId=");
1733-
builder.append(fieldId());
1733+
builder.append(this.fieldId());
17341734
builder.append('|');
17351735
builder.append("tokenVersion=");
1736-
builder.append(tokenVersion());
1736+
builder.append(this.tokenVersion());
17371737
builder.append('|');
17381738
builder.append("componentTokenCount=");
1739-
builder.append(componentTokenCount());
1739+
builder.append(this.componentTokenCount());
17401740
builder.append('|');
17411741
builder.append("signal=");
1742-
builder.append(signal());
1742+
builder.append(this.signal());
17431743
builder.append('|');
17441744
builder.append("primitiveType=");
1745-
builder.append(primitiveType());
1745+
builder.append(this.primitiveType());
17461746
builder.append('|');
17471747
builder.append("byteOrder=");
1748-
builder.append(byteOrder());
1748+
builder.append(this.byteOrder());
17491749
builder.append('|');
17501750
builder.append("presence=");
1751-
builder.append(presence());
1751+
builder.append(this.presence());
17521752
builder.append('|');
17531753
builder.append("deprecated=");
1754-
builder.append(deprecated());
1754+
builder.append(this.deprecated());
17551755
builder.append('|');
17561756
builder.append("name=");
17571757
builder.append('\'').append(name()).append('\'');

‎sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/VarDataEncodingDecoder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ public StringBuilder appendTo(final StringBuilder builder)
139139

140140
builder.append('(');
141141
builder.append("length=");
142-
builder.append(length());
142+
builder.append(this.length());
143143
builder.append('|');
144144
builder.append(')');
145145

‎sbe-tool/src/test/java/uk/co/real_logic/sbe/generation/java/QualifiedYieldTest.java‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/*
2+
* Copyright 2013-2022 Real Logic Limited.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package uk.co.real_logic.sbe.generation.java;
17+
18+
import java.util.Map;
19+
20+
import org.agrona.DirectBuffer;
21+
import org.agrona.MutableDirectBuffer;
22+
import org.agrona.generation.CompilerUtil;
23+
import org.agrona.generation.StringWriterOutputManager;
24+
import org.junit.jupiter.api.Test;
25+
import org.junit.jupiter.api.condition.EnabledForJreRange;
26+
import org.junit.jupiter.api.condition.JRE;
27+
28+
import static org.junit.jupiter.api.Assertions.assertNotNull;
29+
30+
31+
import uk.co.real_logic.sbe.Tests;
32+
import uk.co.real_logic.sbe.ir.Ir;
33+
import uk.co.real_logic.sbe.xml.IrGenerator;
34+
import uk.co.real_logic.sbe.xml.MessageSchema;
35+
import uk.co.real_logic.sbe.xml.ParserOptions;
36+
37+
import static uk.co.real_logic.sbe.xml.XmlSchemaParser.parse;
38+
39+
public class QualifiedYieldTest
40+
{
41+
private static final Class<?> BUFFER_CLASS = MutableDirectBuffer.class;
42+
private static final String BUFFER_NAME = BUFFER_CLASS.getName();
43+
private static final Class<DirectBuffer> READ_ONLY_BUFFER_CLASS = DirectBuffer.class;
44+
private static final String READ_ONLY_BUFFER_NAME = READ_ONLY_BUFFER_CLASS.getName();
45+
46+
private final StringWriterOutputManager outputManager = new StringWriterOutputManager();
47+
48+
@Test
49+
@EnabledForJreRange(min = JRE.JAVA_17)
50+
void shouldGenerateValidJava() throws Exception
51+
{
52+
final ParserOptions options = ParserOptions.builder().stopOnError(true).build();
53+
final MessageSchema schema = parse(Tests.getLocalResource("issue910.xml"), options);
54+
final IrGenerator irg = new IrGenerator();
55+
final Ir ir = irg.generate(schema);
56+
final JavaGenerator generator = new JavaGenerator(
57+
ir, BUFFER_NAME, READ_ONLY_BUFFER_NAME, false, false, false, outputManager);
58+
59+
outputManager.setPackageName(ir.applicableNamespace());
60+
generator.generateMessageHeaderStub();
61+
generator.generateTypeStubs();
62+
generator.generate();
63+
64+
final Map<String, CharSequence> sources = outputManager.getSources();
65+
66+
{
67+
final String fqClassName = ir.applicableNamespace() + "." + "Issue910FieldDecoder";
68+
final Class<?> aClass = CompilerUtil.compileInMemory(fqClassName, sources);
69+
assertNotNull(aClass);
70+
}
71+
}
72+
}

‎sbe-tool/src/test/resources/issue910.xml‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
2+
<sbe:messageSchema xmlns:sbe="http://fixprotocol.io/2016/sbe"
3+
package="issue910"
4+
id="910"
5+
version="0"
6+
semanticVersion="1.0"
7+
description="issue 910 test case"
8+
byteOrder="bigEndian">
9+
<types>
10+
<composite name="messageHeader" description="Message identifiers and length of message root">
11+
<type name="blockLength" primitiveType="uint16"/>
12+
<type name="templateId" primitiveType="uint16"/>
13+
<type name="schemaId" primitiveType="uint16"/>
14+
<type name="version" primitiveType="uint16"/>
15+
</composite>
16+
<composite name="varDataEncoding" semanticType="Length">
17+
<type name="length" primitiveType="uint8" semanticType="Length"/>
18+
<type name="varData" primitiveType="char" semanticType="data"/>
19+
</composite>
20+
<composite name="groupSizeEncoding" description="Repeating group dimensions">
21+
<type name="blockLength" primitiveType="uint16"/>
22+
<type name="numInGroup" primitiveType="uint16"/>
23+
</composite>
24+
<enum name="Enum" encodingType="char">
25+
<validValue name="A">A</validValue>
26+
<validValue name="B">B</validValue>
27+
<validValue name="C">C</validValue>
28+
</enum>
29+
<set name="Set" encodingType="uint8">
30+
<choice name="A">0</choice>
31+
<choice name="B">1</choice>
32+
<choice name="C">2</choice>
33+
</set>
34+
<type name="Array" primitiveType="int32" length="5"/>
35+
<type name="CharArray" primitiveType="char" length="5"/>
36+
</types>
37+
<sbe:message name="Issue910Field" id="1" description="issue 910 field test">
38+
<field name="yield" type="uint64" id="1"/>
39+
</sbe:message>
40+
<sbe:message name="Issue910Vardata" id="2" description="issue 910 vardata test">
41+
<field name="yield" type="varDataEncoding" id="1"/>
42+
</sbe:message>
43+
<sbe:message name="Issue910Group" id="3" description="issue 910 group test">
44+
<group name="yield" id="1" dimensionType="groupSizeEncoding">
45+
<field name="whatever" type="uint64" id="1"/>
46+
</group>
47+
</sbe:message>
48+
<sbe:message name="Issue910GroupField" id="4" description="issue 910 group field test">
49+
<group name="whatever" id="1" dimensionType="groupSizeEncoding">
50+
<field name="yield" type="uint64" id="1"/>
51+
</group>
52+
</sbe:message>
53+
<sbe:message name="Issue910Enum" id="5" description="issue 910 enum test">
54+
<field name="yield" type="Enum" id="1"/>
55+
</sbe:message>
56+
<sbe:message name="Issue910Set" id="6" description="issue 910 set test">
57+
<field name="yield" type="Set" id="1"/>
58+
</sbe:message>
59+
<sbe:message name="Issue910Array" id="7" description="issue 910 array test">
60+
<field name="yield" type="Array" id="1"/>
61+
</sbe:message>
62+
<sbe:message name="Issue910CharArray" id="8" description="issue 910 char array test">
63+
<field name="yield" type="CharArray" id="1"/>
64+
</sbe:message>
65+
</sbe:messageSchema>

0 commit comments

Comments
 (0)