Start attempting ambiguous import merges. (#3827)

Just connecting this together. Note the diagnostics still need to be
cleaned up/improved, although this is correctly handling some cases.

This exposed an incorrect parameter passed in DiagnosticEmitter, which
is fixed here -- context_fn shouldn't include args. Note this was caught
by the CHECK, I'm just expanding on the message since I used the detail
to help figure out the bug.
This commit is contained in:
Jon Ross-Perkins
2024-04-02 11:21:47 +00:00
committed by GitHub
parent e66406ec93
commit da86a1bbb9
4 changed files with 21 additions and 40 deletions
+3 -3
View File
@@ -14,6 +14,7 @@
#include "toolchain/check/eval.h"
#include "toolchain/check/import_ref.h"
#include "toolchain/check/inst_block_stack.h"
#include "toolchain/check/merge.h"
#include "toolchain/diagnostics/diagnostic_emitter.h"
#include "toolchain/lex/tokenized_buffer.h"
#include "toolchain/parse/node_ids.h"
@@ -337,11 +338,10 @@ static auto LookupInImportIRScopes(Context& context, SemIRLoc loc,
continue;
}
auto import_inst_id = context.AddImportRef(import_ir_id, it->second);
TryResolveImportRefUnused(context, import_inst_id);
if (result_id.is_valid()) {
// TODO: Add generalized merge functionality (merge_decls.h?).
context.DiagnoseDuplicateName(import_inst_id, result_id);
MergeImportRef(context, import_inst_id, result_id);
} else {
TryResolveImportRefUnused(context, import_inst_id);
result_id = import_inst_id;
}
}
+6 -12
View File
@@ -62,18 +62,12 @@ var c: Other.C = {};
library "fail_merge_define_extern" api;
import Other library "define";
// CHECK:STDERR: fail_merge_define_extern.carbon:[[@LINE+12]]:1: In import.
// CHECK:STDERR: fail_merge_define_extern.carbon:[[@LINE+6]]:1: In import.
// CHECK:STDERR: import Other library "extern";
// CHECK:STDERR: ^~~~~~
// CHECK:STDERR: other_extern.carbon:5:1: ERROR: Duplicate name being declared in the same scope.
// CHECK:STDERR: other_extern.carbon:5:1: ERROR: Semantics TODO: `Merging not yet supported.`.
// CHECK:STDERR: class C;
// CHECK:STDERR: ^~~~~~~~
// CHECK:STDERR: fail_merge_define_extern.carbon:[[@LINE-7]]:1: In import.
// CHECK:STDERR: import Other library "define";
// CHECK:STDERR: ^~~~~~
// CHECK:STDERR: other_define.carbon:4:1: Name is previously declared here.
// CHECK:STDERR: class C {}
// CHECK:STDERR: ^~~~~~~~~
import Other library "extern";
// CHECK:STDERR: fail_merge_define_extern.carbon:[[@LINE+4]]:8: In name lookup for `C`.
@@ -260,10 +254,10 @@ var c: Other.C = {};
// CHECK:STDOUT:
// CHECK:STDOUT: fn @__global_init() {
// CHECK:STDOUT: !entry:
// CHECK:STDOUT: %.loc23_19.1: {} = struct_literal ()
// CHECK:STDOUT: %.loc23_19.2: init C = class_init (), file.%c.var [template = constants.%.4]
// CHECK:STDOUT: %.loc23_19.3: init C = converted %.loc23_19.1, %.loc23_19.2 [template = constants.%.4]
// CHECK:STDOUT: assign file.%c.var, %.loc23_19.3
// CHECK:STDOUT: %.loc17_19.1: {} = struct_literal ()
// CHECK:STDOUT: %.loc17_19.2: init C = class_init (), file.%c.var [template = constants.%.4]
// CHECK:STDOUT: %.loc17_19.3: init C = converted %.loc17_19.1, %.loc17_19.2 [template = constants.%.4]
// CHECK:STDOUT: assign file.%c.var, %.loc17_19.3
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT:
@@ -54,30 +54,14 @@ fn Run() {
Other.F2();
}
// --- fail_main_use_other_extern.carbon
// --- main_use_other_extern.carbon
library "use_other_extern" api;
import Other library "fn";
// CHECK:STDERR: fail_main_use_other_extern.carbon:[[@LINE+12]]:1: In import.
// CHECK:STDERR: import Other library "fn_extern";
// CHECK:STDERR: ^~~~~~
// CHECK:STDERR: other_fn_extern.carbon:4:1: ERROR: Duplicate name being declared in the same scope.
// CHECK:STDERR: extern fn F();
// CHECK:STDERR: ^~~~~~~~~~~~~~
// CHECK:STDERR: fail_main_use_other_extern.carbon:[[@LINE-7]]:1: In import.
// CHECK:STDERR: import Other library "fn";
// CHECK:STDERR: ^~~~~~
// CHECK:STDERR: other_fn.carbon:4:1: Name is previously declared here.
// CHECK:STDERR: fn F() {}
// CHECK:STDERR: ^~~~~~~~
import Other library "fn_extern";
fn Run() {
// CHECK:STDERR: fail_main_use_other_extern.carbon:[[@LINE+4]]:3: In name lookup for `F`.
// CHECK:STDERR: Other.F();
// CHECK:STDERR: ^~~~~~~
// CHECK:STDERR:
Other.F();
}
@@ -96,13 +80,13 @@ import Other library "fn";
// CHECK:STDERR: fail_main_use_other_ambiguous.carbon:[[@LINE+12]]:1: In import.
// CHECK:STDERR: import Other library "fn_conflict";
// CHECK:STDERR: ^~~~~~
// CHECK:STDERR: other_fn_conflict.carbon:4:1: ERROR: Duplicate name being declared in the same scope.
// CHECK:STDERR: other_fn_conflict.carbon:4:1: ERROR: Function redeclaration differs because of parameter count of 1.
// CHECK:STDERR: fn F(x: i32) {}
// CHECK:STDERR: ^~~~~~~~~~~~~~
// CHECK:STDERR: fail_main_use_other_ambiguous.carbon:[[@LINE-7]]:1: In import.
// CHECK:STDERR: import Other library "fn";
// CHECK:STDERR: ^~~~~~
// CHECK:STDERR: other_fn.carbon:4:1: Name is previously declared here.
// CHECK:STDERR: other_fn.carbon:4:1: Previously declared with parameter count of 0.
// CHECK:STDERR: fn F() {}
// CHECK:STDERR: ^~~~~~~~
import Other library "fn_conflict";
@@ -273,7 +257,7 @@ fn Other.G() {}
// CHECK:STDOUT:
// CHECK:STDOUT: fn @F2();
// CHECK:STDOUT:
// CHECK:STDOUT: --- fail_main_use_other_extern.carbon
// CHECK:STDOUT: --- main_use_other_extern.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
// CHECK:STDOUT: %.1: type = tuple_type () [template]
@@ -294,7 +278,7 @@ fn Other.G() {}
// CHECK:STDOUT: !entry:
// CHECK:STDOUT: %Other.ref: <namespace> = name_ref Other, file.%Other [template = file.%Other]
// CHECK:STDOUT: %F.ref: <function> = name_ref F, file.%import_ref.1 [template = imports.%F.1]
// CHECK:STDOUT: %.loc24: init () = call %F.ref()
// CHECK:STDOUT: %.loc8: init () = call %F.ref()
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT:
+7 -4
View File
@@ -101,9 +101,10 @@ class DiagnosticEmitter {
AddMessageWithDiagnosticLoc(
emitter_->converter_->ConvertLoc(
loc,
[&](DiagnosticLoc loc,
const Internal::DiagnosticBase<>& diagnostic_base) {
AddMessageWithDiagnosticLoc(loc, diagnostic_base, args);
[&](DiagnosticLoc context_loc,
const Internal::DiagnosticBase<>& context_diagnostic_base) {
AddMessageWithDiagnosticLoc(context_loc,
context_diagnostic_base, {});
}),
diagnostic_base, args);
}
@@ -136,7 +137,9 @@ class DiagnosticEmitter {
static auto FormatFn(const DiagnosticMessage& message,
std::index_sequence<N...> /*indices*/) -> std::string {
static_assert(sizeof...(Args) == sizeof...(N), "Invalid template args");
CARBON_CHECK(message.format_args.size() == sizeof...(Args));
CARBON_CHECK(message.format_args.size() == sizeof...(Args))
<< "Argument count mismatch on " << message.kind << ": "
<< message.format_args.size() << " != " << sizeof...(Args);
return llvm::formatv(
message.format.data(),
llvm::any_cast<