Skip to content

Review of Nullness Checker#1706

Merged
wmdietl merged 3 commits into
masterfrom
nullness-perf
May 12, 2026
Merged

Review of Nullness Checker#1706
wmdietl merged 3 commits into
masterfrom
nullness-perf

Conversation

@wmdietl
Copy link
Copy Markdown
Member

@wmdietl wmdietl commented May 11, 2026

No description provided.

Copilot AI review requested due to automatic review settings May 11, 2026 18:51
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refines implementation details of the Nullness Checker (NoInit variant), primarily by reducing repeated work in type/annotation scanning and simplifying some receiver-handling logic during method invocability checks.

Changes:

  • Simplifies receiver processing in NullnessNoInitVisitor.checkMethodInvocability.
  • Reuses a stateful AnnotatedTypeScanner in NullnessNoInitTransfer to reduce allocations.
  • Optimizes nullness-annotation detection in NullnessNoInitAnnotatedTypeFactory by canonicalizing aliases once.
  • Minor refactor to reduce repeated getTypeArguments() calls in CollectionToArrayHeuristics.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
checker/src/main/java/org/checkerframework/checker/nullness/NullnessNoInitVisitor.java Simplifies receiver annotation extraction during method invocability checks.
checker/src/main/java/org/checkerframework/checker/nullness/NullnessNoInitTransfer.java Introduces reusable scanner instance and resets it per visit to avoid repeated allocations.
checker/src/main/java/org/checkerframework/checker/nullness/NullnessNoInitAnnotatedTypeFactory.java Canonicalizes annotation aliases once before checking canonical nullness qualifier names.
checker/src/main/java/org/checkerframework/checker/nullness/CollectionToArrayHeuristics.java Small readability refactor by caching type arguments list locally.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 339 to +343
private boolean containsPolyNullNotAtTopLevel(AnnotatedTypeMirror t) {
return new ContainsPolyNullNotAtTopLevelScanner().visit(t);
// The scanner is stateful (isTopLevel and visitedNodes); reset before reuse.
polyNullScanner.isTopLevel = true;
polyNullScanner.reset();
return polyNullScanner.visit(t);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Implemented in 5559ab0. I moved isTopLevel reset into ContainsPolyNullNotAtTopLevelScanner.reset() (calling super.reset()), and removed the explicit polyNullScanner.reset()/state reset in containsPolyNullNotAtTopLevel, so visit(...) performs a single reset per call.

Copy link
Copy Markdown

Copilot AI commented May 11, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • central.sonatype.com
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -Xmx2g -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en -Duser.variant -cp /home/REDACTED/.gradle/wrapper/dists/gradle-8.14.5-bin/690y85m0j9nfaub7xoiayko8a/gradle-8.14.5/lib/gradle-daemon-main-8.14.5.jar (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@wmdietl wmdietl enabled auto-merge (squash) May 11, 2026 23:51
@wmdietl wmdietl merged commit 0cc7c38 into master May 12, 2026
50 checks passed
@wmdietl wmdietl deleted the nullness-perf branch May 12, 2026 00:46
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.

3 participants