Skip to content

Commit

Permalink
Add SemIR support for virtual functions (carbon-language#4272)
Browse files Browse the repository at this point in the history
I guess this technically would also allow code to pass check that hasn't
before, and that isn't covered by tests (since it's masked by other
failures in the tests that already test this functionality) - should I
add another test/add some code to a valid test case?

Also, this'll miscompile in lowering, since there's no support there yet
- should I do anything about that to make lowering fail in some way? Or
is it acceptable that some things just silently mis-lower? (I could add
a currently-miscompiling test case too, to demonstrate this? (not sure
if the autogenerated tests leave space for comments that would explain
that the currently-tested behavior is incorrect?))

Is the addition to EntityWithParamsBase suitable? of course not all
functions can be virtual, so it's a wasted bit at the moment for all
those cases (though it's free, since it's bitpacked - but as we want to
add more bits in there it might not be a scalable solution)?

---------

Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
  • Loading branch information
dwblaikie and jonmeow committed Sep 10, 2024
1 parent 8ac9c80 commit 5806d83
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 92 deletions.
17 changes: 13 additions & 4 deletions toolchain/check/handle_function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,17 @@ static auto BuildFunctionDecl(Context& context,
DiagnoseModifiers(context, introducer, is_definition, parent_scope_inst_id,
parent_scope_inst);
bool is_extern = introducer.modifier_set.HasAnyOf(KeywordModifierSet::Extern);
if (introducer.modifier_set.HasAnyOf(KeywordModifierSet::Method)) {
context.TODO(introducer.modifier_node_id(ModifierOrder::Decl),
"method modifier");
SemIR::FunctionFields::VirtualModifier virtual_modifier =
SemIR::FunctionFields::VirtualModifier::None;

if (introducer.modifier_set.HasAnyOf(KeywordModifierSet::Virtual)) {
virtual_modifier = SemIR::FunctionFields::VirtualModifier::Virtual;
}
if (introducer.modifier_set.HasAnyOf(KeywordModifierSet::Abstract)) {
virtual_modifier = SemIR::FunctionFields::VirtualModifier::Abstract;
}
if (introducer.modifier_set.HasAnyOf(KeywordModifierSet::Impl)) {
virtual_modifier = SemIR::FunctionFields::VirtualModifier::Impl;
}
if (introducer.modifier_set.HasAnyOf(KeywordModifierSet::Interface)) {
// TODO: Once we are saving the modifiers for a function, add check that
Expand All @@ -216,7 +224,8 @@ static auto BuildFunctionDecl(Context& context,
auto function_info =
SemIR::Function{{name_context.MakeEntityWithParamsBase(
name, decl_id, is_extern, introducer.extern_library)},
{.return_storage_id = return_storage_id}};
{.return_storage_id = return_storage_id,
.virtual_modifier = virtual_modifier}};
if (is_definition) {
function_info.definition_id = decl_id;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
//
// AUTOUPDATE
// TIP: To test this file alone, run:
// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/class/fail_todo_modifiers.carbon
// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/class/todo_access_modifiers.carbon
// TIP: To dump output, run:
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/class/fail_todo_modifiers.carbon
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/class/todo_access_modifiers.carbon

// TODO: Test calls to these (member access control is not yet implemented).
class Access {
Expand All @@ -19,42 +19,7 @@ class Access {
protected var l: i32;
}

base class Base {

// CHECK:STDERR: fail_todo_modifiers.carbon:[[@LINE+4]]:3: ERROR: Semantics TODO: `method modifier`.
// CHECK:STDERR: virtual fn H();
// CHECK:STDERR: ^~~~~~~
// CHECK:STDERR:
virtual fn H();

// CHECK:STDERR: fail_todo_modifiers.carbon:[[@LINE+4]]:3: ERROR: Semantics TODO: `method modifier`.
// CHECK:STDERR: impl fn I();
// CHECK:STDERR: ^~~~
// CHECK:STDERR:
impl fn I();
}

abstract class Abstract {

// CHECK:STDERR: fail_todo_modifiers.carbon:[[@LINE+4]]:3: ERROR: Semantics TODO: `method modifier`.
// CHECK:STDERR: abstract fn J();
// CHECK:STDERR: ^~~~~~~~
// CHECK:STDERR:
abstract fn J();

// CHECK:STDERR: fail_todo_modifiers.carbon:[[@LINE+4]]:3: ERROR: Semantics TODO: `method modifier`.
// CHECK:STDERR: virtual fn K();
// CHECK:STDERR: ^~~~~~~
// CHECK:STDERR:
virtual fn K();

// CHECK:STDERR: fail_todo_modifiers.carbon:[[@LINE+3]]:3: ERROR: Semantics TODO: `method modifier`.
// CHECK:STDERR: impl fn L();
// CHECK:STDERR: ^~~~
impl fn L();
}

// CHECK:STDOUT: --- fail_todo_modifiers.carbon
// CHECK:STDOUT: --- todo_access_modifiers.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
// CHECK:STDOUT: %Access: type = class_type @Access [template]
Expand All @@ -67,19 +32,6 @@ abstract class Abstract {
// CHECK:STDOUT: %Int32: %Int32.type = struct_value () [template]
// CHECK:STDOUT: %.2: type = unbound_element_type %Access, i32 [template]
// CHECK:STDOUT: %.3: type = struct_type {.k: i32, .l: i32} [template]
// CHECK:STDOUT: %Base: type = class_type @Base [template]
// CHECK:STDOUT: %H.type: type = fn_type @H [template]
// CHECK:STDOUT: %H: %H.type = struct_value () [template]
// CHECK:STDOUT: %I.type: type = fn_type @I [template]
// CHECK:STDOUT: %I: %I.type = struct_value () [template]
// CHECK:STDOUT: %.4: type = struct_type {} [template]
// CHECK:STDOUT: %Abstract: type = class_type @Abstract [template]
// CHECK:STDOUT: %J.type: type = fn_type @J [template]
// CHECK:STDOUT: %J: %J.type = struct_value () [template]
// CHECK:STDOUT: %K.type: type = fn_type @K [template]
// CHECK:STDOUT: %K: %K.type = struct_value () [template]
// CHECK:STDOUT: %L.type: type = fn_type @L [template]
// CHECK:STDOUT: %L: %L.type = struct_value () [template]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: imports {
Expand All @@ -101,13 +53,9 @@ abstract class Abstract {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .Core = imports.%Core
// CHECK:STDOUT: .Access = %Access.decl
// CHECK:STDOUT: .Base = %Base.decl
// CHECK:STDOUT: .Abstract = %Abstract.decl
// CHECK:STDOUT: }
// CHECK:STDOUT: %Core.import = import Core
// CHECK:STDOUT: %Access.decl: type = class_decl @Access [template = constants.%Access] {}
// CHECK:STDOUT: %Base.decl: type = class_decl @Base [template = constants.%Base] {}
// CHECK:STDOUT: %Abstract.decl: type = class_decl @Abstract [template = constants.%Abstract] {}
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: class @Access {
Expand All @@ -130,41 +78,9 @@ abstract class Abstract {
// CHECK:STDOUT: .l [protected] = %.loc19_18
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: class @Base {
// CHECK:STDOUT: %H.decl: %H.type = fn_decl @H [template = constants.%H] {}
// CHECK:STDOUT: %I.decl: %I.type = fn_decl @I [template = constants.%I] {}
// CHECK:STDOUT:
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .Self = constants.%Base
// CHECK:STDOUT: .H = %H.decl
// CHECK:STDOUT: .I = %I.decl
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: class @Abstract {
// CHECK:STDOUT: %J.decl: %J.type = fn_decl @J [template = constants.%J] {}
// CHECK:STDOUT: %K.decl: %K.type = fn_decl @K [template = constants.%K] {}
// CHECK:STDOUT: %L.decl: %L.type = fn_decl @L [template = constants.%L] {}
// CHECK:STDOUT:
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .Self = constants.%Abstract
// CHECK:STDOUT: .J = %J.decl
// CHECK:STDOUT: .K = %K.decl
// CHECK:STDOUT: .L = %L.decl
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @F();
// CHECK:STDOUT:
// CHECK:STDOUT: fn @G();
// CHECK:STDOUT:
// CHECK:STDOUT: fn @Int32() -> type = "int.make_type_32";
// CHECK:STDOUT:
// CHECK:STDOUT: fn @H();
// CHECK:STDOUT:
// CHECK:STDOUT: fn @I();
// CHECK:STDOUT:
// CHECK:STDOUT: fn @J();
// CHECK:STDOUT:
// CHECK:STDOUT: fn @K();
// CHECK:STDOUT:
// CHECK:STDOUT: fn @L();
// CHECK:STDOUT:
100 changes: 100 additions & 0 deletions toolchain/check/testdata/class/virtual_modifiers.carbon
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
// Exceptions. See /LICENSE for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
// AUTOUPDATE
// TIP: To test this file alone, run:
// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/class/virtual_modifiers.carbon
// TIP: To dump output, run:
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/class/virtual_modifiers.carbon


base class Base {
virtual fn H();

impl fn I();
}

abstract class Abstract {
abstract fn J();

virtual fn K();

impl fn L();
}

// CHECK:STDOUT: --- virtual_modifiers.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
// CHECK:STDOUT: %Base: type = class_type @Base [template]
// CHECK:STDOUT: %H.type: type = fn_type @H [template]
// CHECK:STDOUT: %.1: type = tuple_type () [template]
// CHECK:STDOUT: %H: %H.type = struct_value () [template]
// CHECK:STDOUT: %I.type: type = fn_type @I [template]
// CHECK:STDOUT: %I: %I.type = struct_value () [template]
// CHECK:STDOUT: %.2: type = struct_type {} [template]
// CHECK:STDOUT: %Abstract: type = class_type @Abstract [template]
// CHECK:STDOUT: %J.type: type = fn_type @J [template]
// CHECK:STDOUT: %J: %J.type = struct_value () [template]
// CHECK:STDOUT: %K.type: type = fn_type @K [template]
// CHECK:STDOUT: %K: %K.type = struct_value () [template]
// CHECK:STDOUT: %L.type: type = fn_type @L [template]
// CHECK:STDOUT: %L: %L.type = struct_value () [template]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: imports {
// CHECK:STDOUT: %Core: <namespace> = namespace file.%Core.import, [template] {
// CHECK:STDOUT: import Core//prelude
// CHECK:STDOUT: import Core//prelude/operators
// CHECK:STDOUT: import Core//prelude/types
// CHECK:STDOUT: import Core//prelude/operators/arithmetic
// CHECK:STDOUT: import Core//prelude/operators/as
// CHECK:STDOUT: import Core//prelude/operators/bitwise
// CHECK:STDOUT: import Core//prelude/operators/comparison
// CHECK:STDOUT: import Core//prelude/types/bool
// CHECK:STDOUT: }
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .Core = imports.%Core
// CHECK:STDOUT: .Base = %Base.decl
// CHECK:STDOUT: .Abstract = %Abstract.decl
// CHECK:STDOUT: }
// CHECK:STDOUT: %Core.import = import Core
// CHECK:STDOUT: %Base.decl: type = class_decl @Base [template = constants.%Base] {}
// CHECK:STDOUT: %Abstract.decl: type = class_decl @Abstract [template = constants.%Abstract] {}
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: class @Base {
// CHECK:STDOUT: %H.decl: %H.type = fn_decl @H [template = constants.%H] {}
// CHECK:STDOUT: %I.decl: %I.type = fn_decl @I [template = constants.%I] {}
// CHECK:STDOUT:
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .Self = constants.%Base
// CHECK:STDOUT: .H = %H.decl
// CHECK:STDOUT: .I = %I.decl
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: class @Abstract {
// CHECK:STDOUT: %J.decl: %J.type = fn_decl @J [template = constants.%J] {}
// CHECK:STDOUT: %K.decl: %K.type = fn_decl @K [template = constants.%K] {}
// CHECK:STDOUT: %L.decl: %L.type = fn_decl @L [template = constants.%L] {}
// CHECK:STDOUT:
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .Self = constants.%Abstract
// CHECK:STDOUT: .J = %J.decl
// CHECK:STDOUT: .K = %K.decl
// CHECK:STDOUT: .L = %L.decl
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: virtual fn @H();
// CHECK:STDOUT:
// CHECK:STDOUT: impl fn @I();
// CHECK:STDOUT:
// CHECK:STDOUT: abstract fn @J();
// CHECK:STDOUT:
// CHECK:STDOUT: virtual fn @K();
// CHECK:STDOUT:
// CHECK:STDOUT: impl fn @L();
// CHECK:STDOUT:
20 changes: 19 additions & 1 deletion toolchain/sem_ir/formatter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,25 @@ class FormatterImpl {
// Formats a full function.
auto FormatFunction(FunctionId id) -> void {
const Function& fn = sem_ir_.functions().Get(id);
FormatEntityStart(fn.is_extern ? "extern fn" : "fn", fn.generic_id, id);
std::string function_start;
switch (fn.virtual_modifier) {
case FunctionFields::VirtualModifier::Virtual:
function_start += "virtual ";
break;
case FunctionFields::VirtualModifier::Abstract:
function_start += "abstract ";
break;
case FunctionFields::VirtualModifier::Impl:
function_start += "impl ";
break;
case FunctionFields::VirtualModifier::None:
break;
}
if (fn.is_extern) {
function_start += "extern ";
}
function_start += "fn";
FormatEntityStart(function_start, fn.generic_id, id);

llvm::SaveAndRestore function_scope(scope_, inst_namer_->GetScopeFor(id));

Expand Down
7 changes: 7 additions & 0 deletions toolchain/sem_ir/function.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ namespace Carbon::SemIR {

// Function-specific fields.
struct FunctionFields {
// Kinds of virtual modifiers that can apply to functions.
enum class VirtualModifier { None, Virtual, Abstract, Impl };

// The following members always have values, and do not change throughout the
// lifetime of the function.

Expand All @@ -23,6 +26,10 @@ struct FunctionFields {
// always present if the function has a declared return type.
InstId return_storage_id;

// Which, if any, virtual modifier (virtual, abstract, or impl) is applied to
// this function.
VirtualModifier virtual_modifier;

// The following member is set on the first call to the function, or at the
// point where the function is defined.

Expand Down

0 comments on commit 5806d83

Please sign in to comment.