Skip to content
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

INSP: Implement unreachable code detection #6829

Merged
merged 2 commits into from Mar 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .idea/dictionaries/intellij_rust.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions src/main/kotlin/org/rust/ide/inspections/lints/RsLint.kt
Expand Up @@ -55,6 +55,14 @@ enum class RsLint(

UnreachablePattern("unreachable_patterns", listOf("unused")),

UnreachableCode("unreachable_code") {
override fun toHighlightingType(level: RsLintLevel): ProblemHighlightType =
when (level) {
WARN -> ProblemHighlightType.LIKE_UNUSED_SYMBOL
else -> super.toHighlightingType(level)
}
},

BareTraitObjects("bare_trait_objects", listOf("rust_2018_idioms"));

protected open fun toHighlightingType(level: RsLintLevel): ProblemHighlightType =
Expand Down
@@ -0,0 +1,82 @@
/*
* Use of this source code is governed by the MIT license that can be
* found in the LICENSE file.
*/

package org.rust.ide.inspections.lints

import com.intellij.openapi.util.Segment
import com.intellij.openapi.util.TextRange
import com.intellij.psi.PsiElement
import com.intellij.refactoring.suggested.stripWhitespace
import org.rust.ide.injected.isDoctestInjection
import org.rust.ide.inspections.RsProblemsHolder
import org.rust.ide.inspections.fixes.SubstituteTextFix
import org.rust.lang.core.psi.RsFunction
import org.rust.lang.core.psi.RsVisitor
import org.rust.lang.core.psi.ext.rangeWithPrevSpace
import org.rust.lang.core.psi.ext.startOffset
import org.rust.lang.core.types.controlFlowGraph
import org.rust.openapiext.document
import org.rust.stdext.mapToMutableList
import java.util.*

class RsUnreachableCodeInspection : RsLintInspection() {
override fun getDisplayName(): String = "Unreachable code"

override fun getLint(element: PsiElement): RsLint = RsLint.UnreachableCode

override fun buildVisitor(holder: RsProblemsHolder, isOnTheFly: Boolean) = object : RsVisitor() {
override fun visitFunction(func: RsFunction) {
artemmukhin marked this conversation as resolved.
Show resolved Hide resolved
if (func.isDoctestInjection) return
val controlFlowGraph = func.controlFlowGraph ?: return

val elementsToReport = controlFlowGraph
.unreachableElements
.takeIf { it.isNotEmpty() }
?: return

// Collect text ranges of unreachable elements and merge them in order to highlight
// most enclosing ranges entirely, instead of highlighting each element separately
val sortedRanges = elementsToReport
.filter { it.isPhysical }
.mapToMutableList { it.rangeWithPrevSpace }
.apply { sortWith(Segment.BY_START_OFFSET_THEN_END_OFFSET) }
.takeIf { it.isNotEmpty() }
?: return

val mergedRanges = mergeRanges(sortedRanges)
for (range in mergedRanges) {
registerUnreachableProblem(holder, func, range)
}
}
}

/** Merges intersecting (including adjacent) text ranges into one */
private fun mergeRanges(sortedRanges: List<TextRange>): Collection<TextRange> {
val mergedRanges = ArrayDeque<TextRange>()
mergedRanges.add(sortedRanges[0])
for (range in sortedRanges.drop(1)) {
val leftNeighbour = mergedRanges.peek()
if (leftNeighbour.intersects(range)) {
mergedRanges.pop()
mergedRanges.push(leftNeighbour.union(range))
} else {
mergedRanges.push(range)
}
}
return mergedRanges
}

private fun registerUnreachableProblem(holder: RsProblemsHolder, func: RsFunction, range: TextRange) {
val chars = func.containingFile.document?.immutableCharSequence ?: return
val strippedRangeInFunction = range.stripWhitespace(chars).shiftLeft(func.startOffset)

holder.registerLintProblem(
func,
"Unreachable code",
strippedRangeInFunction,
SubstituteTextFix.delete("Remove unreachable code", func.containingFile, range)
)
}
}
5 changes: 3 additions & 2 deletions src/main/kotlin/org/rust/lang/core/dfa/CFGBuilder.kt
Expand Up @@ -583,8 +583,9 @@ class CFGBuilder(
}
}

override fun visitLitExpr(litExpr: RsLitExpr) =
finishWith { straightLine(litExpr, pred, emptyList()) }
override fun visitLitExpr(litExpr: RsLitExpr) = finishWithAstNode(litExpr, pred)

override fun visitUnitExpr(unitExpr: RsUnitExpr) = finishWithAstNode(unitExpr, pred)

override fun visitMatchExpr(matchExpr: RsMatchExpr) {
fun processGuard(guard: RsMatchArmGuard, prevGuards: ArrayDeque<CFGNode>, guardStart: CFGNode): CFGNode {
Expand Down
116 changes: 108 additions & 8 deletions src/main/kotlin/org/rust/lang/core/dfa/ControlFlowGraph.kt
Expand Up @@ -6,6 +6,7 @@
package org.rust.lang.core.dfa

import com.intellij.psi.PsiElement
import org.rust.ide.utils.isEnabledByCfg
import org.rust.lang.core.psi.*
import org.rust.lang.core.psi.ext.*
import org.rust.lang.core.types.regions.ScopeTree
Expand All @@ -14,6 +15,7 @@ import org.rust.lang.core.types.type
import org.rust.lang.utils.Node
import org.rust.lang.utils.PresentableGraph
import org.rust.lang.utils.PresentableNodeData
import org.rust.stdext.buildSet

sealed class CFGNodeData(val element: RsElement? = null) : PresentableNodeData {

Expand All @@ -38,7 +40,7 @@ sealed class CFGNodeData(val element: RsElement? = null) : PresentableNodeData {

override val text: String
get() = when (this) {
is AST -> element?.cfgText()?.trim() ?: "AST"
is AST -> element!!.cfgText().trim()
Entry -> "Entry"
Exit -> "Exit"
Termination -> "Termination"
Expand All @@ -54,6 +56,7 @@ sealed class CFGNodeData(val element: RsElement? = null) : PresentableNodeData {
is RsLoopExpr -> "LOOP"
is RsForExpr -> "FOR"
is RsMatchExpr -> "MATCH"
is RsLambdaExpr -> "CLOSURE"
is RsExprStmt -> expr.cfgText() + ";"
else -> this.text
}
Expand All @@ -62,14 +65,16 @@ sealed class CFGNodeData(val element: RsElement? = null) : PresentableNodeData {
class CFGEdgeData(val exitingScopes: List<RsElement>)

typealias CFGNode = Node<CFGNodeData, CFGEdgeData>
typealias CFGGraph = PresentableGraph<CFGNodeData, CFGEdgeData>

class ControlFlowGraph private constructor(
val owner: RsElement,
val graph: PresentableGraph<CFGNodeData, CFGEdgeData>,
val graph: CFGGraph,
val body: RsBlock,
val regionScopeTree: ScopeTree,
val entry: CFGNode,
val exit: CFGNode
val exit: CFGNode,
val unreachableElements: Set<RsElement>
) {
companion object {
fun buildFor(body: RsBlock, regionScopeTree: ScopeTree): ControlFlowGraph {
Expand All @@ -84,13 +89,108 @@ class ControlFlowGraph private constructor(
cfgBuilder.addContainedEdge(bodyExit, fnExit)
cfgBuilder.addContainedEdge(fnExit, termination)

return ControlFlowGraph(owner, graph, body, regionScopeTree, entry, fnExit)
val unreachableElements = collectUnreachableElements(graph, entry)

return ControlFlowGraph(owner, graph, body, regionScopeTree, entry, fnExit, unreachableElements)
}
}

// TODO: Implement dead code elimination
@Suppress("unused")
fun isNodeReachable(item: RsElement) = graph.depthFirstTraversal(entry).any { it.data.element == item }
/**
* Collects unreachable elements, i.e. elements cannot be reached by execution of [body].
*
* Only collects [RsExprStmt]s, [RsLetDecl]s and tail expressions, ignoring conditionally disabled.
*/
private fun collectUnreachableElements(graph: CFGGraph, entry: CFGNode): Set<RsElement> {
/**
* In terms of our control-flow graph, a [RsElement]'s node is not reachable if it cannot be fully executed,
* since [CFGBuilder] processes elements in post-order.
* But partially executed elements should not be treated as unreachable because the execution
* actually reaches them; only some parts of such elements should be treated as unreachable instead.
*
* So, first of all, collect all the unexecuted [RsElement]s (including partially executed)
*/
val unexecutedElements = buildSet<RsElement> {
val fullyExecutedNodeIndices = graph.depthFirstTraversal(entry).map { it.index }.toSet()
graph.forEachNode { node ->
if (node.index !in fullyExecutedNodeIndices) {
val element = node.data.element ?: return@forEachNode
add(element)
}
}
}
val unexecutedStmts = unexecutedElements
.filterIsInstance<RsStmt>()
val unexecutedTailExprs = unexecutedElements
artemmukhin marked this conversation as resolved.
Show resolved Hide resolved
.filterIsInstance<RsBlock>()
.mapNotNull { block ->
block.expr?.takeIf { expr ->
when (expr) {
is RsMacroExpr -> expr.macroCall in unexecutedElements
else -> expr in unexecutedElements
}
}
}

/**
* If [unexecuted] produces termination by itself
* (which means its inner expression type is [TyNever], see the corresponding checks below),
* it should be treated as unreachable only if the execution does not reach the beginning of [unexecuted].
* The latter means that previous statement is not fully executed.
* This is needed in order not to treat the first, basically reachable, `return` as unreachable
*/
fun isUnreachable(unexecuted: RsElement, innerExpr: RsExpr?): Boolean {
artemmukhin marked this conversation as resolved.
Show resolved Hide resolved
// Ignore conditionally disabled elements
if (!unexecuted.isEnabledByCfg) {
return false
}
if (innerExpr != null && !innerExpr.isEnabledByCfg) {
return false
}

// Unexecuted element is definitely unreachable if its inner expression does not produce termination
if (innerExpr != null && innerExpr.type !is TyNever) {
return true
}

// Otherwise, unexecuted element's child produces termination and therefore
// we should check if it is really unreachable or just not fully executed
val parentBlock = unexecuted.ancestorStrict<RsBlock>() ?: return false
val blockStmts = parentBlock.stmtList.takeIf { it.isNotEmpty() } ?: return false
val blockTailExpr = parentBlock.expr
val index = blockStmts.indexOf(unexecuted)
return when {
index >= 1 -> {
// `{ ... <unreachable>; <element>; ... }`
blockStmts[index - 1] in unexecutedElements
}
unexecuted == blockTailExpr -> {
// `{ ... <unreachable>; <element> }`
blockStmts.last() in unexecutedElements
}
else -> false
}
}

val unreachableElements = mutableSetOf<RsElement>()

for (stmt in unexecutedStmts) {
when {
stmt is RsExprStmt && isUnreachable(stmt, stmt.expr) -> {
unreachableElements.add(stmt)
}
stmt is RsLetDecl && isUnreachable(stmt, stmt.expr) -> {
unreachableElements.add(stmt)
}
}
}
for (tailExpr in unexecutedTailExprs) {
if (isUnreachable(tailExpr, tailExpr)) {
unreachableElements.add(tailExpr)
}
}

return unreachableElements
}
}

fun buildLocalIndex(): HashMap<RsElement, MutableList<CFGNode>> {
val table = hashMapOf<RsElement, MutableList<CFGNode>>()
Expand Down
5 changes: 5 additions & 0 deletions src/main/resources/META-INF/rust-core.xml
Expand Up @@ -589,6 +589,11 @@
enabledByDefault="true" level="WEAK WARNING"
implementationClass="org.rust.ide.inspections.RsDuplicatedTraitMethodBindingInspection"/>

<localInspection language="Rust" groupPath="Rust" groupName="Lints"
displayName="Unreachable code"
enabledByDefault="true" level="WARNING"
implementationClass="org.rust.ide.inspections.lints.RsUnreachableCodeInspection"/>

<!-- Surrounders -->

<lang.surroundDescriptor language="Rust"
Expand Down
@@ -0,0 +1 @@
Reports unreachable code.