Skip to content

Commit 9907586

Browse files
msridharmsridhar
authored
Use refined types from dataflow analysis in generic method inference (#1309)
Fixes #1287 In generic method inference, we now try to obtain a refined type from dataflow-based type inference for arguments when generating constraints. This introduces a recursive dependence between dataflow analysis and generic method inference, which we handle as follows. We track whether generic method inference has been invoked from dataflow by passing a `calledFromDataflow` parameter. When the parameter is true, generic method inference calls a new API to get the _current_ dataflow fact for the argument from the running dataflow analysis, avoiding a recursive run. Since this may not be the fact in the final fixed point of dataflow, we skip caching of generic method inference results when it is invoked from the dataflow analysis. This may skip caching unnecessarily in some cases, but we prefer to make correctness more obvious for now; we can optimize further in the future if needed. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Dataflow-aware argument/type refinement for improved nullness precision. * Ability to query in-flight ("running") dataflow nullness and control whether analysis runs. * Utility to strip nullable annotations from types. * **Bug Fixes** * Guarded caching and path-aware propagation to avoid reusing dataflow-dependent inference results. * **Tests** * Re-enabled a test, added dataflow/loop/local-variable tests, and updated diagnostics. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent c6d2ba3 commit 9907586

7 files changed

Lines changed: 377 additions & 48 deletions

File tree

‎nullaway/src/main/java/com/uber/nullaway/NullAway.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2711,7 +2711,7 @@ private boolean mayBeNullMethodCall(
27112711
// it will do so without an assignment context. If this becomes a problem, we can revisit
27122712
if (config.isJSpecifyMode()
27132713
&& genericsChecks
2714-
.getGenericReturnNullnessAtInvocation(exprSymbol, invocationTree, null, state)
2714+
.getGenericReturnNullnessAtInvocation(exprSymbol, invocationTree, null, state, false)
27152715
.equals(Nullness.NULLABLE)) {
27162716
return true;
27172717
}

‎nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessAnalysis.java‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,19 @@ public static AccessPathNullnessAnalysis instance(VisitorState state, NullAway a
112112
* @return nullness info for expression, from dataflow
113113
*/
114114
public @Nullable Nullness getNullness(TreePath exprPath, Context context) {
115-
return dataFlow.expressionDataflow(exprPath, context, nullnessPropagation);
115+
return dataFlow.expressionDataflow(exprPath, context, nullnessPropagation, false);
116+
}
117+
118+
/**
119+
* Get the nullness info for an expression from the current running dataflow analysis (so it may
120+
* not be the final result).
121+
*
122+
* @param exprPath tree path of expression
123+
* @param context Javac context
124+
* @return nullness info for expression, from running dataflow
125+
*/
126+
public @Nullable Nullness getNullnessFromRunning(TreePath exprPath, Context context) {
127+
return dataFlow.expressionDataflow(exprPath, context, nullnessPropagation, true);
116128
}
117129

118130
/**
@@ -125,7 +137,7 @@ public static AccessPathNullnessAnalysis instance(VisitorState state, NullAway a
125137
*/
126138
public @Nullable Nullness getNullnessForContractDataflow(TreePath exprPath, Context context) {
127139
return dataFlow.expressionDataflow(
128-
exprPath, context, castToNonNull(contractNullnessPropagation));
140+
exprPath, context, castToNonNull(contractNullnessPropagation), false);
129141
}
130142

131143
/**

‎nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1154,7 +1154,7 @@ private boolean genericReturnIsNullable(MethodInvocationNode node) {
11541154
if (tree != null) {
11551155
Nullness nullness =
11561156
genericsChecks.getGenericReturnNullnessAtInvocation(
1157-
ASTHelpers.getSymbol(tree), tree, node.getTreePath(), state);
1157+
ASTHelpers.getSymbol(tree), tree, node.getTreePath(), state, true);
11581158
return nullness.equals(NULLABLE);
11591159
}
11601160
}

‎nullaway/src/main/java/com/uber/nullaway/dataflow/DataFlow.java‎

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,10 @@ public final class DataFlow {
8585
new CacheLoader<AnalysisParams, Analysis<?, ?, ?>>() {
8686
@Override
8787
public Analysis<?, ?, ?> load(AnalysisParams key) {
88-
ControlFlowGraph cfg = key.cfg();
8988
ForwardTransferFunction<?, ?> transfer = key.transferFunction();
9089

9190
@SuppressWarnings({"unchecked", "rawtypes"})
9291
Analysis<?, ?, ?> analysis = new ForwardAnalysisImpl<>(transfer);
93-
analysis.performAnalysis(cfg);
9492
return analysis;
9593
}
9694
});
@@ -147,14 +145,25 @@ public ControlFlowGraph load(CfgParams key) {
147145
* their control flow graph is the same. - if two transfer functions are {@code equal}, and are
148146
* run over the same control flow graph, the analysis result is the same. - for all contexts, the
149147
* analysis result is the same.
148+
*
149+
* @param path path to method, lambda or initializer
150+
* @param context Javac context
151+
* @param transfer transfer functions
152+
* @param performAnalysis whether dataflow analysis should be initiated via a call to {@code
153+
* performAnalysis}. If the analysis is already running, this parameter should be {@code
154+
* false}; dataflow cannot be run recursively.
150155
*/
151156
private <A extends AbstractValue<A>, S extends Store<S>, T extends ForwardTransferFunction<A, S>>
152-
Result<A, S, T> dataflow(TreePath path, Context context, T transfer) {
157+
Result<A, S, T> dataflow(
158+
TreePath path, Context context, T transfer, boolean performAnalysis) {
153159
ProcessingEnvironment env = JavacProcessingEnvironment.instance(context);
154160
ControlFlowGraph cfg = cfgCache.getUnchecked(CfgParams.create(path, env));
155161
AnalysisParams aparams = AnalysisParams.create(transfer, cfg);
156162
@SuppressWarnings("unchecked")
157163
Analysis<A, S, T> analysis = (Analysis<A, S, T>) analysisCache.getUnchecked(aparams);
164+
if (performAnalysis) {
165+
analysis.performAnalysis(cfg);
166+
}
158167

159168
return new Result<A, S, T>() {
160169
@Override
@@ -187,7 +196,8 @@ ControlFlowGraph getControlFlowGraph(TreePath path, Context context, T transfer)
187196
throw new IllegalArgumentException(
188197
"Cannot get CFG for node outside a method, lambda, or initializer");
189198
}
190-
return dataflow(enclosingMethodOrLambdaOrInitializer, context, transfer).getControlFlowGraph();
199+
return dataflow(enclosingMethodOrLambdaOrInitializer, context, transfer, true)
200+
.getControlFlowGraph();
191201
}
192202

193203
/**
@@ -197,15 +207,31 @@ ControlFlowGraph getControlFlowGraph(TreePath path, Context context, T transfer)
197207
* @param exprPath expression
198208
* @param context Javac context
199209
* @param transfer transfer functions
210+
* @param isRunning true if the dataflow analysis is currently running
200211
* @param <A> values in abstraction
201212
* @param <S> store type
202213
* @param <T> transfer function type
203214
* @return dataflow value for expression
204215
*/
205216
public <A extends AbstractValue<A>, S extends Store<S>, T extends ForwardTransferFunction<A, S>>
206-
@Nullable A expressionDataflow(TreePath exprPath, Context context, T transfer) {
207-
AnalysisResult<A, S> analysisResult = resultForExpr(exprPath, context, transfer);
208-
return analysisResult == null ? null : analysisResult.getValue(exprPath.getLeaf());
217+
@Nullable A expressionDataflow(
218+
TreePath exprPath, Context context, T transfer, boolean isRunning) {
219+
if (isRunning) {
220+
// get the Analysis object from the cache, and get the current result from that
221+
Analysis<A, S, T> analysis =
222+
dataflow(
223+
Preconditions.checkNotNull(
224+
findEnclosingMethodOrLambdaOrInitializer(exprPath),
225+
"expression is not inside a method, lambda or initializer block!"),
226+
context,
227+
transfer,
228+
false)
229+
.getAnalysis();
230+
return analysis.getValue(exprPath.getLeaf());
231+
} else {
232+
AnalysisResult<A, S> analysisResult = resultForExpr(exprPath, context, transfer);
233+
return analysisResult == null ? null : analysisResult.getValue(exprPath.getLeaf());
234+
}
209235
}
210236

211237
/**
@@ -230,7 +256,7 @@ ControlFlowGraph getControlFlowGraph(TreePath path, Context context, T transfer)
230256
"Leaf of methodPath must be of type MethodTree, LambdaExpressionTree, BlockTree, or VariableTree, but was %s",
231257
leaf.getClass().getName());
232258

233-
return dataflow(path, context, transfer).getAnalysis().getRegularExitStore();
259+
return dataflow(path, context, transfer, true).getAnalysis().getRegularExitStore();
234260
}
235261

236262
public <A extends AbstractValue<A>, S extends Store<S>, T extends ForwardTransferFunction<A, S>>
@@ -280,7 +306,7 @@ ControlFlowGraph getControlFlowGraph(TreePath path, Context context, T transfer)
280306
// *before* any unboxing operations (like invoking intValue() on an Integer). This is
281307
// important,
282308
// e.g., for actually checking that the unboxing operation is legal.
283-
return dataflow(enclosingPath, context, transfer).getAnalysis().getResult();
309+
return dataflow(enclosingPath, context, transfer, true).getAnalysis().getResult();
284310
}
285311

286312
/** clear the CFG and analysis caches */

0 commit comments

Comments
 (0)