Skip to content

Commit 11afc92

Browse files
ZachBrayZachBray
committed
[Java,C#,C++] Allow custom flag/properties to enable precedence checks.
Some users would like the ability to enable precedence checks on some schemas but not others. Initially, I considered generating flags or system properties with the schema namespace; however, this forces the more-complex scheme on all users. It also might get unwieldly with long namespaces. It also assumes a one-to-one correspondence between namespaces and schemas. Instead, I have introduced two new system properties that can be applied at code generation time: - `sbe.precedence.checks.flagName`, which controls the symbol/macro used to enable precedence checks at build time in the generated C#/C++ code. It defaults to `"SBE_ENABLE_PRECEDENCE_CHECKS"`. - `sbe.precedence.checks.propName`, which controls the property name used to enable precedence checks at runtime in the generated Java code. It defaults to `"sbe.enable.precedence.checks"`. Now, users can run SbeTool with different values for these system properties to generate code with different enablement flags. Note above that the defaults are: - `SBE_ENABLE_PRECEDENCE_CHECKS` rather than `SBE_ENABLE_SEQUENCING_CHECKS`, and - `sbe.enable.precedence.checks` rather than `sbe.enable.sequencing.checks`. These changes and a change to another system property: - `sbe.generate.sequencing.checks` -> `sbe.generate.precedence.checks` address some feedback from Martin on the associated PR #948.
1 parent 69d13b6 commit 11afc92

File tree

10 files changed

+222
-195
lines changed

10 files changed

+222
-195
lines changed

‎build.gradle

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,9 @@ subprojects {
229229
}
230230

231231
javaLauncher.set(toolchainLauncher)
232+
233+
systemProperty 'sbe.enable.ir.precedence.checks', 'true'
234+
systemProperty 'sbe.enable.test.precedence.checks', 'true'
232235
}
233236
}
234237

@@ -280,7 +283,8 @@ project(':sbe-tool') {
280283
'sbe.target.language': 'Java',
281284
'sbe.validation.stop.on.error': 'true',
282285
'sbe.validation.xsd': validationXsdPath,
283-
'sbe.generate.sequencing.checks': 'true')
286+
'sbe.generate.precedence.checks': 'true',
287+
'sbe.precedence.checks.propName': 'sbe.enable.test.precedence.checks')
284288
args = ['src/test/resources/json-printer-test-schema.xml',
285289
'src/test/resources/composite-elements-schema.xml',
286290
'src/test/resources/field-order-check-schema.xml']
@@ -544,7 +548,7 @@ project(':sbe-benchmarks') {
544548
'sbe.validation.xsd': validationXsdPath,
545549
'sbe.java.encoding.buffer.type': 'org.agrona.concurrent.UnsafeBuffer',
546550
'sbe.java.decoding.buffer.type': 'org.agrona.concurrent.UnsafeBuffer',
547-
'sbe.generate.sequencing.checks': 'false')
551+
'sbe.generate.precedence.checks': 'false')
548552
args = ['src/main/resources/car.xml', 'src/main/resources/fix-message-samples.xml']
549553
}
550554

@@ -734,7 +738,7 @@ tasks.register('generateCSharpCodecsTests', JavaExec) {
734738
'sbe.target.language': 'uk.co.real_logic.sbe.generation.csharp.CSharp',
735739
'sbe.xinclude.aware': 'true',
736740
'sbe.validation.xsd': validationXsdPath,
737-
'sbe.generate.sequencing.checks': 'true')
741+
'sbe.generate.precedence.checks': 'true')
738742
args = ['sbe-tool/src/test/resources/FixBinary.xml',
739743
'sbe-tool/src/test/resources/issue435.xml',
740744
'sbe-tool/src/test/resources/issue483.xml',
@@ -759,7 +763,9 @@ tasks.register('generateJavaIrCodecs', JavaExec) {
759763
'sbe.output.dir': 'sbe-tool/src/main/java',
760764
'sbe.target.language': 'Java',
761765
'sbe.validation.xsd': validationXsdPath,
762-
'sbe.generate.sequencing.checks': 'true')
766+
'sbe.generate.precedence.checks': 'true',
767+
'sbe.precedence.checks.flagName': 'SBE_ENABLE_IR_PRECEDENCE_CHECKS',
768+
'sbe.precedence.checks.propName': 'sbe.enable.ir.precedence.checks')
763769
args = ['sbe-tool/src/main/resources/sbe-ir.xml']
764770
}
765771

@@ -770,7 +776,8 @@ tasks.register('generateCppIrCodecs', JavaExec) {
770776
'sbe.output.dir': 'sbe-tool/src/main/cpp',
771777
'sbe.target.language': 'cpp',
772778
'sbe.validation.xsd': validationXsdPath,
773-
'sbe.generate.sequencing.checks': 'true')
779+
'sbe.generate.precedence.checks': 'true',
780+
'sbe.precedence.checks.flagName': 'SBE_ENABLE_IR_PRECEDENCE_CHECKS')
774781
args = ['sbe-tool/src/main/resources/sbe-ir.xml']
775782
}
776783

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,26 @@
4040
// Therefore, we allow lambdas with code blocks even when a lambda expression is possible.
4141
public final class AccessOrderModel
4242
{
43+
/**
44+
* Whether to generate access order checks.
45+
*/
4346
private static final boolean GENERATE_ACCESS_ORDER_CHECKS = Boolean.parseBoolean(
44-
System.getProperty("sbe.generate.sequencing.checks", "false"));
47+
System.getProperty("sbe.generate.precedence.checks", "false"));
48+
49+
/**
50+
* The name of the symbol or macro that enables access order checks when building
51+
* generated C# or C++ code.
52+
*/
53+
public static final String PRECEDENCE_CHECKS_FLAG_NAME =
54+
System.getProperty("sbe.precedence.checks.flagName", "SBE_ENABLE_PRECEDENCE_CHECKS");
55+
56+
/**
57+
* The name of the system property that enables access order checks at runtime
58+
* in generated Java code.
59+
*/
60+
public static final String PRECEDENCE_CHECKS_PROP_NAME =
61+
System.getProperty("sbe.precedence.checks.propName", "sbe.enable.precedence.checks");
62+
4563
private final Map<Token, String> groupPathsByField = new HashMap<>();
4664
private final Set<Token> topLevelBlockFields = new HashSet<>();
4765
private final CodecInteraction.HashConsingFactory interactionFactory =

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ private static CharSequence generateFullyEncodedCheck(final AccessOrderModel acc
207207

208208
sb.append(indent).append("void checkEncodingIsComplete()\n")
209209
.append(indent).append("{\n")
210-
.append("#if defined(SBE_ENABLE_SEQUENCING_CHECKS)\n")
210+
.append("#if defined(").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append(")\n")
211211
.append(indent).append(INDENT).append("switch (m_codecState)\n")
212212
.append(indent).append(INDENT).append("{\n");
213213

@@ -321,7 +321,7 @@ private static CharSequence generateAccessOrderListenerCall(
321321
}
322322

323323
final StringBuilder sb = new StringBuilder();
324-
sb.append("#if defined(SBE_ENABLE_SEQUENCING_CHECKS)\n")
324+
sb.append("#if defined(").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append(")\n")
325325
.append(indent).append(methodName).append("(");
326326

327327
for (int i = 0; i < arguments.length; i++)
@@ -3083,7 +3083,7 @@ private CharSequence generateDecoderWrapListenerCall(final AccessOrderModel acce
30833083
if (accessOrderModel.versionCount() == 1)
30843084
{
30853085
final StringBuilder sb = new StringBuilder();
3086-
sb.append("#if defined(SBE_ENABLE_SEQUENCING_CHECKS)\n")
3086+
sb.append("#if defined(").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append(")\n")
30873087
.append(TWO_INDENT).append("codecState(")
30883088
.append(qualifiedStateCase(accessOrderModel.latestVersionWrappedState())).append(");\n")
30893089
.append("#endif\n");
@@ -3101,7 +3101,7 @@ private CharSequence generateEncoderWrapListener(final AccessOrderModel accessOr
31013101
}
31023102

31033103
final StringBuilder sb = new StringBuilder();
3104-
sb.append("#if defined(SBE_ENABLE_SEQUENCING_CHECKS)\n")
3104+
sb.append("#if defined(").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append(")\n")
31053105
.append(TWO_INDENT).append("codecState(")
31063106
.append(qualifiedStateCase(accessOrderModel.latestVersionWrappedState()))
31073107
.append(");\n")

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1583,7 +1583,7 @@ private static CharSequence generateFullyEncodedCheck(
15831583

15841584
sb.append(indent).append("public void CheckEncodingIsComplete()\n")
15851585
.append(indent).append("{\n")
1586-
.append("#if SBE_ENABLE_SEQUENCING_CHECKS\n")
1586+
.append("#if ").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append("\n")
15871587
.append(indent).append(INDENT).append("switch (_codecState)\n")
15881588
.append(indent).append(INDENT).append("{\n");
15891589

@@ -1670,7 +1670,7 @@ private static CharSequence generateAccessOrderListenerCall(
16701670
}
16711671

16721672
final StringBuilder sb = new StringBuilder();
1673-
sb.append("#if SBE_ENABLE_SEQUENCING_CHECKS\n")
1673+
sb.append("#if ").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append("\n")
16741674
.append(indent).append(methodName).append("(");
16751675

16761676
for (int i = 0; i < arguments.length; i++)
@@ -1936,7 +1936,7 @@ private CharSequence generateEncoderWrapListener(
19361936
}
19371937

19381938
final StringBuilder sb = new StringBuilder();
1939-
sb.append("#if SBE_ENABLE_SEQUENCING_CHECKS\n")
1939+
sb.append("#if ").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append("\n")
19401940
.append(indent).append("codecState(")
19411941
.append(qualifiedStateCase(accessOrderModel.latestVersionWrappedState()))
19421942
.append(");\n")
@@ -2556,7 +2556,7 @@ private CharSequence generateDisplay(
25562556
append(sb, TWO_INDENT, " int originalLimit = this.Limit;");
25572557
if (null != accessOrderModel)
25582558
{
2559-
sb.append("#if SBE_ENABLE_SEQUENCING_CHECKS\n");
2559+
sb.append("#if ").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append("\n");
25602560
append(sb, TWO_INDENT, " CodecState originalState = _codecState;");
25612561
sb.append(THREE_INDENT).append("_codecState = ")
25622562
.append(qualifiedStateCase(accessOrderModel.notWrappedState())).append(";\n");
@@ -2588,7 +2588,7 @@ private CharSequence generateDisplay(
25882588
sb.append('\n');
25892589
if (null != accessOrderModel)
25902590
{
2591-
sb.append("#if SBE_ENABLE_SEQUENCING_CHECKS\n");
2591+
sb.append("#if ").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append("\n");
25922592
append(sb, TWO_INDENT, " _codecState = originalState;");
25932593
sb.append("#endif\n");
25942594
}

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -307,9 +307,10 @@ private static CharSequence generateFieldOrderStates(final AccessOrderModel acce
307307

308308
sb.append(" private static final boolean ENABLE_BOUNDS_CHECKS = ")
309309
.append("!Boolean.getBoolean(\"agrona.disable.bounds.checks\");\n\n");
310-
sb.append(" private static final boolean SBE_ENABLE_SEQUENCING_CHECKS = ")
310+
sb.append(" private static final boolean ")
311+
.append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append(" = ")
311312
.append("Boolean.parseBoolean(System.getProperty(\n")
312-
.append(" \"sbe.enable.sequencing.checks\",\n")
313+
.append(" \"").append(AccessOrderModel.PRECEDENCE_CHECKS_PROP_NAME).append("\",\n")
313314
.append(" Boolean.toString(ENABLE_BOUNDS_CHECKS)));\n\n");
314315

315316
sb.append(" /**\n");
@@ -410,7 +411,7 @@ private static void generateFullyEncodedCheck(
410411

411412
sb.append(" public void checkEncodingIsComplete()\n")
412413
.append(" {\n")
413-
.append(" if (SBE_ENABLE_SEQUENCING_CHECKS)\n")
414+
.append(" if (").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append(")\n")
414415
.append(" {\n")
415416
.append(" switch (codecState)\n")
416417
.append(" {\n");
@@ -493,7 +494,7 @@ private static CharSequence generateAccessOrderListenerCall(
493494
}
494495

495496
final StringBuilder sb = new StringBuilder();
496-
sb.append(indent).append("if (SBE_ENABLE_SEQUENCING_CHECKS)\n")
497+
sb.append(indent).append("if (").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append(")\n")
497498
.append(indent).append("{\n")
498499
.append(indent).append(" ").append(methodName).append("(");
499500

@@ -755,7 +756,7 @@ private CharSequence generateEncoderWrapListener(
755756
}
756757

757758
final StringBuilder sb = new StringBuilder();
758-
sb.append(indent).append("if (SBE_ENABLE_SEQUENCING_CHECKS)")
759+
sb.append(indent).append("if (").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append(")")
759760
.append("\n").append(indent).append("{\n")
760761
.append(indent).append(" codecState(")
761762
.append(qualifiedStateCase(accessOrderModel.latestVersionWrappedState()))
@@ -3424,7 +3425,7 @@ private CharSequence generateDecoderFlyweightCode(
34243425

34253426
if (null != accessOrderModel)
34263427
{
3427-
methods.append(" if (SBE_ENABLE_SEQUENCING_CHECKS)\n")
3428+
methods.append(" if (").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append(")\n")
34283429
.append(" {\n")
34293430
.append(" codecState(currentCodecState);\n")
34303431
.append(" }\n\n");

0 commit comments

Comments
 (0)