From 41b6bb5688b63a3b89451971e835b2a5814e0d43 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Tue, 21 Jan 2025 08:50:25 -0800 Subject: [PATCH] Update TODO for semantic checking (#4821) I believe `check_syntax` is already controlling the semantic vs syntactic merge, added in #4149. Other parts of the TODO are clarified per discussion. But this is tested, e.g. errors with the bool flipped: ``` impl i32 as I { + // CHECK:STDERR: method.carbon:[[@LINE+6]]:14: error: redeclaration syntax di ffers here [RedeclParamSyntaxDiffers] + // CHECK:STDERR: fn F[self: i32](other: i32) -> i32 = "int.sadd"; + // CHECK:STDERR: ^~~ + // CHECK:STDERR: method.carbon:[[@LINE-7]]:14: note: comparing with previous declaration here [RedeclParamSyntaxPrevious] + // CHECK:STDERR: fn F[self: Self](other: Self) -> Self; + // CHECK:STDERR: ^~~~ fn F[self: i32](other: i32) -> i32 = "int.sadd"; } ``` --- toolchain/check/impl.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/toolchain/check/impl.cpp b/toolchain/check/impl.cpp index 20d05a2e21b2..1293b68f2131 100644 --- a/toolchain/check/impl.cpp +++ b/toolchain/check/impl.cpp @@ -129,9 +129,11 @@ static auto CheckAssociatedFunctionImplementation( .generic_id, self_type_id, witness_inst_id); - // TODO: This should be a semantic check rather than a syntactic one. The - // functions should be allowed to have different signatures as long as we can - // synthesize a suitable thunk. + // TODO: The functions should be allowed to have different signatures as long + // as we can synthesize a suitable thunk. i.e., when there's an implicit + // conversion from the original parameter types to the overriding parameter + // types, and from the overriding return type to the original return type. + // Also, build that thunk. if (!CheckFunctionTypeMatches( context, context.functions().Get(impl_function_decl->function_id), context.functions().Get(interface_function_type.function_id),