Skip to content

Commit df4fdaf

Browse files
author
Dave Bartolomeo
committed
C++: Fix PR feedback
Note that the various predicates to access the singleton instances of the `EdgeKind` classes have been moved into a module named `EdgeKind`.
1 parent a166a4d commit df4fdaf

File tree

15 files changed

+119
-103
lines changed

15 files changed

+119
-103
lines changed

cpp/ql/src/semmle/code/cpp/ir/implementation/EdgeKind.qll

Lines changed: 42 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,6 @@ class GotoEdge extends EdgeKind, TGotoEdge {
2929
final override string toString() { result = "Goto" }
3030
}
3131

32-
/**
33-
* Gets the single instance of `GotoEdge`, representing the unconditional successor of an
34-
* `Instruction`
35-
*/
36-
GotoEdge gotoEdge() { result = TGotoEdge() }
37-
3832
/**
3933
* A "true" edge, representing the successor of a conditional branch when the
4034
* condition is non-zero.
@@ -43,12 +37,6 @@ class TrueEdge extends EdgeKind, TTrueEdge {
4337
final override string toString() { result = "True" }
4438
}
4539

46-
/**
47-
* Gets the single instance of `TrueEdge`, representing the successor of a conditional branch when
48-
* the condition is non-zero.
49-
*/
50-
TrueEdge trueEdge() { result = TTrueEdge() }
51-
5240
/**
5341
* A "false" edge, representing the successor of a conditional branch when the
5442
* condition is zero.
@@ -57,12 +45,6 @@ class FalseEdge extends EdgeKind, TFalseEdge {
5745
final override string toString() { result = "False" }
5846
}
5947

60-
/**
61-
* Gets the single instance of `FalseEdge`, representing the successor of a conditional branch when
62-
* the condition is zero.
63-
*/
64-
FalseEdge falseEdge() { result = TFalseEdge() }
65-
6648
/**
6749
* An "exception" edge, representing the successor of an instruction when that
6850
* instruction's evaluation throws an exception.
@@ -71,12 +53,6 @@ class ExceptionEdge extends EdgeKind, TExceptionEdge {
7153
final override string toString() { result = "Exception" }
7254
}
7355

74-
/**
75-
* Gets the single instance of `ExceptionEdge`, representing the successor of an instruction when
76-
* that instruction's evaluation throws an exception.
77-
*/
78-
ExceptionEdge exceptionEdge() { result = TExceptionEdge() }
79-
8056
/**
8157
* A "default" edge, representing the successor of a `Switch` instruction when
8258
* none of the case values matches the condition value.
@@ -85,12 +61,6 @@ class DefaultEdge extends EdgeKind, TDefaultEdge {
8561
final override string toString() { result = "Default" }
8662
}
8763

88-
/**
89-
* Gets the single instance of `DefaultEdge`, representing the successor of a `Switch` instruction
90-
* when none of the case values matches the condition value.
91-
*/
92-
DefaultEdge defaultEdge() { result = TDefaultEdge() }
93-
9464
/**
9565
* A "case" edge, representing the successor of a `Switch` instruction when the
9666
* the condition value matches a correponding `case` label.
@@ -112,8 +82,45 @@ class CaseEdge extends EdgeKind, TCaseEdge {
11282
string getMaxValue() { result = maxValue }
11383
}
11484

115-
/**
116-
* Gets the `CaseEdge` representing the successor of a `Switch` instruction corresponding to the
117-
* `case` label with the specified lower and upper bounds.
118-
*/
119-
CaseEdge caseEdge(string minValue, string maxValue) { result = TCaseEdge(minValue, maxValue) }
85+
module EdgeKind {
86+
/**
87+
* Gets the single instance of the `GotoEdge` class.
88+
*/
89+
GotoEdge gotoEdge() { result = TGotoEdge() }
90+
91+
/**
92+
* Gets the single instance of the `TrueEdge` class.
93+
*/
94+
TrueEdge trueEdge() { result = TTrueEdge() }
95+
96+
/**
97+
* Gets the single instance of the `FalseEdge` class.
98+
*/
99+
FalseEdge falseEdge() { result = TFalseEdge() }
100+
101+
/**
102+
* Gets the single instance of the `ExceptionEdge` class.
103+
*/
104+
ExceptionEdge exceptionEdge() { result = TExceptionEdge() }
105+
106+
/**
107+
* Gets the single instance of the `DefaultEdge` class.
108+
*/
109+
DefaultEdge defaultEdge() { result = TDefaultEdge() }
110+
111+
/**
112+
* Gets the `CaseEdge` representing a `case` label with the specified lower and upper bounds.
113+
* For example:
114+
* ```
115+
* switch (x) {
116+
* case 1: // Edge kind is `caseEdge("1", "1")`
117+
* return x;
118+
* case 2...8: // Edge kind is `caseEdge("2", "8")`
119+
* return x - 1;
120+
* default: // Edge kind is `defaultEdge()`
121+
* return 0;
122+
* }
123+
* ```
124+
*/
125+
CaseEdge caseEdge(string minValue, string maxValue) { result = TCaseEdge(minValue, maxValue) }
126+
}

cpp/ql/src/semmle/code/cpp/ir/implementation/TempVariableTag.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ private import internal.TempVariableTagInternal
66
private import Imports::TempVariableTag
77

88
/**
9-
* Describes the reason that a particular IR temporary variable was generated. For example, it could
10-
* be generated to hold the return value of a function, or to hold the result of a `?:` operator
9+
* A reason that a particular IR temporary variable was generated. For example, it could be
10+
* generated to hold the return value of a function, or to hold the result of a `?:` operator
1111
* computed on each branch. The set of possible `TempVariableTag`s is language-dependent.
1212
*/
1313
class TempVariableTag extends TTempVariableTag {

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -584,9 +584,9 @@ class ConditionalBranchInstruction extends Instruction {
584584

585585
final Instruction getCondition() { result = getConditionOperand().getDef() }
586586

587-
final Instruction getTrueSuccessor() { result = getSuccessor(trueEdge()) }
587+
final Instruction getTrueSuccessor() { result = getSuccessor(EdgeKind::trueEdge()) }
588588

589-
final Instruction getFalseSuccessor() { result = getSuccessor(falseEdge()) }
589+
final Instruction getFalseSuccessor() { result = getSuccessor(EdgeKind::falseEdge()) }
590590
}
591591

592592
class ExitFunctionInstruction extends Instruction {
@@ -906,7 +906,7 @@ class SwitchInstruction extends Instruction {
906906

907907
final Instruction getACaseSuccessor() { exists(CaseEdge edge | result = getSuccessor(edge)) }
908908

909-
final Instruction getDefaultSuccessor() { result = getSuccessor(defaultEdge()) }
909+
final Instruction getDefaultSuccessor() { result = getSuccessor(EdgeKind::defaultEdge()) }
910910
}
911911

912912
/**

cpp/ql/src/semmle/code/cpp/ir/implementation/internal/OperandTag.qll

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* Defines the set of possible `OperandTag`s, which are used to identify the role each `Operand`
33
* plays in the evaluation of its `Instruction`.
44
*/
5+
56
private import OperandTagInternal
67

78
private newtype TOperandTag =
@@ -32,13 +33,13 @@ abstract class OperandTag extends TOperandTag {
3233
abstract string toString();
3334

3435
/**
35-
* Gets an integer representing the order in which this operand should appear in the operand list
36-
* of an instruction when printing the IR.
36+
* Gets an integer that represents where this this operand will appear in the operand list of an
37+
* instruction when the IR is printed.
3738
*/
3839
abstract int getSortOrder();
3940

4041
/**
41-
* Gets a label that will appear before the operand when printing the IR.
42+
* Gets a label that will appear before the operand when the IR is printed.
4243
*/
4344
string getLabel() { result = "" }
4445
}
@@ -59,7 +60,7 @@ abstract class RegisterOperandTag extends OperandTag { }
5960
abstract class TypedOperandTag extends MemoryOperandTag { }
6061

6162
// Note: individual subtypes are listed in the order that the operands should
62-
// appear in the operand list of the instruction when printing.
63+
// appear in the operand list of the instruction when the IR is printed.
6364
/**
6465
* The address operand of an instruction that loads or stores a value from
6566
* memory (e.g. `Load`, `Store`, `InitializeParameter`, `IndirectReadSideEffect`).

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -584,9 +584,9 @@ class ConditionalBranchInstruction extends Instruction {
584584

585585
final Instruction getCondition() { result = getConditionOperand().getDef() }
586586

587-
final Instruction getTrueSuccessor() { result = getSuccessor(trueEdge()) }
587+
final Instruction getTrueSuccessor() { result = getSuccessor(EdgeKind::trueEdge()) }
588588

589-
final Instruction getFalseSuccessor() { result = getSuccessor(falseEdge()) }
589+
final Instruction getFalseSuccessor() { result = getSuccessor(EdgeKind::falseEdge()) }
590590
}
591591

592592
class ExitFunctionInstruction extends Instruction {
@@ -906,7 +906,7 @@ class SwitchInstruction extends Instruction {
906906

907907
final Instruction getACaseSuccessor() { exists(CaseEdge edge | result = getSuccessor(edge)) }
908908

909-
final Instruction getDefaultSuccessor() { result = getSuccessor(defaultEdge()) }
909+
final Instruction getDefaultSuccessor() { result = getSuccessor(EdgeKind::defaultEdge()) }
910910
}
911911

912912
/**

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ class TranslatedAllocationSideEffects extends TranslatedSideEffects,
375375

376376
override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
377377
tag = OnlyInstructionTag() and
378-
kind = gotoEdge() and
378+
kind = EdgeKind::gotoEdge() and
379379
if exists(getChild(0))
380380
then result = getChild(0).getFirstInstruction()
381381
else result = getParent().getChildSuccessor(this)

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedFunction.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -720,7 +720,7 @@ class TranslatedReadEffect extends TranslatedElement, TTranslatedReadEffect {
720720

721721
override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind edge) {
722722
tag = OnlyInstructionTag() and
723-
edge = gotoEdge() and
723+
edge = EdgeKind::gotoEdge() and
724724
result = getParent().getChildSuccessor(this)
725725
}
726726

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -584,9 +584,9 @@ class ConditionalBranchInstruction extends Instruction {
584584

585585
final Instruction getCondition() { result = getConditionOperand().getDef() }
586586

587-
final Instruction getTrueSuccessor() { result = getSuccessor(trueEdge()) }
587+
final Instruction getTrueSuccessor() { result = getSuccessor(EdgeKind::trueEdge()) }
588588

589-
final Instruction getFalseSuccessor() { result = getSuccessor(falseEdge()) }
589+
final Instruction getFalseSuccessor() { result = getSuccessor(EdgeKind::falseEdge()) }
590590
}
591591

592592
class ExitFunctionInstruction extends Instruction {
@@ -906,7 +906,7 @@ class SwitchInstruction extends Instruction {
906906

907907
final Instruction getACaseSuccessor() { exists(CaseEdge edge | result = getSuccessor(edge)) }
908908

909-
final Instruction getDefaultSuccessor() { result = getSuccessor(defaultEdge()) }
909+
final Instruction getDefaultSuccessor() { result = getSuccessor(EdgeKind::defaultEdge()) }
910910
}
911911

912912
/**

cpp/ql/src/semmle/code/cpp/ir/internal/IntegerConstant.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
2-
* Provides predicates for manipulating integer constants tracked by constant folding and similar
3-
* analyses.
2+
* Provides predicates for manipulating integer constants that are tracked by constant folding and
3+
* similar analyses.
44
*/
55

66
/**

csharp/ql/src/semmle/code/csharp/ir/implementation/EdgeKind.qll

Lines changed: 42 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,6 @@ class GotoEdge extends EdgeKind, TGotoEdge {
2929
final override string toString() { result = "Goto" }
3030
}
3131

32-
/**
33-
* Gets the single instance of `GotoEdge`, representing the unconditional successor of an
34-
* `Instruction`
35-
*/
36-
GotoEdge gotoEdge() { result = TGotoEdge() }
37-
3832
/**
3933
* A "true" edge, representing the successor of a conditional branch when the
4034
* condition is non-zero.
@@ -43,12 +37,6 @@ class TrueEdge extends EdgeKind, TTrueEdge {
4337
final override string toString() { result = "True" }
4438
}
4539

46-
/**
47-
* Gets the single instance of `TrueEdge`, representing the successor of a conditional branch when
48-
* the condition is non-zero.
49-
*/
50-
TrueEdge trueEdge() { result = TTrueEdge() }
51-
5240
/**
5341
* A "false" edge, representing the successor of a conditional branch when the
5442
* condition is zero.
@@ -57,12 +45,6 @@ class FalseEdge extends EdgeKind, TFalseEdge {
5745
final override string toString() { result = "False" }
5846
}
5947

60-
/**
61-
* Gets the single instance of `FalseEdge`, representing the successor of a conditional branch when
62-
* the condition is zero.
63-
*/
64-
FalseEdge falseEdge() { result = TFalseEdge() }
65-
6648
/**
6749
* An "exception" edge, representing the successor of an instruction when that
6850
* instruction's evaluation throws an exception.
@@ -71,12 +53,6 @@ class ExceptionEdge extends EdgeKind, TExceptionEdge {
7153
final override string toString() { result = "Exception" }
7254
}
7355

74-
/**
75-
* Gets the single instance of `ExceptionEdge`, representing the successor of an instruction when
76-
* that instruction's evaluation throws an exception.
77-
*/
78-
ExceptionEdge exceptionEdge() { result = TExceptionEdge() }
79-
8056
/**
8157
* A "default" edge, representing the successor of a `Switch` instruction when
8258
* none of the case values matches the condition value.
@@ -85,12 +61,6 @@ class DefaultEdge extends EdgeKind, TDefaultEdge {
8561
final override string toString() { result = "Default" }
8662
}
8763

88-
/**
89-
* Gets the single instance of `DefaultEdge`, representing the successor of a `Switch` instruction
90-
* when none of the case values matches the condition value.
91-
*/
92-
DefaultEdge defaultEdge() { result = TDefaultEdge() }
93-
9464
/**
9565
* A "case" edge, representing the successor of a `Switch` instruction when the
9666
* the condition value matches a correponding `case` label.
@@ -112,8 +82,45 @@ class CaseEdge extends EdgeKind, TCaseEdge {
11282
string getMaxValue() { result = maxValue }
11383
}
11484

115-
/**
116-
* Gets the `CaseEdge` representing the successor of a `Switch` instruction corresponding to the
117-
* `case` label with the specified lower and upper bounds.
118-
*/
119-
CaseEdge caseEdge(string minValue, string maxValue) { result = TCaseEdge(minValue, maxValue) }
85+
module EdgeKind {
86+
/**
87+
* Gets the single instance of the `GotoEdge` class.
88+
*/
89+
GotoEdge gotoEdge() { result = TGotoEdge() }
90+
91+
/**
92+
* Gets the single instance of the `TrueEdge` class.
93+
*/
94+
TrueEdge trueEdge() { result = TTrueEdge() }
95+
96+
/**
97+
* Gets the single instance of the `FalseEdge` class.
98+
*/
99+
FalseEdge falseEdge() { result = TFalseEdge() }
100+
101+
/**
102+
* Gets the single instance of the `ExceptionEdge` class.
103+
*/
104+
ExceptionEdge exceptionEdge() { result = TExceptionEdge() }
105+
106+
/**
107+
* Gets the single instance of the `DefaultEdge` class.
108+
*/
109+
DefaultEdge defaultEdge() { result = TDefaultEdge() }
110+
111+
/**
112+
* Gets the `CaseEdge` representing a `case` label with the specified lower and upper bounds.
113+
* For example:
114+
* ```
115+
* switch (x) {
116+
* case 1: // Edge kind is `caseEdge("1", "1")`
117+
* return x;
118+
* case 2...8: // Edge kind is `caseEdge("2", "8")`
119+
* return x - 1;
120+
* default: // Edge kind is `defaultEdge()`
121+
* return 0;
122+
* }
123+
* ```
124+
*/
125+
CaseEdge caseEdge(string minValue, string maxValue) { result = TCaseEdge(minValue, maxValue) }
126+
}

0 commit comments

Comments
 (0)