From c6d8d29172b830f5438475b798a8049dc2fbfed6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=96zg=C3=BCr=20T=2E=20=C3=96nsoy?= <ozguronsoy@gmail.com>
Date: Tue, 8 Sep 2026 23:15:42 +0000
Subject: [PATCH] Diagnose redundant redeclarations in impl files (#7695)

While forward declarations in impl files are allowed, having one in the
impl file is redundant when we also have one in the API file.
---
 toolchain/check/merge.cpp                     |  5 +++
 .../class/no_definition_in_impl_file.carbon   | 39 ++++++++++++++++---
 .../no_definition_in_impl_file.carbon         | 39 ++++++++++++++++---
 .../no_definition_in_impl_file.carbon         | 39 ++++++++++++++++---
 4 files changed, 104 insertions(+), 18 deletions(-)

diff --git a/toolchain/check/merge.cpp b/toolchain/check/merge.cpp
index f84a4c51bf63..ffa56276e255 100644
--- a/toolchain/check/merge.cpp
+++ b/toolchain/check/merge.cpp
@@ -137,6 +137,11 @@ auto DiagnoseIfInvalidRedecl(Context& context, Lex::TokenKind decl_kind,
                              prev_decl.loc_id);
       return;
     }
+    if (!new_decl.is_definition) {
+      DiagnoseRedundant(context, decl_kind, name_id, new_decl.loc_id,
+                        prev_decl.loc_id);
+      return;
+    }
     return;
   }
 
diff --git a/toolchain/check/testdata/class/no_definition_in_impl_file.carbon b/toolchain/check/testdata/class/no_definition_in_impl_file.carbon
index f80fd7e5e00c..a071fc63b8b6 100644
--- a/toolchain/check/testdata/class/no_definition_in_impl_file.carbon
+++ b/toolchain/check/testdata/class/no_definition_in_impl_file.carbon
@@ -16,13 +16,10 @@ library "[[@TEST_NAME]]";
 
 class A;
 
-// --- todo_fail_decl_in_api_definition_in_impl.impl.carbon
+// --- decl_in_api_definition_in_impl.impl.carbon
 
 impl library "[[@TEST_NAME]]";
 
-// TODO: This should be diagnosed per #3762: A declaration should always add new information.
-class A;
-
 class A {}
 
 // --- use_decl_in_api.carbon
@@ -55,12 +52,42 @@ class C;
 
 impl library "[[@TEST_NAME]]";
 
+// CHECK:STDERR: fail_decl_in_api_decl_in_impl.impl.carbon:[[@LINE+12]]:1: error: redeclaration of `class C` is redundant [RedeclRedundant]
+// CHECK:STDERR: class C;
+// CHECK:STDERR: ^~~~~~~~
+// CHECK:STDERR: fail_decl_in_api_decl_in_impl.impl.carbon:[[@LINE-5]]:1: in import [InImport]
+// CHECK:STDERR: decl_in_api_decl_in_impl.carbon:4:1: note: previously declared here [RedeclPrevDecl]
+// CHECK:STDERR: class C;
+// CHECK:STDERR: ^~~~~~~~
+// CHECK:STDERR:
 // CHECK:STDERR: fail_decl_in_api_decl_in_impl.impl.carbon:[[@LINE+4]]:1: error: no definition found for declaration in impl file [MissingDefinitionInImpl]
 // CHECK:STDERR: class C;
 // CHECK:STDERR: ^~~~~~~~
 // CHECK:STDERR:
 class C;
 
+// --- decl_in_api_decl_and_definition_in_impl.carbon
+
+library "[[@TEST_NAME]]";
+
+class D;
+
+// --- fail_decl_in_api_decl_and_definition_in_impl.impl.carbon
+
+impl library "[[@TEST_NAME]]";
+
+// CHECK:STDERR: fail_decl_in_api_decl_and_definition_in_impl.impl.carbon:[[@LINE+8]]:1: error: redeclaration of `class D` is redundant [RedeclRedundant]
+// CHECK:STDERR: class D;
+// CHECK:STDERR: ^~~~~~~~
+// CHECK:STDERR: fail_decl_in_api_decl_and_definition_in_impl.impl.carbon:[[@LINE-5]]:1: in import [InImport]
+// CHECK:STDERR: decl_in_api_decl_and_definition_in_impl.carbon:4:1: note: previously declared here [RedeclPrevDecl]
+// CHECK:STDERR: class D;
+// CHECK:STDERR: ^~~~~~~~
+// CHECK:STDERR:
+class D;
+
+class D {}
+
 // --- decl_only_in_impl.carbon
 
 library "[[@TEST_NAME]]";
@@ -70,7 +97,7 @@ library "[[@TEST_NAME]]";
 impl library "[[@TEST_NAME]]";
 
 // CHECK:STDERR: fail_decl_only_in_impl.impl.carbon:[[@LINE+4]]:1: error: no definition found for declaration in impl file [MissingDefinitionInImpl]
-// CHECK:STDERR: class D;
+// CHECK:STDERR: class E;
 // CHECK:STDERR: ^~~~~~~~
 // CHECK:STDERR:
-class D;
+class E;
diff --git a/toolchain/check/testdata/function/declaration/no_definition_in_impl_file.carbon b/toolchain/check/testdata/function/declaration/no_definition_in_impl_file.carbon
index 96e03af73c05..270458dee837 100644
--- a/toolchain/check/testdata/function/declaration/no_definition_in_impl_file.carbon
+++ b/toolchain/check/testdata/function/declaration/no_definition_in_impl_file.carbon
@@ -16,13 +16,10 @@ library "[[@TEST_NAME]]";
 
 fn A();
 
-// --- todo_fail_decl_in_api_definition_in_impl.impl.carbon
+// --- decl_in_api_definition_in_impl.impl.carbon
 
 impl library "[[@TEST_NAME]]";
 
-// TODO: This should be diagnosed per #3762: A declaration should always add new information.
-fn A();
-
 fn A() {}
 
 // --- use_decl_in_api.carbon
@@ -55,12 +52,42 @@ fn C();
 
 impl library "[[@TEST_NAME]]";
 
+// CHECK:STDERR: fail_decl_in_api_decl_in_impl.impl.carbon:[[@LINE+12]]:1: error: redeclaration of `fn C` is redundant [RedeclRedundant]
+// CHECK:STDERR: fn C();
+// CHECK:STDERR: ^~~~~~~
+// CHECK:STDERR: fail_decl_in_api_decl_in_impl.impl.carbon:[[@LINE-5]]:1: in import [InImport]
+// CHECK:STDERR: decl_in_api_decl_in_impl.carbon:4:1: note: previously declared here [RedeclPrevDecl]
+// CHECK:STDERR: fn C();
+// CHECK:STDERR: ^~~~~~~
+// CHECK:STDERR:
 // CHECK:STDERR: fail_decl_in_api_decl_in_impl.impl.carbon:[[@LINE+4]]:1: error: no definition found for declaration in impl file [MissingDefinitionInImpl]
 // CHECK:STDERR: fn C();
 // CHECK:STDERR: ^~~~~~~
 // CHECK:STDERR:
 fn C();
 
+// --- decl_in_api_decl_and_definition_in_impl.carbon
+
+library "[[@TEST_NAME]]";
+
+fn D();
+
+// --- fail_decl_in_api_decl_and_definition_in_impl.impl.carbon
+
+impl library "[[@TEST_NAME]]";
+
+// CHECK:STDERR: fail_decl_in_api_decl_and_definition_in_impl.impl.carbon:[[@LINE+8]]:1: error: redeclaration of `fn D` is redundant [RedeclRedundant]
+// CHECK:STDERR: fn D();
+// CHECK:STDERR: ^~~~~~~
+// CHECK:STDERR: fail_decl_in_api_decl_and_definition_in_impl.impl.carbon:[[@LINE-5]]:1: in import [InImport]
+// CHECK:STDERR: decl_in_api_decl_and_definition_in_impl.carbon:4:1: note: previously declared here [RedeclPrevDecl]
+// CHECK:STDERR: fn D();
+// CHECK:STDERR: ^~~~~~~
+// CHECK:STDERR:
+fn D();
+
+fn D() {}
+
 // --- decl_only_in_impl.carbon
 
 library "[[@TEST_NAME]]";
@@ -70,7 +97,7 @@ library "[[@TEST_NAME]]";
 impl library "[[@TEST_NAME]]";
 
 // CHECK:STDERR: fail_decl_only_in_impl.impl.carbon:[[@LINE+4]]:1: error: no definition found for declaration in impl file [MissingDefinitionInImpl]
-// CHECK:STDERR: fn D();
+// CHECK:STDERR: fn E();
 // CHECK:STDERR: ^~~~~~~
 // CHECK:STDERR:
-fn D();
+fn E();
diff --git a/toolchain/check/testdata/interface/no_definition_in_impl_file.carbon b/toolchain/check/testdata/interface/no_definition_in_impl_file.carbon
index bd3d551f7b36..3a57a8517ea3 100644
--- a/toolchain/check/testdata/interface/no_definition_in_impl_file.carbon
+++ b/toolchain/check/testdata/interface/no_definition_in_impl_file.carbon
@@ -16,13 +16,10 @@ library "[[@TEST_NAME]]";
 
 interface A;
 
-// --- todo_fail_decl_in_api_definition_in_impl.impl.carbon
+// --- decl_in_api_definition_in_impl.impl.carbon
 
 impl library "[[@TEST_NAME]]";
 
-// TODO: This should be diagnosed per #3762: A declaration should always add new information.
-interface A;
-
 interface A {}
 
 // --- use_decl_in_api.carbon
@@ -55,12 +52,42 @@ interface C;
 
 impl library "[[@TEST_NAME]]";
 
+// CHECK:STDERR: fail_decl_in_api_decl_in_impl.impl.carbon:[[@LINE+12]]:1: error: redeclaration of `interface C` is redundant [RedeclRedundant]
+// CHECK:STDERR: interface C;
+// CHECK:STDERR: ^~~~~~~~~~~~
+// CHECK:STDERR: fail_decl_in_api_decl_in_impl.impl.carbon:[[@LINE-5]]:1: in import [InImport]
+// CHECK:STDERR: decl_in_api_decl_in_impl.carbon:4:1: note: previously declared here [RedeclPrevDecl]
+// CHECK:STDERR: interface C;
+// CHECK:STDERR: ^~~~~~~~~~~~
+// CHECK:STDERR:
 // CHECK:STDERR: fail_decl_in_api_decl_in_impl.impl.carbon:[[@LINE+4]]:1: error: no definition found for declaration in impl file [MissingDefinitionInImpl]
 // CHECK:STDERR: interface C;
 // CHECK:STDERR: ^~~~~~~~~~~~
 // CHECK:STDERR:
 interface C;
 
+// --- decl_in_api_decl_and_definition_in_impl.carbon
+
+library "[[@TEST_NAME]]";
+
+interface D;
+
+// --- fail_decl_in_api_decl_and_definition_in_impl.impl.carbon
+
+impl library "[[@TEST_NAME]]";
+
+// CHECK:STDERR: fail_decl_in_api_decl_and_definition_in_impl.impl.carbon:[[@LINE+8]]:1: error: redeclaration of `interface D` is redundant [RedeclRedundant]
+// CHECK:STDERR: interface D;
+// CHECK:STDERR: ^~~~~~~~~~~~
+// CHECK:STDERR: fail_decl_in_api_decl_and_definition_in_impl.impl.carbon:[[@LINE-5]]:1: in import [InImport]
+// CHECK:STDERR: decl_in_api_decl_and_definition_in_impl.carbon:4:1: note: previously declared here [RedeclPrevDecl]
+// CHECK:STDERR: interface D;
+// CHECK:STDERR: ^~~~~~~~~~~~
+// CHECK:STDERR:
+interface D;
+
+interface D {}
+
 // --- decl_only_in_impl.carbon
 
 library "[[@TEST_NAME]]";
@@ -70,7 +97,7 @@ library "[[@TEST_NAME]]";
 impl library "[[@TEST_NAME]]";
 
 // CHECK:STDERR: fail_decl_only_in_impl.impl.carbon:[[@LINE+4]]:1: error: no definition found for declaration in impl file [MissingDefinitionInImpl]
-// CHECK:STDERR: interface D;
+// CHECK:STDERR: interface E;
 // CHECK:STDERR: ^~~~~~~~~~~~
 // CHECK:STDERR:
-interface D;
+interface E;