Skip to content

Commit 4ebfdd4

Browse files
committed
Fix comments on diff/overlay-informedness.
1 parent aa500df commit 4ebfdd4

File tree

1 file changed

+11
-32
lines changed

1 file changed

+11
-32
lines changed

shared/dataflow/codeql/dataflow/internal/DataFlowImplStage1.qll

Lines changed: 11 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -144,49 +144,31 @@ module MakeImplStage1<LocationSig Location, InputSig<Location> Lang> {
144144
exists(node)
145145
}
146146

147-
/**
148-
* Holds if `node` comes from a file that was in the diff for which we
149-
* are producing results.
150-
*/
151-
overlay[global]
152-
private predicate isDiffFileNode(Node node) {
153-
exists(string filePath |
154-
node.getLocation().hasLocationInfo(filePath, _, _, _, _) and
155-
AlertFiltering::fileIsInDiff(filePath)
156-
)
157-
}
158-
159147
overlay[global]
160148
pragma[nomagic]
161149
private predicate isFilteredSource(Node source) {
162150
Config::isSource(source, _) and
163151
// Data flow is always incremental in one of two ways.
164-
// 1. If the configuration is diff-informed and diff information is
165-
// available, we filter to only include nodes in the diff, which
166-
// gives the smallest set of nodes.
152+
// 1. If the configuration is diff-informed, we filter to only include nodes in the diff,
153+
// which gives the smallest set of nodes.
154+
// If diff information is not available, we do not filter at all.
167155
// 2. If not, in global evaluation with overlay, we filter to only
168-
// include nodes from files in the overlay or the diff; flow from
169-
// other nodes will be added back later. There can be two reasons
170-
// why we are in this case:
171-
// 1. This could be the primary configuration for a query that
172-
// hasn't yet become diff-informed. In that case, the
173-
// `getASelectedSourceLocation` information is probably just the
174-
// default, and it's a fairly safe overapproximation to
175-
// effectively expand to all nodes in the file (via
176-
// `isDiffFileNode`).
177-
// 2. This could be a secondary configuration, like a helper
178-
// configuration for finding sources or sinks of a primary
179-
// configuration. In that case, the results of this configuration
180-
// are typically in the same file as the final alert.
156+
// include nodes from files in the overlay; flow from
157+
// other nodes will be added back later.
158+
159+
// We start by seing if we should be in case 1.
181160
if
182161
Config::observeDiffInformedIncrementalMode()
162+
// Case 1: We are meant to be diff-informed.
163+
// We still only filter if we have diff information.
183164
then AlertFiltering::diffInformationAvailable() implies AlertFiltering::locationIsInDiff(Config::getASelectedSourceLocation(source))
184165
else (
166+
// Case 2: We are not meant to be diff-informed, so we fall back to overlay-based filtering.
185167
// If we are in base-only global evaluation, do not filter out any sources.
186168
not isEvaluatingInOverlay()
187169
or
188170
// If we are in global evaluation with an overlay present, restrict
189-
// sources to those visible in the overlay or
171+
// sources to those visible in the overlay.
190172
isOverlayNode(source)
191173
)
192174
}
@@ -203,11 +185,8 @@ module MakeImplStage1<LocationSig Location, InputSig<Location> Lang> {
203185
Config::observeDiffInformedIncrementalMode()
204186
then AlertFiltering::diffInformationAvailable() implies AlertFiltering::locationIsInDiff(Config::getASelectedSinkLocation(sink))
205187
else (
206-
// If we are in base-only global evaluation, do not filter out any sources.
207188
not isEvaluatingInOverlay()
208189
or
209-
// If we are in global evaluation with an overlay present, restrict
210-
// sources to those visible in the overlay or
211190
isOverlayNode(sink)
212191
)
213192

0 commit comments

Comments
 (0)