Skip to content

Commit 3d4d347

Browse files
committed
SuccessorType: Address review comments.
1 parent 4e70627 commit 3d4d347

File tree

4 files changed

+50
-54
lines changed

4 files changed

+50
-54
lines changed

csharp/ql/lib/semmle/code/csharp/controlflow/internal/Splitting.qll

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -470,12 +470,9 @@ module FinallySplitting {
470470
* then the `finally` block must end with a `return` as well (provided that
471471
* the `finally` block exits normally).
472472
*/
473-
class FinallySplitType instanceof Cfg::SuccessorType {
473+
class FinallySplitType extends Cfg::SuccessorType {
474474
FinallySplitType() { not this instanceof Cfg::ConditionalSuccessor }
475475

476-
/** Gets a textual representation of this successor type. */
477-
string toString() { result = super.toString() }
478-
479476
/** Holds if this split type matches entry into a `finally` block with completion `c`. */
480477
predicate isSplitForEntryCompletion(Completion c) {
481478
if c instanceof NormalCompletion

ruby/ql/lib/codeql/ruby/controlflow/internal/Splitting.qll

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,12 +122,9 @@ module EnsureSplitting {
122122
* the `ensure` block must end with a `return` as well (provided that
123123
* the `ensure` block executes normally).
124124
*/
125-
class EnsureSplitType instanceof SuccessorType {
125+
class EnsureSplitType extends SuccessorType {
126126
EnsureSplitType() { not this instanceof ConditionalSuccessor }
127127

128-
/** Gets a textual representation of this successor type. */
129-
string toString() { result = super.toString() }
130-
131128
/** Holds if this split type matches entry into an `ensure` block with completion `c`. */
132129
predicate isSplitForEntryCompletion(Completion c) {
133130
if c instanceof NormalCompletion

shared/controlflow/codeql/controlflow/Cfg.qll

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -945,6 +945,12 @@ module MakeWithSplitting<
945945
)
946946
}
947947

948+
/** Holds if `t` is an abnormal exit type out of a CFG scope. */
949+
private predicate isAbnormalExitType(SuccessorType t) {
950+
t instanceof ExceptionSuccessor or
951+
t instanceof ExitSuccessor
952+
}
953+
948954
/**
949955
* Internal representation of control flow nodes in the control flow graph.
950956
* The control flow graph is pruned for unreachable nodes.

shared/controlflow/codeql/controlflow/SuccessorType.qll

Lines changed: 42 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,8 @@
11
/**
22
* Provides different types of control flow successor types. These are used as
33
* edge labels in the control flow graph.
4-
*/
5-
overlay[local]
6-
module;
7-
8-
private import codeql.util.Boolean
9-
10-
/*
4+
*
5+
* ```text
116
* SuccessorType
127
* |- NormalSuccessor
138
* | |- DirectSuccessor
@@ -25,9 +20,13 @@ private import codeql.util.Boolean
2520
* |- ContinueSuccessor
2621
* |- GotoSuccessor
2722
* |- RedoSuccessor // rare, used in Ruby
28-
* |- RetrySuccessor // rare, used in Ruby
29-
* \- JavaYieldSuccessor
23+
* \- RetrySuccessor // rare, used in Ruby
24+
* ```
3025
*/
26+
overlay[local]
27+
module;
28+
29+
private import codeql.util.Boolean
3130

3231
private newtype TSuccessorType =
3332
TDirectSuccessor() or
@@ -42,8 +41,7 @@ private newtype TSuccessorType =
4241
TContinueSuccessor() or
4342
TGotoSuccessor() or
4443
TRedoSuccessor() or
45-
TRetrySuccessor() or
46-
TJavaYieldSuccessor()
44+
TRetrySuccessor()
4745

4846
/**
4947
* The type of a control flow successor.
@@ -52,21 +50,25 @@ private newtype TSuccessorType =
5250
* successors, or abrupt, which covers all other types of successors including
5351
* for example exceptions, returns, and other jumps.
5452
*/
55-
class SuccessorType extends TSuccessorType {
53+
private class SuccessorTypeImpl extends TSuccessorType {
5654
/** Gets a textual representation of this successor type. */
5755
abstract string toString();
5856
}
5957

58+
final class SuccessorType = SuccessorTypeImpl;
59+
6060
private class TNormalSuccessor = TDirectSuccessor or TConditionalSuccessor;
6161

6262
/**
6363
* A normal control flow successor. This is either a direct or a conditional
6464
* successor.
6565
*/
66-
abstract class NormalSuccessor extends SuccessorType, TNormalSuccessor { }
66+
abstract private class NormalSuccessorImpl extends SuccessorTypeImpl, TNormalSuccessor { }
67+
68+
final class NormalSuccessor = NormalSuccessorImpl;
6769

6870
/** A direct control flow successor. */
69-
class DirectSuccessor extends NormalSuccessor, TDirectSuccessor {
71+
class DirectSuccessor extends NormalSuccessorImpl, TDirectSuccessor {
7072
override string toString() { result = "successor" }
7173
}
7274

@@ -78,11 +80,13 @@ private class TConditionalSuccessor =
7880
* a nullness successor (`NullnessSuccessor`), a matching successor (`MatchingSuccessor`),
7981
* or an emptiness successor (`EmptinessSuccessor`).
8082
*/
81-
abstract class ConditionalSuccessor extends NormalSuccessor, TConditionalSuccessor {
83+
abstract private class ConditionalSuccessorImpl extends NormalSuccessorImpl, TConditionalSuccessor {
8284
/** Gets the Boolean value of this successor. */
8385
abstract boolean getValue();
8486
}
8587

88+
final class ConditionalSuccessor = ConditionalSuccessorImpl;
89+
8690
/**
8791
* A Boolean control flow successor.
8892
*
@@ -97,7 +101,7 @@ abstract class ConditionalSuccessor extends NormalSuccessor, TConditionalSuccess
97101
*
98102
* has a control flow graph containing Boolean successors:
99103
*
100-
* ```
104+
* ```text
101105
* if
102106
* |
103107
* x < 0
@@ -109,7 +113,7 @@ abstract class ConditionalSuccessor extends NormalSuccessor, TConditionalSuccess
109113
* return 0 return 1
110114
* ```
111115
*/
112-
class BooleanSuccessor extends ConditionalSuccessor, TBooleanSuccessor {
116+
class BooleanSuccessor extends ConditionalSuccessorImpl, TBooleanSuccessor {
113117
override boolean getValue() { this = TBooleanSuccessor(result) }
114118

115119
override string toString() { result = this.getValue().toString() }
@@ -126,7 +130,7 @@ class BooleanSuccessor extends ConditionalSuccessor, TBooleanSuccessor {
126130
*
127131
* has a control flow graph containing nullness successors:
128132
*
129-
* ```
133+
* ```text
130134
* enter M
131135
* |
132136
* s
@@ -141,7 +145,7 @@ class BooleanSuccessor extends ConditionalSuccessor, TBooleanSuccessor {
141145
* exit M
142146
* ```
143147
*/
144-
class NullnessSuccessor extends ConditionalSuccessor, TNullnessSuccessor {
148+
class NullnessSuccessor extends ConditionalSuccessorImpl, TNullnessSuccessor {
145149
/** Holds if this is a `null` successor. */
146150
predicate isNull() { this = TNullnessSuccessor(true) }
147151

@@ -166,7 +170,7 @@ class NullnessSuccessor extends ConditionalSuccessor, TNullnessSuccessor {
166170
*
167171
* has a control flow graph containing matching successors:
168172
*
169-
* ```
173+
* ```text
170174
* switch
171175
* |
172176
* x
@@ -182,7 +186,7 @@ class NullnessSuccessor extends ConditionalSuccessor, TNullnessSuccessor {
182186
* return 1
183187
* ```
184188
*/
185-
class MatchingSuccessor extends ConditionalSuccessor, TMatchingSuccessor {
189+
class MatchingSuccessor extends ConditionalSuccessorImpl, TMatchingSuccessor {
186190
/** Holds if this is a match successor. */
187191
predicate isMatch() { this = TMatchingSuccessor(true) }
188192

@@ -206,7 +210,7 @@ class MatchingSuccessor extends ConditionalSuccessor, TMatchingSuccessor {
206210
*
207211
* has a control flow graph containing emptiness successors:
208212
*
209-
* ```
213+
* ```text
210214
* args
211215
* |
212216
* loop-header------<-----
@@ -223,7 +227,7 @@ class MatchingSuccessor extends ConditionalSuccessor, TMatchingSuccessor {
223227
* \_________/
224228
* ```
225229
*/
226-
class EmptinessSuccessor extends ConditionalSuccessor, TEmptinessSuccessor {
230+
class EmptinessSuccessor extends ConditionalSuccessorImpl, TEmptinessSuccessor {
227231
/** Holds if this is an empty successor. */
228232
predicate isEmpty() { this = TEmptinessSuccessor(true) }
229233

@@ -236,7 +240,9 @@ private class TAbruptSuccessor =
236240
TExceptionSuccessor or TReturnSuccessor or TExitSuccessor or TJumpSuccessor;
237241

238242
/** An abrupt control flow successor. */
239-
abstract class AbruptSuccessor extends SuccessorType, TAbruptSuccessor { }
243+
abstract private class AbruptSuccessorImpl extends SuccessorTypeImpl, TAbruptSuccessor { }
244+
245+
final class AbruptSuccessor = AbruptSuccessorImpl;
240246

241247
/**
242248
* An exceptional control flow successor.
@@ -255,7 +261,7 @@ abstract class AbruptSuccessor extends SuccessorType, TAbruptSuccessor { }
255261
* The callable exit node of `M` is an exceptional successor of the node
256262
* `throw new ArgumentNullException(nameof(s));`.
257263
*/
258-
class ExceptionSuccessor extends AbruptSuccessor, TExceptionSuccessor {
264+
class ExceptionSuccessor extends AbruptSuccessorImpl, TExceptionSuccessor {
259265
override string toString() { result = "exception" }
260266
}
261267

@@ -274,7 +280,7 @@ class ExceptionSuccessor extends AbruptSuccessor, TExceptionSuccessor {
274280
* The callable exit node of `M` is a `return` successor of the `return;`
275281
* statement.
276282
*/
277-
class ReturnSuccessor extends AbruptSuccessor, TReturnSuccessor {
283+
class ReturnSuccessor extends AbruptSuccessorImpl, TReturnSuccessor {
278284
override string toString() { result = "return" }
279285
}
280286

@@ -294,54 +300,44 @@ class ReturnSuccessor extends AbruptSuccessor, TReturnSuccessor {
294300
*
295301
* The callable exit node of `M` is an exit successor of the node on line 4.
296302
*/
297-
class ExitSuccessor extends AbruptSuccessor, TExitSuccessor {
303+
class ExitSuccessor extends AbruptSuccessorImpl, TExitSuccessor {
298304
override string toString() { result = "exit" }
299305
}
300306

301307
private class TJumpSuccessor =
302-
TBreakSuccessor or TContinueSuccessor or TGotoSuccessor or TRedoSuccessor or TRetrySuccessor or
303-
TJavaYieldSuccessor;
308+
TBreakSuccessor or TContinueSuccessor or TGotoSuccessor or TRedoSuccessor or TRetrySuccessor;
304309

305310
/**
306311
* A jump control flow successor.
307312
*
308313
* This covers non-exceptional, non-local control flow, such as `break`,
309314
* `continue`, and `goto`.
310315
*/
311-
abstract class JumpSuccessor extends AbruptSuccessor, TJumpSuccessor { }
316+
abstract private class JumpSuccessorImpl extends AbruptSuccessorImpl, TJumpSuccessor { }
317+
318+
final class JumpSuccessor = JumpSuccessorImpl;
312319

313320
/** A `break` control flow successor. */
314-
class BreakSuccessor extends JumpSuccessor, TBreakSuccessor {
321+
class BreakSuccessor extends JumpSuccessorImpl, TBreakSuccessor {
315322
override string toString() { result = "break" }
316323
}
317324

318325
/** A `continue` control flow successor. */
319-
class ContinueSuccessor extends JumpSuccessor, TContinueSuccessor {
326+
class ContinueSuccessor extends JumpSuccessorImpl, TContinueSuccessor {
320327
override string toString() { result = "continue" }
321328
}
322329

323330
/** A `goto` control flow successor. */
324-
class GotoSuccessor extends JumpSuccessor, TGotoSuccessor {
331+
class GotoSuccessor extends JumpSuccessorImpl, TGotoSuccessor {
325332
override string toString() { result = "goto" }
326333
}
327334

328335
/** A `redo` control flow successor (rare, used in Ruby). */
329-
class RedoSuccessor extends JumpSuccessor, TRedoSuccessor {
336+
class RedoSuccessor extends JumpSuccessorImpl, TRedoSuccessor {
330337
override string toString() { result = "redo" }
331338
}
332339

333340
/** A `retry` control flow successor (rare, used in Ruby). */
334-
class RetrySuccessor extends JumpSuccessor, TRetrySuccessor {
341+
class RetrySuccessor extends JumpSuccessorImpl, TRetrySuccessor {
335342
override string toString() { result = "retry" }
336343
}
337-
338-
/** A Java `yield` control flow successor. */
339-
class JavaYieldSuccessor extends JumpSuccessor, TJavaYieldSuccessor {
340-
override string toString() { result = "yield" }
341-
}
342-
343-
/** Holds if `t` is an abnormal exit type out of a CFG scope. */
344-
predicate isAbnormalExitType(SuccessorType t) {
345-
t instanceof ExceptionSuccessor or
346-
t instanceof ExitSuccessor
347-
}

0 commit comments

Comments
 (0)