Skip to content

Apply the capture dependency rules only to captured wildcards - #8034

Open
smillst wants to merge 2 commits into
typetools:masterfrom
smillst:issue7694-capture-2
Open

Apply the capture dependency rules only to captured wildcards#8034
smillst wants to merge 2 commits into
typetools:masterfrom
smillst:issue7694-capture-2

Conversation

@smillst

@smillst smillst commented Aug 21, 2026

Copy link
Copy Markdown
Member

JLS 18.5.2.1 says that when the return type of a poly invocation is a parameterized type G<A1, ..., An> and one of A1, ..., An is a wildcard, fresh inference variables B1, ..., Bn are created along with the bound G<B1, ..., Bn> = capture(G<A1, ..., An>). A variable is created for every type argument, not only for the wildcards.

javac creates fewer. Infer#generateReturnConstraints captures the return type and then adds an inference variable only for a type argument that capture conversion replaced, which is only a wildcard. For the return type Collector<E, ?, List<E>> of toImmutableList() in the new test case, javac adds one variable and the Checker Framework adds three.

The extra variables change inference, because the two dependency rules that JLS 18.4 states for a variable on the left-hand side of a capture bound reverse the usual direction of a dependency. For a non-wildcard type argument Ai the bound alphai = Ai holds, so applying those rules makes every variable mentioned in Ai's bounds depend on alphai. In the new test case that puts the variable for Optional.empty() in the same resolution set as the variable it is a lower bound of. Resolution then instantiates them together, discards the lower bound Optional<T> because it is not proper, and derives the false bound Optional<Object> <: Optional<Foo>. Inference reported "type.argument.inference.crashed"; it now infers Optional<? extends Object>, as javac does.

Apply the two rules only to a variable that captures a wildcard.

Fixes #7694.

JLS 18.5.2.1 says that when the return type of a poly invocation is a
parameterized type G<A1, ..., An> and one of A1, ..., An is a wildcard, fresh
inference variables B1, ..., Bn are created along with the bound
G<B1, ..., Bn> = capture(G<A1, ..., An>).  A variable is created for every
type argument, not only for the wildcards.

javac creates fewer.  Infer#generateReturnConstraints captures the return type
and then adds an inference variable only for a type argument that capture
conversion replaced, which is only a wildcard.  For the return type
`Collector<E, ?, List<E>>` of `toImmutableList()` in the new test case, javac
adds one variable and the Checker Framework adds three.

The extra variables change inference, because the two dependency rules that
JLS 18.4 states for a variable on the left-hand side of a capture bound
reverse the usual direction of a dependency.  For a non-wildcard type argument
Ai the bound alphai = Ai holds, so applying those rules makes every variable
mentioned in Ai's bounds depend on alphai.  In the new test case that puts the
variable for `Optional.empty()` in the same resolution set as the variable it
is a lower bound of.  Resolution then instantiates them together, discards the
lower bound `Optional<T>` because it is not proper, and derives the false
bound `Optional<Object> <: Optional<Foo>`.  Inference reported
"type.argument.inference.crashed"; it now infers `Optional<? extends Object>`,
as javac does.

Apply the two rules only to a variable that captures a wildcard.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 43641a94-e830-407b-b01c-b6b9776075c6

📥 Commits

Reviewing files that changed from the base of the PR and between 3a61c10 and 9205631.

📒 Files selected for processing (1)
  • framework/src/main/java/org/checkerframework/framework/util/typeinference8/types/CaptureVariable.java

Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The change records whether capture variables represent wildcard type arguments. CaptureBound sets this state, and Variable exposes it. BoundSet.getDependencies now applies two dependency rules only to captured wildcard variables. A new Issue7694 test fixture covers dependency behavior for non-wildcard type arguments.

Suggested reviewers: mernst

Merge Risk: ⚪ Minimal · up to 92056

The change is merge-ready after normal checks and review; no actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mernst mernst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@mernst
mernst enabled auto-merge (squash) August 21, 2026 15:57
@mernst mernst removed their assignment Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Type argument inference crashed

2 participants