Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.Element;
import javax.lang.model.element.VariableElement;
import org.checkerframework.checker.mustcall.qual.MustCall;
import org.checkerframework.dataflow.cfg.node.MethodInvocationNode;
import org.checkerframework.dataflow.cfg.visualize.CFGVisualizer;
import org.checkerframework.dataflow.expression.ClassName;
Expand All @@ -28,10 +29,14 @@
* A store that extends {@code CFAbstractStore} and additionally tracks which fields of the 'self'
* reference have been initialized.
*
* @param <V> the type of values in the abstract store
* @param <S> the type of teh abstract store
Comment thread
kelloggm marked this conversation as resolved.
Outdated
* @see InitializationTransfer
*/
public class InitializationStore<V extends CFAbstractValue<V>, S extends InitializationStore<V, S>>
extends CFAbstractStore<V, S> {
public class InitializationStore<
V extends CFAbstractValue<@MustCall({}) V>,
S extends InitializationStore<@MustCall({}) V, @MustCall({}) S>>
extends CFAbstractStore<@MustCall({}) V, @MustCall({}) S> {

/** The set of fields that are initialized. */
protected final Set<VariableElement> initializedFields;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import javax.lang.model.type.TypeKind;
import javax.lang.model.type.TypeMirror;
import javax.lang.model.util.ElementFilter;
import org.checkerframework.checker.mustcall.qual.MustCall;
import org.checkerframework.dataflow.analysis.RegularTransferResult;
import org.checkerframework.dataflow.analysis.TransferInput;
import org.checkerframework.dataflow.analysis.TransferResult;
Expand Down Expand Up @@ -56,7 +57,7 @@
public class InitializationTransfer<
V extends CFAbstractValue<V>,
T extends InitializationTransfer<V, T, S>,
S extends InitializationStore<V, S>>
S extends InitializationStore<@MustCall({}) V, @MustCall({}) S>>
Comment on lines 59 to +62
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Show the recursive value bounds in InitializationStore and InitializationTransfer.
# Expectation after the fix: both value bounds use CFAbstractValue<@MustCall({}) V>.

rg -n -C 3 'class Initialization(Store|Transfer)|V extends CFAbstractValue' \
  --iglob 'InitializationStore.java' \
  --iglob 'InitializationTransfer.java'

Repository: typetools/checker-framework

Length of output: 2234


Align V's bound with InitializationStore.

Line 62 passes @MustCall({}) V to InitializationStore, but line 60 binds V as CFAbstractValue<V> instead of CFAbstractValue<@MustCall({}) V>. Mirror InitializationStore's value bound to resolve the annotated type-argument mismatch.

Proposed fix
 public class InitializationTransfer<
-        V extends CFAbstractValue<V>,
+        V extends CFAbstractValue<@MustCall({}) V>,
         T extends InitializationTransfer<V, T, S>,
         S extends InitializationStore<@MustCall({}) V, `@MustCall`({}) S>>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public class InitializationTransfer<
V extends CFAbstractValue<V>,
T extends InitializationTransfer<V, T, S>,
S extends InitializationStore<V, S>>
S extends InitializationStore<@MustCall({}) V, @MustCall({}) S>>
public class InitializationTransfer<
V extends CFAbstractValue<@MustCall({}) V>,
T extends InitializationTransfer<V, T, S>,
S extends InitializationStore<@MustCall({}) V, `@MustCall`({}) S>>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@checker/src/main/java/org/checkerframework/checker/initialization/InitializationTransfer.java`
around lines 59 - 62, The generic type V in InitializationTransfer is bound as
CFAbstractValue<V> but InitializationStore is parameterized with `@MustCall`({})
V, causing an annotated type-argument mismatch; change V's bound to
CFAbstractValue<@MustCall({}) V> so the value type annotation matches
InitializationStore's value parameter (update the type parameter declaration on
class InitializationTransfer accordingly, keeping T and S as-is and preserving
references to InitializationStore and `@MustCall`).

extends CFAbstractTransfer<V, S, T> {

protected final InitializationAnnotatedTypeFactory<?, ?, ?, ?> atypeFactory;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ public boolean derivedFromMustCallAlias() {
}

/**
* Gets the must-call type associated with the given resource alias, falling on back on the
* Gets the must-call type associated with the given resource alias, falling back on the
* declared type if there is no refined type for the alias in the store.
*
* @param alias a resource alias
Expand All @@ -366,7 +366,9 @@ private static AnnotationMirror getMustCallValue(
CFValue value = mcStore == null ? null : mcStore.getValue(reference);
if (value != null) {
AnnotationMirror result =
AnnotationUtils.getAnnotationByClass(value.getAnnotations(), MustCall.class);
mcAtf
.getQualifierHierarchy()
.findAnnotationInHierarchy(value.getAnnotations(), mcAtf.TOP);
if (result != null) {
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* Demonstrates an issue in the Checker Framework with handling the nearest enclosing element for
* temporary variable declarations, leading to a crash during analysis.
*/
@SuppressWarnings("all") // only check for crashes
public abstract class CrashForTempVar<T extends Number> {

private final CrashForTempVar<T> _base;
Expand Down
15 changes: 15 additions & 0 deletions checker/tests/resourceleak/DropOwning.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// test case for https://github.com/typetools/checker-framework/issues/6990

import java.io.Closeable;
import org.checkerframework.checker.mustcall.qual.MustCallUnknown;
import org.checkerframework.checker.mustcall.qual.Owning;

public class DropOwning {

public void f(@Owning Closeable resource) {
drop(resource);
}

// :: error: required.method.not.known
private void drop(@Owning @MustCallUnknown Object resourceCF6990) {}
}
Loading