Skip to content

Commit d3f9abc

Browse files
ZachBrayZachBray
committed
[Java] Treat top-level block fields as a special case.
To reduce the number of false negatives the field access order checker generates, in this commit, we treat the top-level block fields differently. We know that once the top-level encoder is wrapped, these fields are always safe to access. As a result, we were able to enable several tests that previously failed.
1 parent 02268d7 commit d3f9abc

File tree

7 files changed

+140
-240
lines changed

7 files changed

+140
-240
lines changed

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

Lines changed: 48 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ final class FieldOrderModel
3636
{
3737
private final Int2ObjectHashMap<State> states = new Int2ObjectHashMap<>();
3838
private final Map<Token, TransitionGroup> transitions = new LinkedHashMap<>();
39+
private final Set<Token> topLevelBlockFields = new HashSet<>();
3940
private final Int2ObjectHashMap<State> versionWrappedStates = new Int2ObjectHashMap<>();
4041
private final Set<String> reservedNames = new HashSet<>();
4142
private final State notWrappedState = allocateState("NOT_WRAPPED");
@@ -69,6 +70,11 @@ public void forEachState(final Consumer<State> consumer)
6970
.forEach(consumer);
7071
}
7172

73+
public boolean isTopLevelBlockField(final Token token)
74+
{
75+
return topLevelBlockFields.contains(token);
76+
}
77+
7278
public List<Transition> getTransitions(final TransitionContext context, final Token token)
7379
{
7480
final TransitionGroup transitionGroup = transitions.get(token);
@@ -80,45 +86,14 @@ public List<Transition> getTransitions(final TransitionContext context, final To
8086
return transitionGroup.transitions.get(context);
8187
}
8288

83-
public static FieldOrderModel findTransitions(
89+
public static FieldOrderModel newInstance(
8490
final Token msgToken,
8591
final List<Token> fields,
8692
final List<Token> groups,
8793
final List<Token> varData)
8894
{
8995
final FieldOrderModel model = new FieldOrderModel();
90-
91-
final IntHashSet versions = new IntHashSet();
92-
versions.add(msgToken.version());
93-
findVersions(versions, fields, groups, varData);
94-
95-
versions.stream().sorted().forEach(version ->
96-
{
97-
final State versionWrappedState = model.allocateState("V" + version + "_BLOCK");
98-
99-
model.versionWrappedStates.put(version, versionWrappedState);
100-
101-
model.encoderWrappedState = versionWrappedState;
102-
103-
model.allocateTransition(
104-
version,
105-
".wrap(...)",
106-
null,
107-
Collections.singletonList(model.notWrappedState),
108-
versionWrappedState
109-
);
110-
111-
model.findTransitions(
112-
Collections.singletonList(versionWrappedState),
113-
versionWrappedState,
114-
"V" + version + "_",
115-
fields,
116-
groups,
117-
varData,
118-
token -> token.version() <= version
119-
);
120-
});
121-
96+
model.findTransitions(msgToken, fields, groups, varData);
12297
return model;
12398
}
12499

@@ -192,6 +167,46 @@ private static void findVersions(
192167
}
193168
}
194169

170+
private void findTransitions(
171+
final Token msgToken,
172+
final List<Token> fields,
173+
final List<Token> groups,
174+
final List<Token> varData)
175+
{
176+
final IntHashSet versions = new IntHashSet();
177+
versions.add(msgToken.version());
178+
findVersions(versions, fields, groups, varData);
179+
180+
Generators.forEachField(fields, (fieldToken, ignored) -> topLevelBlockFields.add(fieldToken));
181+
182+
versions.stream().sorted().forEach(version ->
183+
{
184+
final State versionWrappedState = allocateState("V" + version + "_BLOCK");
185+
186+
versionWrappedStates.put(version, versionWrappedState);
187+
188+
encoderWrappedState = versionWrappedState;
189+
190+
allocateTransition(
191+
version,
192+
".wrap(...)",
193+
null,
194+
Collections.singletonList(notWrappedState),
195+
versionWrappedState
196+
);
197+
198+
findTransitions(
199+
Collections.singletonList(versionWrappedState),
200+
versionWrappedState,
201+
"V" + version + "_",
202+
fields,
203+
groups,
204+
varData,
205+
token -> token.version() <= version
206+
);
207+
});
208+
}
209+
195210
@SuppressWarnings("checkstyle:MethodLength")
196211
private List<State> findTransitions(
197212
final List<State> entryStates,

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

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,7 @@ private void generateEncoder(
250250
generateAnnotations(BASE_INDENT, className, groups, out, this::encoderName);
251251
}
252252
out.append(generateDeclaration(className, implementsString, msgToken));
253-
final FieldOrderModel fieldOrderModel = FieldOrderModel.findTransitions(
254-
msgToken, fields, groups, varData);
253+
final FieldOrderModel fieldOrderModel = FieldOrderModel.newInstance(msgToken, fields, groups, varData);
255254
out.append(generateFieldOrderStates(fieldOrderModel));
256255
out.append(generateEncoderFlyweightCode(className, fieldOrderModel, msgToken));
257256

@@ -390,25 +389,39 @@ private static void generateFieldOrderStateTransitions(
390389
final FieldOrderModel.TransitionContext context,
391390
final Token token)
392391
{
393-
sb.append(indent).append("switch (codecState())\n")
394-
.append(indent).append("{\n");
392+
if (fieldOrderModel.isTopLevelBlockField(token))
393+
{
394+
sb.append(indent).append("if (codecState() == ")
395+
.append(qualifiedStateCase(fieldOrderModel.notWrappedState()))
396+
.append(")\n")
397+
.append(indent).append("{\n")
398+
.append(indent).append(" throw new IllegalStateException(")
399+
.append("\"Cannot access field \\\"").append(token.name())
400+
.append("\\\" in state: \" + codecState());\n")
401+
.append(indent).append("}\n");
402+
}
403+
else
404+
{
405+
sb.append(indent).append("switch (codecState())\n")
406+
.append(indent).append("{\n");
395407

396-
final List<FieldOrderModel.Transition> transitions = fieldOrderModel.getTransitions(context, token);
408+
final List<FieldOrderModel.Transition> transitions = fieldOrderModel.getTransitions(context, token);
397409

398-
transitions.forEach(transition ->
399-
{
400-
transition.forEachStartState(startState ->
401-
sb.append(indent).append(" case ").append(unqualifiedStateCase(startState)).append(":\n"));
402-
sb.append(indent).append(" codecState(")
403-
.append(qualifiedStateCase(transition.endState())).append(");\n")
404-
.append(indent).append(" break;\n");
405-
});
410+
transitions.forEach(transition ->
411+
{
412+
transition.forEachStartState(startState ->
413+
sb.append(indent).append(" case ").append(unqualifiedStateCase(startState)).append(":\n"));
414+
sb.append(indent).append(" codecState(")
415+
.append(qualifiedStateCase(transition.endState())).append(");\n")
416+
.append(indent).append(" break;\n");
417+
});
406418

407-
sb.append(indent).append(" default:\n")
408-
.append(indent).append(" throw new IllegalStateException(")
409-
.append("\"Cannot access field \\\"").append(token.name())
410-
.append("\\\" in state: \" + codecState());\n")
411-
.append(indent).append("}\n");
419+
sb.append(indent).append(" default:\n")
420+
.append(indent).append(" throw new IllegalStateException(")
421+
.append("\"Cannot access field \\\"").append(token.name())
422+
.append("\\\" in state: \" + codecState());\n")
423+
.append(indent).append("}\n");
424+
}
412425
}
413426

414427
private static CharSequence generateFieldOrderStateTransitionsForNextGroupElement(
@@ -499,7 +512,7 @@ private void generateDecoder(
499512
generateAnnotations(BASE_INDENT, className, groups, out, this::decoderName);
500513
}
501514
out.append(generateDeclaration(className, implementsString, msgToken));
502-
final FieldOrderModel fieldOrderModel = FieldOrderModel.findTransitions(
515+
final FieldOrderModel fieldOrderModel = FieldOrderModel.newInstance(
503516
msgToken, fields, groups, varData);
504517
out.append(generateFieldOrderStates(fieldOrderModel));
505518
out.append(generateDecoderFlyweightCode(fieldOrderModel, className, msgToken));

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

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -243,13 +243,9 @@ public int irId()
243243
{
244244
if (DEBUG_MODE)
245245
{
246-
switch (codecState())
246+
if (codecState() == CodecState.NOT_WRAPPED)
247247
{
248-
case V0_BLOCK:
249-
codecState(CodecState.V0_BLOCK);
250-
break;
251-
default:
252-
throw new IllegalStateException("Cannot access field \"irId\" in state: " + codecState());
248+
throw new IllegalStateException("Cannot access field \"irId\" in state: " + codecState());
253249
}
254250
}
255251

@@ -306,13 +302,9 @@ public int irVersion()
306302
{
307303
if (DEBUG_MODE)
308304
{
309-
switch (codecState())
305+
if (codecState() == CodecState.NOT_WRAPPED)
310306
{
311-
case V0_BLOCK:
312-
codecState(CodecState.V0_BLOCK);
313-
break;
314-
default:
315-
throw new IllegalStateException("Cannot access field \"irVersion\" in state: " + codecState());
307+
throw new IllegalStateException("Cannot access field \"irVersion\" in state: " + codecState());
316308
}
317309
}
318310

@@ -369,13 +361,9 @@ public int schemaVersion()
369361
{
370362
if (DEBUG_MODE)
371363
{
372-
switch (codecState())
364+
if (codecState() == CodecState.NOT_WRAPPED)
373365
{
374-
case V0_BLOCK:
375-
codecState(CodecState.V0_BLOCK);
376-
break;
377-
default:
378-
throw new IllegalStateException("Cannot access field \"schemaVersion\" in state: " + codecState());
366+
throw new IllegalStateException("Cannot access field \"schemaVersion\" in state: " + codecState());
379367
}
380368
}
381369

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

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -200,13 +200,9 @@ public FrameCodecEncoder irId(final int value)
200200
{
201201
if (DEBUG_MODE)
202202
{
203-
switch (codecState())
203+
if (codecState() == CodecState.NOT_WRAPPED)
204204
{
205-
case V0_BLOCK:
206-
codecState(CodecState.V0_BLOCK);
207-
break;
208-
default:
209-
throw new IllegalStateException("Cannot access field \"irId\" in state: " + codecState());
205+
throw new IllegalStateException("Cannot access field \"irId\" in state: " + codecState());
210206
}
211207
}
212208

@@ -264,13 +260,9 @@ public FrameCodecEncoder irVersion(final int value)
264260
{
265261
if (DEBUG_MODE)
266262
{
267-
switch (codecState())
263+
if (codecState() == CodecState.NOT_WRAPPED)
268264
{
269-
case V0_BLOCK:
270-
codecState(CodecState.V0_BLOCK);
271-
break;
272-
default:
273-
throw new IllegalStateException("Cannot access field \"irVersion\" in state: " + codecState());
265+
throw new IllegalStateException("Cannot access field \"irVersion\" in state: " + codecState());
274266
}
275267
}
276268

@@ -328,13 +320,9 @@ public FrameCodecEncoder schemaVersion(final int value)
328320
{
329321
if (DEBUG_MODE)
330322
{
331-
switch (codecState())
323+
if (codecState() == CodecState.NOT_WRAPPED)
332324
{
333-
case V0_BLOCK:
334-
codecState(CodecState.V0_BLOCK);
335-
break;
336-
default:
337-
throw new IllegalStateException("Cannot access field \"schemaVersion\" in state: " + codecState());
325+
throw new IllegalStateException("Cannot access field \"schemaVersion\" in state: " + codecState());
338326
}
339327
}
340328

0 commit comments

Comments
 (0)