Skip to content

Commit e115b35

Browse files
committed
[C++] Port access order checking tests and implementation to C++.
In this commit, I also addressed these issues (1-3): --- 1. Tidy up string format usage. --- For convenience and readability, I had inlined some concatenation into `string.format(...)` templates; however, this makes it unclear whether the variable/method-output being concatenated uses template parameters. Therefore, I have adjusted to inject these strings via template parameters. FWIW I'd prefer to use `StringBuilder` everywhere, as I think it is hard to read large string format templates. --- 2. Explcitly delete copy constructors of flyweights. --- I found it easy to accidentally write sub-optimal code where I relied on copy constructors. For example: ``` MyMessage::MyGroup myGroup = myMessage.myGroup(10); ``` rather than: ``` MyMessage::MyGroup &myGroup = myMessage.myGroup(10); ``` Copying semantics are a bit strange when it comes to flyweights and trees of flyweights. Therefore, I think it is best to avoid entirely. This change is behind a feature flag. To re-enable implicit copy constructors specify the system property `-Dsbe.generate.implicit.copy.constructors=true`. --- 3. Adjust state machines to allow repeated decoding of variable-length --- data lengths, as accesses do not advance `limit`.
1 parent 58bb4f4 commit e115b35

16 files changed

+6699
-408
lines changed

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ out
3333
# cmake
3434
cmake-build-debug
3535
codecs
36+
.cmake
37+
CMakeFiles
38+
thirdparty
3639

3740
# cpp build linux
3841
cppbuild/CMakeCache.txt

csharp/sbe-tests/FieldAccessOrderCheckTests.cs

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,19 @@ public void DisallowsSkippingDecodingOfVariableLengthField3()
131131
Assert.IsTrue(exception.Message.Contains("Cannot access field \"c\" in state: V0_BLOCK"));
132132
}
133133

134+
[TestMethod]
135+
public void AllowsRepeatedDecodingOfVariableLengthDataLength()
136+
{
137+
var decoder = DecodeUntilVarLengthFields();
138+
Assert.AreEqual(3, decoder.BLength());
139+
Assert.AreEqual(3, decoder.BLength());
140+
Assert.AreEqual(3, decoder.BLength());
141+
Assert.AreEqual("abc", decoder.GetB());
142+
Assert.AreEqual(3, decoder.CLength());
143+
Assert.AreEqual(3, decoder.CLength());
144+
Assert.AreEqual(3, decoder.CLength());
145+
}
146+
134147
[TestMethod]
135148
public void DisallowsReDecodingEarlierVariableLengthField()
136149
{
@@ -2806,7 +2819,8 @@ public void DisallowsMissedEncodingOfVarLengthFieldInNestedGroupToNextOuterEleme
28062819
Assert.IsTrue(exception.Message.Contains("Cannot access next element in repeating group \"b\" in state: " +
28072820
expectedState));
28082821
Assert.IsTrue(
2809-
exception.Message.Contains("Expected one of these transitions: [\"b.d.e(?)\", \"b.d.f(?)\"]."));
2822+
exception.Message.Contains(
2823+
"Expected one of these transitions: [\"b.d.e(?)\", \"b.d.fLength()\", \"b.d.f(?)\"]."));
28102824
}
28112825

28122826
[TestMethod]
@@ -2841,7 +2855,8 @@ public void DisallowsMissedDecodingOfVarLengthFieldInNestedGroupToNextInnerEleme
28412855
exception.Message.Contains(
28422856
"Cannot access next element in repeating group \"b.d\" in state: V0_B_1_D_N_BLOCK."));
28432857
Assert.IsTrue(
2844-
exception.Message.Contains("Expected one of these transitions: [\"b.d.e(?)\", \"b.d.f(?)\"]."));
2858+
exception.Message.Contains(
2859+
"Expected one of these transitions: [\"b.d.e(?)\", \"b.d.fLength()\", \"b.d.f(?)\"]."));
28452860
}
28462861

28472862
[TestMethod]
@@ -2878,7 +2893,8 @@ public void DisallowsMissedDecodingOfVarLengthFieldInNestedGroupToNextInnerEleme
28782893
exception.Message.Contains(
28792894
"Cannot access next element in repeating group \"b.d\" in state: V0_B_N_D_N_BLOCK."));
28802895
Assert.IsTrue(
2881-
exception.Message.Contains("Expected one of these transitions: [\"b.d.e(?)\", \"b.d.f(?)\"]."));
2896+
exception.Message.Contains(
2897+
"Expected one of these transitions: [\"b.d.e(?)\", \"b.d.fLength()\", \"b.d.f(?)\"]."));
28822898
}
28832899

28842900
[TestMethod]
@@ -2915,7 +2931,8 @@ public void DisallowsMissedDecodingOfVarLengthFieldInNestedGroupToNextOuterEleme
29152931
exception.Message.Contains(
29162932
"Cannot access next element in repeating group \"b\" in state: V0_B_N_D_N_BLOCK."));
29172933
Assert.IsTrue(
2918-
exception.Message.Contains("Expected one of these transitions: [\"b.d.e(?)\", \"b.d.f(?)\"]."));
2934+
exception.Message.Contains(
2935+
"Expected one of these transitions: [\"b.d.e(?)\", \"b.d.fLength()\", \"b.d.f(?)\"]."));
29192936
}
29202937

29212938
[TestMethod]
@@ -2949,7 +2966,8 @@ public void DisallowsMissedDecodingOfVarLengthFieldInNestedGroupToNextOuterEleme
29492966
exception.Message.Contains(
29502967
"Cannot access next element in repeating group \"b\" in state: V0_B_N_D_1_BLOCK."));
29512968
Assert.IsTrue(
2952-
exception.Message.Contains("Expected one of these transitions: [\"b.d.e(?)\", \"b.d.f(?)\"]."));
2969+
exception.Message.Contains(
2970+
"Expected one of these transitions: [\"b.d.e(?)\", \"b.d.fLength()\", \"b.d.f(?)\"]."));
29532971
}
29542972

29552973
[TestMethod]
@@ -2962,7 +2980,7 @@ public void DisallowsIncompleteMessagesDueToMissingVarLengthField1()
29622980
var exception = Assert.ThrowsException<InvalidOperationException>(() => encoder.CheckEncodingIsComplete());
29632981
Assert.IsTrue(
29642982
exception.Message.Contains(
2965-
"Not fully encoded, current state: V0_B_DONE, allowed transitions: \"c(?)\""));
2983+
"Not fully encoded, current state: V0_B_DONE, allowed transitions: \"cLength()\", \"c(?)\""));
29662984
}
29672985

29682986
[TestMethod]
@@ -2973,7 +2991,7 @@ public void DisallowsIncompleteMessagesDueToMissingVarLengthField2()
29732991
var exception = Assert.ThrowsException<InvalidOperationException>(() => encoder.CheckEncodingIsComplete());
29742992
Assert.IsTrue(
29752993
exception.Message.Contains(
2976-
"Not fully encoded, current state: V0_BLOCK, allowed transitions: \"a(?)\""));
2994+
"Not fully encoded, current state: V0_BLOCK, allowed transitions: \"aLength()\", \"a(?)\""));
29772995
}
29782996

29792997
[TestMethod]

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

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,15 @@ public void forEachWrappedStateByVersion(final IntObjConsumer<State> consumer)
116116
}
117117
}
118118

119+
/**
120+
* Returns the number of schema versions.
121+
* @return the number of schema versions.
122+
*/
123+
public int versionCount()
124+
{
125+
return versionWrappedStates.size();
126+
}
127+
119128
/**
120129
* Iterates over the states in which a codec is fully encoded.
121130
* @param consumer the consumer of the states.
@@ -136,6 +145,15 @@ public void forEachStateOrderedByStateNumber(final Consumer<State> consumer)
136145
.forEach(consumer);
137146
}
138147

148+
/**
149+
* Returns the number of states in the state machine.
150+
* @return the number of states in the state machine.
151+
*/
152+
public int stateCount()
153+
{
154+
return transitionsByState.size();
155+
}
156+
139157
/**
140158
* Returns a hash-consing factory for codec interactions.
141159
* These interactions are the transitions in the state machine.
@@ -541,6 +559,12 @@ public void onVarData(final Token token)
541559
{
542560
if (filter.test(token))
543561
{
562+
final CodecInteraction lengthAccessInteraction = interactionFactory.accessVarDataLength(token);
563+
currentStates.forEach(state ->
564+
{
565+
allocateTransitions(lengthAccessInteraction, Collections.singleton(state), state);
566+
});
567+
544568
final CodecInteraction codecInteraction = interactionFactory.accessField(token);
545569
final State accessedState = allocateState(statePrefix + token.name().toUpperCase() + "_DONE");
546570
allocateTransitions(codecInteraction, currentStates, accessedState);
@@ -635,6 +659,15 @@ public State endState()
635659
return to;
636660
}
637661

662+
/**
663+
* Returns {@code true} if the transitions in this group do not change state.
664+
* @return {@code true} if the transitions in this group do not change state.
665+
*/
666+
public boolean alwaysEndsInStartState()
667+
{
668+
return from.size() == 1 && from.contains(to);
669+
}
670+
638671
/**
639672
* Returns some example code for the codec interaction that the transitions in this group share.
640673
* Useful for producing error messages.
@@ -902,6 +935,42 @@ String exampleConditions()
902935
}
903936
}
904937

938+
/**
939+
* When the length of a variable length field is accessed without adjusting the position.
940+
*/
941+
private static final class AccessVarDataLength extends CodecInteraction
942+
{
943+
private final String groupPath;
944+
private final Token token;
945+
946+
private AccessVarDataLength(final String groupPath, final Token token)
947+
{
948+
assert groupPath != null;
949+
assert token.signal() == Signal.BEGIN_VAR_DATA;
950+
this.groupPath = groupPath;
951+
this.token = token;
952+
}
953+
954+
@Override
955+
public String groupQualifiedName()
956+
{
957+
return groupPath + token.name();
958+
}
959+
960+
@Override
961+
String exampleCode()
962+
{
963+
return groupPath + token.name() + "Length()";
964+
}
965+
966+
@Override
967+
String exampleConditions()
968+
{
969+
return "";
970+
}
971+
}
972+
973+
905974
/**
906975
* Factory for creating {@link CodecInteraction} instances. This factory
907976
* is used to hash-cons the instances, so that they can be compared by
@@ -915,6 +984,7 @@ public static final class HashConsingFactory
915984
private final Map<Token, CodecInteraction> determineGroupHasElementsInteractions = new HashMap<>();
916985
private final Map<Token, CodecInteraction> moveToNextElementInteractions = new HashMap<>();
917986
private final Map<Token, CodecInteraction> moveToLastElementInteractions = new HashMap<>();
987+
private final Map<Token, CodecInteraction> accessVarDataLengthInteractions = new HashMap<>();
918988
private final Map<Token, String> groupPathsByField;
919989
private final Set<Token> topLevelBlockFields;
920990

@@ -1029,6 +1099,24 @@ public CodecInteraction moveToLastElement(final Token token)
10291099
return moveToLastElementInteractions.computeIfAbsent(token,
10301100
t -> new MoveToLastElement(groupPathsByField.get(t), t));
10311101
}
1102+
1103+
/**
1104+
* Find or create a {@link CodecInteraction} to represent accessing the length
1105+
* of a variable-length data field without advancing the codec position.
1106+
*
1107+
* <p>For decoders and codecs, this will be when the length is accessed, e.g.,
1108+
* {@code decoder.myVarDataLength()}.
1109+
*
1110+
* <p>The supplied token must carry a {@link Signal#BEGIN_VAR_DATA} signal.
1111+
*
1112+
* @param token the token identifying the field
1113+
* @return the {@link CodecInteraction} instance
1114+
*/
1115+
public CodecInteraction accessVarDataLength(final Token token)
1116+
{
1117+
return accessVarDataLengthInteractions.computeIfAbsent(token,
1118+
t -> new AccessVarDataLength(groupPathsByField.get(t), t));
1119+
}
10321120
}
10331121
}
10341122
}

0 commit comments

Comments
 (0)