[IR] Fix validating unbound symbols but not tree consistency

This fixes the checkUnboundSymbols() function, which specifies
checkUnboundSymbols = true, checkTreeConsistency = false.
Because only checkTreeConsistency was tested in IrValidator, it was a
no-op.

Also, relax the rule for early return in the case of duplicated nodes.
There is, unfortunately, a case where IR nodes are duplicated, but not
in a way that causes an infinite loop (KT-79807). But in
checkUnboundSymbols() we don't yet want to fail on that, as we only
check symbols.

KT-77819
KT-79807

Author: mcpiroman

compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/IrValidator.kt

@@ -185,7 +185,7 @@ private fun performBasicIrValidation(
 ) {
     // Phase 1: Traverse the IR tree to check for structural consistency.
     // If any issues are detected, validation stops here to avoid problems like infinite recursion during the next phase.
-    if (validatorConfig.checkTreeConsistency) {
+    if (validatorConfig.checkTreeConsistency || validatorConfig.checkUnboundSymbols) {
         try {
             element.checkTreeConsistency(reportError, validatorConfig)
         } catch (_: TreeConsistencyError) {

compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/checkers/CheckTreeConsistencyVisitor.kt

@@ -74,6 +74,7 @@ private class CheckTreeConsistencyVisitor(val reportError: ReportIrValidationErr
     }
 
     private fun handleParent(declaration: IrDeclaration, actualParent: IrDeclarationParent?) {
+        if (!config.checkTreeConsistency) return
         if (actualParent == null) return
         try {
             val assignedParent = declaration.parent
@@ -102,12 +103,17 @@ private class CheckTreeConsistencyVisitor(val reportError: ReportIrValidationErr
 
     private fun checkDuplicateNode(element: IrElement) {
         if (!visitedElements.add(element)) {
-            val renderString = if (element is IrTypeParameter) element.render() + " of " + element.parent.render() else element.render()
-            reportError(null, element, "Duplicate IR node: $renderString", parentChain)
+            if (config.checkTreeConsistency) {
+                hasInconsistency = true
+                val renderString = if (element is IrTypeParameter) element.render() + " of " + element.parent.render() else element.render()
+                reportError(null, element, "Duplicate IR node: $renderString", parentChain)
+            }
 
-            // The IR tree is completely messed up if it includes one element twice. It may not be a tree at all, there may be cycles.
-            // Give up early to avoid stack overflow.
-            throw TreeConsistencyError(element)
+            if (element in parentChain) {
+                // Not only is the same element twice in the tree, there is some cycle, so it is not a tree at all.
+                // Give up early to avoid stack overflow.
+                throw TreeConsistencyError(element)
+            }
         }
     }
 }

compiler/ir/backend.common/test/org/jetbrains/kotlin/backend/common/IrValidatorTest.kt

@@ -50,6 +50,17 @@ class IrValidatorTest {
 
     private lateinit var messageCollector: MessageCollectorImpl
     private lateinit var module: IrModuleFragment
+    private val defaultValidationConfig = IrValidatorConfig(
+        checkTypes = true,
+        checkValueScopes = true,
+        checkTypeParameterScopes = true,
+        checkCrossFileFieldUsage = true,
+        checkAllKotlinFieldsArePrivate = true,
+        checkVisibilities = true,
+        checkVarargTypes = true,
+        checkUnboundSymbols = true,
+        checkInlineFunctionUseSites = { it.symbol.owner.name.toString() != "inlineFunctionUseSiteNotPermitted" }
+    )
 
     @BeforeTest
     fun setUp() {
@@ -145,24 +156,19 @@ class IrValidatorTest {
         }
     }
 
-    private fun testValidation(mode: IrVerificationMode, tree: IrElement, expectedMessages: List<Message>) {
+    private fun testValidation(
+        mode: IrVerificationMode,
+        tree: IrElement,
+        expectedMessages: List<Message>,
+        config: IrValidatorConfig = defaultValidationConfig,
+    ) {
         runValidationAndAssert(mode) {
             validateIr(messageCollector, mode) {
                 performBasicIrValidation(
                     tree,
                     TestIrBuiltins,
                     phaseName = "IrValidatorTest",
-                    IrValidatorConfig(
-                        checkTypes = true,
-                        checkValueScopes = true,
-                        checkTypeParameterScopes = true,
-                        checkCrossFileFieldUsage = true,
-                        checkAllKotlinFieldsArePrivate = true,
-                        checkVisibilities = true,
-                        checkVarargTypes = true,
-                        checkUnboundSymbols = true,
-                        checkInlineFunctionUseSites = { it.symbol.owner.name.toString() != "inlineFunctionUseSiteNotPermitted" }
-                    )
+                    config
                 )
                 assertEquals(expectedMessages, messageCollector.messages)
             }
@@ -266,6 +272,55 @@ class IrValidatorTest {
         )
     }
 
+    @Test
+    fun `duplicated nodes do not stop validation if checkTreeConsistency = false`() {
+        testValidation(
+            IrVerificationMode.WARNING,
+            buildInvalidIrTreeWithLocations(),
+            listOf(
+                Message(
+                    WARNING,
+                    $$"""
+                    [IR VALIDATION] IrValidatorTest: unexpected type: expected kotlin.String, got kotlin.Any
+                    STRING_CONCATENATION type=kotlin.Any
+                      inside CALL 'public final fun foo ($receiver: kotlin.String, p0: kotlin.Any): kotlin.Unit declared in org.sample' type=kotlin.Any origin=null
+                        inside BLOCK_BODY
+                          inside FUN name:foo visibility:public modality:FINAL <> ($receiver:kotlin.String, p0:kotlin.Any) returnType:kotlin.Unit
+                            inside FILE fqName:org.sample fileName:test.kt
+                    """.trimIndent(),
+                    CompilerMessageLocation.create("test.kt", 1, 10, null),
+                ),
+                Message(
+                    WARNING,
+                    $$"""
+                    [IR VALIDATION] IrValidatorTest: unexpected type: expected kotlin.String, got kotlin.Any
+                    STRING_CONCATENATION type=kotlin.Any
+                      inside CALL 'public final fun foo ($receiver: kotlin.String, p0: kotlin.Any): kotlin.Unit declared in org.sample' type=kotlin.Any origin=null
+                        inside BLOCK_BODY
+                          inside FUN name:foo visibility:public modality:FINAL <> ($receiver:kotlin.String, p0:kotlin.Any) returnType:kotlin.Unit
+                            inside FILE fqName:org.sample fileName:test.kt
+                    """.trimIndent(),
+                    CompilerMessageLocation.create("test.kt", 1, 10, null),
+                ),
+                Message(
+                    WARNING,
+                    $$"""
+                    [IR VALIDATION] IrValidatorTest: unexpected type: expected kotlin.Unit, got kotlin.Any
+                    CALL 'public final fun foo ($receiver: kotlin.String, p0: kotlin.Any): kotlin.Unit declared in org.sample' type=kotlin.Any origin=null
+                      inside BLOCK_BODY
+                        inside FUN name:foo visibility:public modality:FINAL <> ($receiver:kotlin.String, p0:kotlin.Any) returnType:kotlin.Unit
+                          inside FILE fqName:org.sample fileName:test.kt
+                    """.trimIndent(),
+                    CompilerMessageLocation.create("test.kt", 1, 7, null),
+                ),
+            ),
+            config = IrValidatorConfig(
+                checkTreeConsistency = false,
+                checkTypes = true,
+            )
+        )
+    }
+
     @Test
     fun `no infinite recursion if there are cycles in IR`() {
         val klass = IrFactoryImpl.buildClass {