Skip to content

Commit

Permalink
ownership fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@jonmeow
jonmeow committed Apr 18, 2024
1 parent d4852f2 commit 8c6885a
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 37 deletions.
9 changes: 8 additions & 1 deletion toolchain/check/class.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
namespace Carbon::Check {

auto MergeClassRedecl(Context& context, SemIRLoc new_loc,
SemIR::Class& new_class, bool /*new_is_import*/,
SemIR::Class& new_class, bool new_is_import,
bool new_is_definition, bool new_is_extern,
SemIR::ClassId prev_class_id, bool prev_is_extern,
SemIR::ImportIRInstId prev_import_ir_inst_id) -> bool {
Expand Down Expand Up @@ -51,6 +51,13 @@ auto MergeClassRedecl(Context& context, SemIRLoc new_loc,
prev_class.object_repr_id = new_class.object_repr_id;
}

if ((prev_import_ir_inst_id.is_valid() && !new_is_import) ||
(prev_is_extern && !new_is_extern)) {
prev_class.decl_id = new_class.decl_id;
ReplacePrevInstForMerge(
context, prev_class.enclosing_scope_id, prev_class.name_id,
new_is_import ? new_loc.inst_id : new_class.decl_id);
}
return true;
}

Expand Down
47 changes: 30 additions & 17 deletions toolchain/check/testdata/class/extern.carbon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,13 @@ class C {}
library "fail_extern_decl_after_decl" api;

class C;
// CHECK:STDERR: fail_extern_decl_after_decl.carbon:[[@LINE+6]]:1: ERROR: Redeclaration of `class C` is redundant.
// CHECK:STDERR: fail_extern_decl_after_decl.carbon:[[@LINE+7]]:1: ERROR: Redeclaration of `class C` is redundant.
// CHECK:STDERR: extern class C;
// CHECK:STDERR: ^~~~~~~~~~~~~~~
// CHECK:STDERR: fail_extern_decl_after_decl.carbon:[[@LINE-4]]:1: Previously declared here.
// CHECK:STDERR: class C;
// CHECK:STDERR: ^~~~~~~~
// CHECK:STDERR:
extern class C;

// --- import_extern_decl_then_decl.carbon
Expand All @@ -148,12 +149,24 @@ library "import_extern_decl_then_def" api;
import library "extern_decl";
import library "def";

// --- todo_fail_import_ownership_conflict.carbon
// --- fail_import_ownership_conflict.carbon

library "fail_import_ownership_conflict" api;

import library "extern_decl";
import library "decl";
// CHECK:STDERR: fail_import_ownership_conflict.carbon:[[@LINE+12]]:1: In import.
// CHECK:STDERR: import library "def";
// CHECK:STDERR: ^~~~~~
// CHECK:STDERR: def.carbon:4:1: ERROR: Only one library can declare `class C` without `extern`.
// CHECK:STDERR: class C {}
// CHECK:STDERR: ^~~~~~~~~
// CHECK:STDERR: fail_import_ownership_conflict.carbon:[[@LINE-7]]:1: In import.
// CHECK:STDERR: import library "decl";
// CHECK:STDERR: ^~~~~~
// CHECK:STDERR: decl.carbon:4:1: Previously declared here.
// CHECK:STDERR: class C;
// CHECK:STDERR: ^~~~~~~~
import library "def";

// --- import_extern_decl_copy.carbon
Expand Down Expand Up @@ -345,7 +358,7 @@ extern class C;
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .Core = %Core
// CHECK:STDOUT: .C = %.loc4
// CHECK:STDOUT: .C = %C.decl.loc12
// CHECK:STDOUT: }
// CHECK:STDOUT: %Core: <namespace> = namespace [template] {}
// CHECK:STDOUT: %C.decl.loc4: type = class_decl @C [template = constants.%C] {}
Expand Down Expand Up @@ -393,7 +406,7 @@ extern class C;
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .Core = %Core
// CHECK:STDOUT: .C = %.loc4
// CHECK:STDOUT: .C = %C.decl.loc12
// CHECK:STDOUT: }
// CHECK:STDOUT: %Core: <namespace> = namespace [template] {}
// CHECK:STDOUT: %C.decl.loc4: type = class_decl @C [template = constants.%C] {}
Expand All @@ -420,8 +433,8 @@ extern class C;
// CHECK:STDOUT: }
// CHECK:STDOUT: %Core: <namespace> = namespace [template] {}
// CHECK:STDOUT: %C.decl.loc4: type = class_decl @C [template = constants.%C] {}
// CHECK:STDOUT: %C.decl.loc11: type = class_decl @C [template = constants.%C] {}
// CHECK:STDOUT: %.loc11: type = extern_decl %C.decl.loc11 [template = constants.%.1]
// CHECK:STDOUT: %C.decl.loc12: type = class_decl @C [template = constants.%C] {}
// CHECK:STDOUT: %.loc12: type = extern_decl %C.decl.loc12 [template = constants.%.1]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: class @C;
Expand All @@ -436,7 +449,7 @@ extern class C;
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .C = %import_ref.1
// CHECK:STDOUT: .C = %import_ref.2
// CHECK:STDOUT: .Core = %Core
// CHECK:STDOUT: }
// CHECK:STDOUT: %import_ref.1: type = import_ref ir2, inst+3, loaded [template = constants.%.1]
Expand Down Expand Up @@ -485,7 +498,7 @@ extern class C;
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .C = %import_ref.1
// CHECK:STDOUT: .C = %import_ref.2
// CHECK:STDOUT: .Core = %Core
// CHECK:STDOUT: }
// CHECK:STDOUT: %import_ref.1: type = import_ref ir2, inst+3, loaded [template = constants.%.2]
Expand All @@ -506,7 +519,7 @@ extern class C;
// CHECK:STDOUT: .Self = file.%import_ref.3
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: --- todo_fail_import_ownership_conflict.carbon
// CHECK:STDOUT: --- fail_import_ownership_conflict.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
// CHECK:STDOUT: %C.1: type = class_type @C.1 [template]
Expand All @@ -518,7 +531,7 @@ extern class C;
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .C = %import_ref.1
// CHECK:STDOUT: .C = %import_ref.2
// CHECK:STDOUT: .Core = %Core
// CHECK:STDOUT: }
// CHECK:STDOUT: %import_ref.1: type = import_ref ir2, inst+3, loaded [template = constants.%.1]
Expand All @@ -531,13 +544,13 @@ extern class C;
// CHECK:STDOUT: %Core: <namespace> = namespace [template] {}
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: class @C.1;
// CHECK:STDOUT:
// CHECK:STDOUT: class @C.2 {
// CHECK:STDOUT: class @C.1 {
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .Self = file.%import_ref.4
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: class @C.2;
// CHECK:STDOUT:
// CHECK:STDOUT: class @C.3 {
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .Self = file.%import_ref.4
Expand Down Expand Up @@ -577,8 +590,8 @@ extern class C;
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .C = %import_ref
// CHECK:STDOUT: .Core = %Core
// CHECK:STDOUT: .C = %C.decl.loc6
// CHECK:STDOUT: }
// CHECK:STDOUT: %import_ref: type = import_ref ir2, inst+3, loaded [template = constants.%.1]
// CHECK:STDOUT: %Core: <namespace> = namespace [template] {}
Expand All @@ -599,8 +612,8 @@ extern class C;
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .C = %import_ref
// CHECK:STDOUT: .Core = %Core
// CHECK:STDOUT: .C = %C.decl.loc6
// CHECK:STDOUT: }
// CHECK:STDOUT: %import_ref: type = import_ref ir2, inst+2, loaded [template = constants.%C]
// CHECK:STDOUT: %Core: <namespace> = namespace [template] {}
Expand All @@ -622,8 +635,8 @@ extern class C;
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .C = %import_ref.1
// CHECK:STDOUT: .Core = %Core
// CHECK:STDOUT: .C = %C.decl.loc6
// CHECK:STDOUT: }
// CHECK:STDOUT: %import_ref.1: type = import_ref ir2, inst+2, loaded [template = constants.%C]
// CHECK:STDOUT: %Core: <namespace> = namespace [template] {}
Expand All @@ -649,8 +662,8 @@ extern class C;
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .C = %import_ref.1
// CHECK:STDOUT: .Core = %Core
// CHECK:STDOUT: .C = %C.decl.loc6
// CHECK:STDOUT: }
// CHECK:STDOUT: %import_ref.1: type = import_ref ir2, inst+2, loaded [template = constants.%C]
// CHECK:STDOUT: %Core: <namespace> = namespace [template] {}
Expand Down
2 changes: 1 addition & 1 deletion toolchain/check/testdata/class/fail_import_misuses.carbon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ var a: Incomplete;
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .Empty = %import_ref.1
// CHECK:STDOUT: .Incomplete = %import_ref.2
// CHECK:STDOUT: .Core = %Core
// CHECK:STDOUT: .Empty = %Empty.decl.loc16
// CHECK:STDOUT: .a = %a
// CHECK:STDOUT: }
// CHECK:STDOUT: %import_ref.1: type = import_ref ir2, inst+2, loaded [template = constants.%Empty]
Expand Down
24 changes: 6 additions & 18 deletions ...lass/fail_todo_import_forward_decl.carbon → ...testdata/class/import_forward_decl.carbon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,10 @@ library "a" api;

class ForwardDecl;

// --- fail_b.carbon
// --- a.impl.carbon

library "b" api;
library "a" impl;

import library "a";

// TODO: This should probably have a valid form.
// CHECK:STDERR: fail_b.carbon:[[@LINE+9]]:1: ERROR: Only one library can declare `class ForwardDecl` without `extern`.
// CHECK:STDERR: class ForwardDecl {
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~
// CHECK:STDERR: fail_b.carbon:[[@LINE-6]]:1: In import.
// CHECK:STDERR: import library "a";
// CHECK:STDERR: ^~~~~~
// CHECK:STDERR: a.carbon:4:1: Previously declared here.
// CHECK:STDERR: class ForwardDecl;
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~
class ForwardDecl {
}

Expand All @@ -46,7 +34,7 @@ class ForwardDecl {
// CHECK:STDOUT:
// CHECK:STDOUT: class @ForwardDecl;
// CHECK:STDOUT:
// CHECK:STDOUT: --- fail_b.carbon
// CHECK:STDOUT: --- a.impl.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
// CHECK:STDOUT: %ForwardDecl: type = class_type @ForwardDecl [template]
Expand All @@ -55,12 +43,12 @@ class ForwardDecl {
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .ForwardDecl = %import_ref
// CHECK:STDOUT: .Core = %Core
// CHECK:STDOUT: .ForwardDecl = %ForwardDecl.decl.loc4
// CHECK:STDOUT: }
// CHECK:STDOUT: %import_ref: type = import_ref ir2, inst+2, loaded [template = constants.%ForwardDecl]
// CHECK:STDOUT: %import_ref: type = import_ref ir1, inst+2, loaded [template = constants.%ForwardDecl]
// CHECK:STDOUT: %Core: <namespace> = namespace [template] {}
// CHECK:STDOUT: %ForwardDecl.decl.loc16: type = class_decl @ForwardDecl [template = constants.%ForwardDecl] {
// CHECK:STDOUT: %ForwardDecl.decl.loc4: type = class_decl @ForwardDecl [template = constants.%ForwardDecl] {
// CHECK:STDOUT: %ForwardDecl.decl.1: invalid = class_decl @ForwardDecl [template = constants.%ForwardDecl] {}
// CHECK:STDOUT: }
// CHECK:STDOUT: }
Expand Down

0 comments on commit 8c6885a

Please sign in to comment.