-
Couldn't load subscription status.
- Fork 664
[javac plugin] Add compile-time checks for unsafe or incorrect coroutine usage #8289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
SamCarlberg
wants to merge
11
commits into
wpilibsuite:2027
Choose a base branch
from
SamCarlberg:compiler-plugin-coroutine-yield-check
base: 2027
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,796
−14
Open
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
ce5ac5d
Add check for calls to Coroutine.yield() in loops
SamCarlberg 34587f6
Expand loop check to apply to any blocking Coroutine call
SamCarlberg 02a0709
Add detector for unreachable code after Coroutine.park()
SamCarlberg 7907a42
Rename CompileTestOptions to CompileTestUtils
SamCarlberg 69ef00d
Update comment formatting
SamCarlberg 488edcb
Check for usage of nonlocal captured coroutines and field assignment
SamCarlberg 374ad3f
Update error message to wrap code in backticks
SamCarlberg 89cb48f
Add license header
SamCarlberg b65df19
Linting
SamCarlberg 4ca4af7
Move shared logic to a base class
SamCarlberg 331b9ea
Add suppressions for coroutine error checks
SamCarlberg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
100 changes: 100 additions & 0 deletions
100
javacPlugin/src/main/java/org/wpilib/javacplugin/CodeAfterCoroutineParkDetector.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| // Copyright (c) FIRST and other WPILib contributors. | ||
| // Open Source Software; you can modify and/or share it under the terms of | ||
| // the WPILib BSD license file in the root directory of this project. | ||
|
|
||
| package org.wpilib.javacplugin; | ||
|
|
||
| import com.sun.source.tree.BlockTree; | ||
| import com.sun.source.tree.CompilationUnitTree; | ||
| import com.sun.source.tree.EmptyStatementTree; | ||
| import com.sun.source.tree.ExpressionStatementTree; | ||
| import com.sun.source.tree.IdentifierTree; | ||
| import com.sun.source.tree.MemberSelectTree; | ||
| import com.sun.source.tree.MethodInvocationTree; | ||
| import com.sun.source.tree.StatementTree; | ||
| import com.sun.source.util.JavacTask; | ||
| import com.sun.source.util.TaskEvent; | ||
| import com.sun.source.util.TaskListener; | ||
| import com.sun.source.util.TreeScanner; | ||
| import com.sun.source.util.Trees; | ||
| import java.util.HashSet; | ||
| import java.util.Set; | ||
| import javax.lang.model.element.VariableElement; | ||
| import javax.lang.model.type.TypeMirror; | ||
| import javax.tools.Diagnostic; | ||
|
|
||
| /** | ||
| * Detects any statements after a call to {@code coroutine.park()} and labels them as unreachable | ||
| * code, similar to a {@code while (true)} statement. | ||
| */ | ||
| public class CodeAfterCoroutineParkDetector implements TaskListener { | ||
| private final JavacTask m_task; | ||
| private final Set<CompilationUnitTree> m_visitedCUs = new HashSet<>(); | ||
|
|
||
| public CodeAfterCoroutineParkDetector(JavacTask task) { | ||
| m_task = task; | ||
| } | ||
|
|
||
| @Override | ||
| public void finished(TaskEvent taskEvent) { | ||
| CompilationUnitTree compilationUnit = taskEvent.getCompilationUnit(); | ||
| if (taskEvent.getKind() == TaskEvent.Kind.ANALYZE && m_visitedCUs.add(compilationUnit)) { | ||
| TypeMirror coroutineType = getCoroutineType(); | ||
| if (coroutineType == null) { | ||
| // Not using the commands library; nothing to scan for | ||
| return; | ||
| } | ||
|
|
||
| compilationUnit.accept(new Scanner(compilationUnit), null); | ||
| } | ||
| } | ||
|
|
||
| private TypeMirror getCoroutineType() { | ||
SamCarlberg marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| var te = m_task.getElements().getTypeElement("org.wpilib.commands3.Coroutine"); | ||
| return te != null ? te.asType() : null; | ||
| } | ||
|
|
||
| private final class Scanner extends TreeScanner<Void, Void> { | ||
| private final CompilationUnitTree m_root; | ||
| private final Trees m_trees = Trees.instance(m_task); | ||
|
|
||
| Scanner(CompilationUnitTree compilationUnit) { | ||
| m_root = compilationUnit; | ||
| } | ||
|
|
||
| @Override | ||
| public Void visitBlock(BlockTree node, Void param) { | ||
| MethodInvocationTree parkInvocation = null; | ||
| for (StatementTree statement : node.getStatements()) { | ||
| if (statement instanceof EmptyStatementTree) { | ||
| // skip empty statements; someone could have just added an extra semicolon by accident | ||
| continue; | ||
| } | ||
|
|
||
| if (parkInvocation != null) { | ||
| m_trees.printMessage( | ||
| Diagnostic.Kind.ERROR, | ||
| "Unreachable statement: `" + parkInvocation + "` will never exit", | ||
| statement, | ||
| m_root); | ||
| break; | ||
| } | ||
|
|
||
| if (statement instanceof ExpressionStatementTree est | ||
| && est.getExpression() instanceof MethodInvocationTree mit | ||
| && mit.getMethodSelect() instanceof MemberSelectTree ms | ||
| && ms.getIdentifier().contentEquals("park") | ||
| && ms.getExpression() instanceof IdentifierTree id) { | ||
| var idPath = m_trees.getPath(m_root, id); | ||
| var identifierElement = m_trees.getElement(idPath); | ||
| if (identifierElement instanceof VariableElement ve | ||
| && m_task.getTypes().isSameType(getCoroutineType(), ve.asType())) { | ||
| parkInvocation = mit; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return super.visitBlock(node, param); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.