Skip to content

Commit 49ba8cf

Browse files
authored
Switch class to use a blanket impl for Destroy (#6125)
Right now, the class destroy impl is incorrectly generated (first discussed [in Discord](https://discord.com/channels/655572317891461132/941071822756143115/1418614787449032826)). If we want it to be correct, deferred definition logic would need to be added, and the declaration would need to be moved inside the `class` scope (along with whatever generic logic that needs). This instead switches to a blanket impl, to avoid creating latent bugs with generating the `impl` and function body in the wrong scope. This approach uses the same blanket impl as aggregate destruction that was added by #6098. The intent here is to allow progress on other parts of `Destroy`. For example, under this model the implementation of the function body could be done as part of lowering the specific.
1 parent 5705b94 commit 49ba8cf

File tree

230 files changed

+3642
-21993
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

230 files changed

+3642
-21993
lines changed

toolchain/check/BUILD

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ cc_library(
4141
"impl.cpp",
4242
"impl_lookup.cpp",
4343
"impl_validation.cpp",
44-
"implicit_type_impls.cpp",
4544
"import.cpp",
4645
"import_ref.cpp",
4746
"inst.cpp",
@@ -95,7 +94,6 @@ cc_library(
9594
"impl.h",
9695
"impl_lookup.h",
9796
"impl_validation.h",
98-
"implicit_type_impls.h",
9997
"import.h",
10098
"import_ref.h",
10199
"inst.h",

toolchain/check/check.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ auto CheckParseTrees(
470470
check_index < static_cast<int>(ready_to_check.size()); ++check_index) {
471471
auto* unit_info = ready_to_check[check_index];
472472
CheckUnit(unit_info, &tree_and_subtrees_getters, fs, clang_invocation,
473-
options.gen_implicit_type_impls, options.vlog_stream)
473+
options.vlog_stream)
474474
.Run();
475475
for (auto* incoming_import : unit_info->incoming_imports) {
476476
--incoming_import->imports_remaining;
@@ -519,7 +519,7 @@ auto CheckParseTrees(
519519
for (auto& unit_info : unit_infos) {
520520
if (unit_info.imports_remaining > 0) {
521521
CheckUnit(&unit_info, &tree_and_subtrees_getters, fs, clang_invocation,
522-
options.gen_implicit_type_impls, options.vlog_stream)
522+
options.vlog_stream)
523523
.Run();
524524
}
525525
}

toolchain/check/check.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,6 @@ struct CheckParseTreesOptions {
4242
// Whether to import the prelude.
4343
bool prelude_import = false;
4444

45-
// Whether to generate standard `impl`s for types, such as `Core.Destroy`.
46-
// This only controls generation of the `impl`; code which expects the `impl`
47-
// is expected to fail.
48-
bool gen_implicit_type_impls = true;
49-
5045
// If set, enables verbose output.
5146
llvm::raw_ostream* vlog_stream = nullptr;
5247

toolchain/check/check_unit.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ CheckUnit::CheckUnit(
6060
const Parse::GetTreeAndSubtreesStore* tree_and_subtrees_getters,
6161
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs,
6262
std::shared_ptr<clang::CompilerInvocation> clang_invocation,
63-
bool gen_implicit_type_impls, llvm::raw_ostream* vlog_stream)
63+
llvm::raw_ostream* vlog_stream)
6464
: unit_and_imports_(unit_and_imports),
6565
tree_and_subtrees_getter_(tree_and_subtrees_getters->Get(
6666
unit_and_imports->unit->sem_ir->check_ir_id())),
@@ -71,8 +71,7 @@ CheckUnit::CheckUnit(
7171
context_(&emitter_, tree_and_subtrees_getter_,
7272
unit_and_imports_->unit->sem_ir,
7373
GetImportedIRCount(unit_and_imports),
74-
unit_and_imports_->unit->total_ir_count, gen_implicit_type_impls,
75-
vlog_stream) {}
74+
unit_and_imports_->unit->total_ir_count, vlog_stream) {}
7675

7776
auto CheckUnit::Run() -> void {
7877
Timings::ScopedTiming timing(unit_and_imports_->unit->timings, "check");

toolchain/check/check_unit.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ class CheckUnit {
128128
const Parse::GetTreeAndSubtreesStore* tree_and_subtrees_getters,
129129
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs,
130130
std::shared_ptr<clang::CompilerInvocation> clang_invocation,
131-
bool gen_implicit_type_impls, llvm::raw_ostream* vlog_stream);
131+
llvm::raw_ostream* vlog_stream);
132132

133133
// Produces and checks the IR for the provided unit.
134134
auto Run() -> void;

toolchain/check/context.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,11 @@ namespace Carbon::Check {
1616
Context::Context(DiagnosticEmitterBase* emitter,
1717
Parse::GetTreeAndSubtreesFn tree_and_subtrees_getter,
1818
SemIR::File* sem_ir, int imported_ir_count, int total_ir_count,
19-
bool gen_implicit_type_impls, llvm::raw_ostream* vlog_stream)
19+
llvm::raw_ostream* vlog_stream)
2020
: emitter_(emitter),
2121
tree_and_subtrees_getter_(tree_and_subtrees_getter),
2222
sem_ir_(sem_ir),
2323
total_ir_count_(total_ir_count),
24-
gen_implicit_type_impls_(gen_implicit_type_impls),
2524
vlog_stream_(vlog_stream),
2625
node_stack_(sem_ir->parse_tree(), vlog_stream),
2726
inst_block_stack_("inst_block_stack_", *sem_ir, vlog_stream),

toolchain/check/context.h

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ class Context {
5858
explicit Context(DiagnosticEmitterBase* emitter,
5959
Parse::GetTreeAndSubtreesFn tree_and_subtrees_getter,
6060
SemIR::File* sem_ir, int imported_ir_count,
61-
int total_ir_count, bool gen_implicit_type_impls,
62-
llvm::raw_ostream* vlog_stream);
61+
int total_ir_count, llvm::raw_ostream* vlog_stream);
6362

6463
// Marks an implementation TODO. Always returns false.
6564
auto TODO(SemIR::LocId loc_id, std::string label) -> bool;
@@ -93,8 +92,6 @@ class Context {
9392
return parse_tree().tokens();
9493
}
9594

96-
auto gen_implicit_type_impls() -> bool { return gen_implicit_type_impls_; }
97-
9895
auto vlog_stream() -> llvm::raw_ostream* { return vlog_stream_; }
9996

10097
auto node_stack() -> NodeStack& { return node_stack_; }
@@ -330,10 +327,6 @@ class Context {
330327
// The total number of files.
331328
int total_ir_count_;
332329

333-
// Whether to generate standard `impl`s for types, such as `Core.Destroy`; see
334-
// `CheckParseTreesOptions`.
335-
bool gen_implicit_type_impls_;
336-
337330
// Whether to print verbose output.
338331
llvm::raw_ostream* vlog_stream_;
339332

toolchain/check/handle_class.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
#include "toolchain/check/generic.h"
1616
#include "toolchain/check/handle.h"
1717
#include "toolchain/check/impl.h"
18-
#include "toolchain/check/implicit_type_impls.h"
1918
#include "toolchain/check/import.h"
2019
#include "toolchain/check/import_ref.h"
2120
#include "toolchain/check/inst.h"
@@ -567,8 +566,6 @@ auto HandleParseNode(Context& context, Parse::ClassDefinitionId node_id)
567566
auto class_id =
568567
context.node_stack().Pop<Parse::NodeKind::ClassDefinitionStart>();
569568

570-
MakeClassDestroyImpl(context, class_id);
571-
572569
// The class type is now fully defined. Compute its object representation.
573570
ComputeClassObjectRepr(context, node_id, class_id,
574571
context.field_decls_stack().PeekArray(),

toolchain/check/impl_lookup.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -550,16 +550,18 @@ static auto TypeCanAggregateDestroy(Context& context,
550550
context.constant_values().GetInstId(query_self_const_id));
551551
CARBON_KIND_SWITCH(inst) {
552552
case CARBON_KIND(SemIR::ClassType class_type): {
553-
// Carbon classes will generate a `Destroy` impl, but we use this to
554-
// provide destruction for `Cpp`-scoped classes.
555-
// TODO: Don't provide this for C++ types that lack a destructor.
556553
auto class_info = context.classes().Get(class_type.class_id);
557-
return context.name_scopes().Get(class_info.scope_id).is_cpp_scope();
554+
// Incomplete classes can't be destroyed.
555+
// TODO: Return false if the object repr doesn't impl `Destroy`.
556+
// TODO: Return false for C++ types that lack a destructor.
557+
return class_info.is_complete();
558558
}
559559
case SemIR::ArrayType::Kind:
560560
case SemIR::MaybeUnformedType::Kind:
561561
case SemIR::StructType::Kind:
562562
case SemIR::TupleType::Kind:
563+
// TODO: Return false for types that indirectly reference a type that
564+
// doesn't impl `Destroy`.
563565
return true;
564566
default:
565567
return false;

toolchain/check/implicit_type_impls.cpp

Lines changed: 0 additions & 215 deletions
This file was deleted.

0 commit comments

Comments
 (0)