Fix super call to mixins in named constructors. (#1956)

src/compiler/ir.h

@@ -356,12 +356,12 @@ class Class : public Node {
   /// The unnamed constructors.
   ///
   /// The named constructors are stored in the [statics] scope.
-  List<Method*> constructors() const { return constructors_; }
-  void set_constructors(List<Method*> constructors) {
+  List<Method*> unnamed_constructors() const { return constructors_; }
+  void set_unnamed_constructors(List<Method*> constructors) {
     ASSERT(constructors_.is_empty());
     constructors_ = constructors;
   }
-  void replace_constructors(List<Method*> new_constructors) { constructors_ = new_constructors; }
+  void replace_unnamed_constructors(List<Method*> new_constructors) { constructors_ = new_constructors; }
 
   /// The unnamed factories.
   ///

src/compiler/lambda.cc

@@ -187,7 +187,7 @@ class BoxVisitor : public ir::ReplacingVisitor {
 
 void add_lambda_boxes(ir::Program* program) {
   ir::Class* box = program->lambda_box();
-  ir::Constructor* constructor = box->constructors()[0]->as_Constructor();
+  ir::Constructor* constructor = box->unnamed_constructors()[0]->as_Constructor();
   ASSERT(constructor != null);
   ir::Field* field = box->fields()[0];
   BoxVisitor visitor(constructor, field);

src/compiler/lsp/completion.cc

@@ -220,7 +220,7 @@ void CompletionHandler::call_class(ast::Dot* node,
                                    IterableScope* scope) {
   bool has_default_constructor = false;
   CallShape default_constructor_shape(1);  // 1 argument for `this`.
-  for (auto constructor : klass->constructors()) {
+  for (auto constructor : klass->unnamed_constructors()) {
     if (constructor->resolution_shape().accepts(default_constructor_shape)) {
       has_default_constructor = true;
       break;

src/compiler/lsp/protocol_summary.cc

@@ -421,7 +421,7 @@ void Writer::print_class(ir::Class* klass) {
   print_list(klass->interfaces(), &Writer::print_toplevel_ref);
   print_list(klass->mixins(), &Writer::print_toplevel_ref);
   print_list(klass->statics()->nodes(), &Writer::print_method);
-  print_list(klass->constructors(), &Writer::print_method);
+  print_list(klass->unnamed_constructors(), &Writer::print_method);
   print_list(klass->factories(), &Writer::print_method);
   print_list(klass->fields(), &Writer::print_field);
   print_list(klass->methods(), &Writer::print_method);
@@ -560,7 +560,7 @@ class ToitdocPathMappingCreator {
       visit_container(ToitdocPath::Kind::GLOBAL, module, null, module->globals());
       for (auto klass : module->classes()) {
         visit_container(ToitdocPath::Kind::STATIC_METHOD, module, klass, klass->statics()->nodes());
-        visit_container(ToitdocPath::Kind::CONSTRUCTOR, module, klass, klass->constructors());
+        visit_container(ToitdocPath::Kind::CONSTRUCTOR, module, klass, klass->unnamed_constructors());
         visit_container(ToitdocPath::Kind::FACTORY, module, klass, klass->factories());
         visit_container(ToitdocPath::Kind::FIELD, module, klass, klass->fields());
         visit_container(ToitdocPath::Kind::METHOD, module, klass, klass->methods());

src/compiler/mixin.cc

@@ -19,6 +19,7 @@
 #include "mixin.h"
 #include "set.h"
 #include "shape.h"
+#include "resolver_scope.h"
 
 namespace toit {
 namespace compiler {
@@ -232,9 +233,9 @@ class MixinConstructorVisitor : protected SuperCallVisitor {
 /// Instead of doing a super call, it calls the block with the values for
 /// the fields.
 static void modify_mixin_constructor(ir::Class* mixin) {
-  ASSERT(mixin->constructors().length() == 1);  // A single default constructor.
-  ASSERT(mixin->constructors()[0]->parameters().length() == 1);  // The object but no other parameter.
-  auto constructor = mixin->constructors()[0];
+  ASSERT(mixin->unnamed_constructors().length() == 1);  // A single default constructor.
+  ASSERT(mixin->unnamed_constructors()[0]->parameters().length() == 1);  // The object but no other parameter.
+  auto constructor = mixin->unnamed_constructors()[0];
   MixinConstructorVisitor visitor(mixin, mixin->fields());
   visitor.insert_mixin_block_calls(constructor);
   ASSERT(visitor.has_seen_static_super());
@@ -526,8 +527,8 @@ class ConstructorVisitor : protected SuperCallVisitor {
       // Blocks must be inside locals so that they can be referenced with `ReferenceBlock`.
       auto block = _new ir::Block(name, range);
       auto block_assig = _new ir::AssignmentDefine(block, block_code, range);
-      ASSERT(mixin->constructors().length() == 1);
-      auto constructor = mixin->constructors()[0];
+      ASSERT(mixin->unnamed_constructors().length() == 1);
+      auto constructor = mixin->unnamed_constructors()[0];
       auto constructor_ref = _new ir::ReferenceMethod(constructor, range);
       auto arguments = ListBuilder<ir::Expression*>::allocate(2);
       auto outer_this = i == 0
@@ -605,9 +606,17 @@ class ConstructorVisitor : protected SuperCallVisitor {
 /// Changes super calls so that they call mixin constructors as well.
 void adjust_super_calls(ir::Class* klass, Map<ir::Field*, ir::Field*> field_map) {
   ConstructorVisitor visitor(klass, field_map);
-  for (auto constructor : klass->constructors()) {
+  for (auto constructor : klass->unnamed_constructors()) {
     visitor.visit(constructor);
   }
+  for (auto node : klass->statics()->nodes()) {
+    if (node->is_Method()) {
+      auto method = node->as_Method();
+      if (method->is_constructor()) {
+        visitor.visit(method);
+      }
+    }
+  };
 }
 
 void apply_mixins(ir::Program* program) {

src/compiler/optimizations/optimizations.cc

@@ -177,7 +177,7 @@ void optimize(Program* program, TypeOracle* oracle) {
     // We need to handle constructors (named and unnamed) here, as we use a
     //   different visitor, than for the globals.
     // Unnamed constructors:
-    for (auto constructor : klass->constructors()) {
+    for (auto constructor : klass->unnamed_constructors()) {
       visitor.visit(constructor);
     }
     // Named constructors are mixed together with the other static entries.

src/compiler/resolver.cc

@@ -140,7 +140,7 @@ ir::Program* Resolver::resolve(const std::vector<ast::Unit*>& units,
     all_classes.add(module->classes());
     all_methods.add(module->methods());
     for (auto klass : module->classes()) {
-      all_methods.add(klass->constructors());
+      all_methods.add(klass->unnamed_constructors());
       all_methods.add(klass->factories());
       for (auto node : klass->statics()->nodes()) {
         if (node->is_Global()) {
@@ -567,7 +567,7 @@ void Resolver::check_clashing_or_conflicting(std::vector<Module*> modules) {
     }
 
     for (auto klass : module->classes()) {
-      auto constructors = klass->constructors();
+      auto constructors = klass->unnamed_constructors();
       auto factories = klass->factories();
       auto unnamed_factories_and_constructors =
           ListBuilder<ir::Node*>::allocate(constructors.length() + factories.length());
@@ -1830,7 +1830,7 @@ void Resolver::fill_classes_with_skeletons(std::vector<Module*> modules) {
         constructors.add(constructor);
       }
 
-      ir_class->set_constructors(constructors.build());
+      ir_class->set_unnamed_constructors(constructors.build());
       ir_class->set_factories(factories.build());
       ir_class->set_methods(methods.build());
       ir_class->set_fields(fields.build());
@@ -2500,7 +2500,7 @@ void Resolver::resolve_fill_class(ir::Class* klass,
     resolve_field(field, klass, &class_scope, entry_module, core_module);
   }
   // Resolve the methods.
-  for (auto constructor : klass->constructors()) {
+  for (auto constructor : klass->unnamed_constructors()) {
     resolve_fill_method(constructor, klass, &class_scope, entry_module, core_module);
   }
   for (auto factory : klass->factories()) {

src/compiler/resolver_method.cc

@@ -2057,7 +2057,7 @@ List<ir::Node*> MethodResolver::_compute_constructor_super_candidates(ast::Node*
   auto super = constructor->klass()->super();
   if (is_literal_super(target_node)) {
     ListBuilder<ir::Node*> candidates;
-    for (auto super_constructor : super->constructors()) {
+    for (auto super_constructor : super->unnamed_constructors()) {
       candidates.add(super_constructor);
     }
     return candidates.build();
@@ -2197,7 +2197,7 @@ MethodResolver::Candidates MethodResolver::_compute_target_candidates(ast::Node*
         continue;
       }
       klass = candidate->as_Class();
-      for (auto constructor : klass->constructors()) candidates_builder.add(constructor);
+      for (auto constructor : klass->unnamed_constructors()) candidates_builder.add(constructor);
       for (auto factory : klass->factories()) candidates_builder.add(factory);
     }
     starting_index = 0;

src/compiler/resolver_toitdoc.cc

@@ -111,7 +111,7 @@ class PrefixIterator : public ToitdocScopeIterator {
 static void ensure_has_toitdoc_scope(ir::Class* klass) {
   if (klass->toitdoc_scope() != null) return;
   ScopeFiller filler;
-  filler.add_all(klass->constructors());
+  filler.add_all(klass->unnamed_constructors());
   filler.add_all(klass->factories());
   filler.add_all(klass->methods());
   filler.add_all(klass->fields());

src/compiler/tree.cc

@@ -672,8 +672,8 @@ static void shake(Program* program,
   for (auto klass : program->classes()) {
     // Note that we already shook the copies of constructors/factories/statics that had
     //   been copied into program->methods.
-    auto remaining_constructors = shake_methods(klass->constructors(), grown_methods);
-    klass->replace_constructors(remaining_constructors);
+    auto remaining_constructors = shake_methods(klass->unnamed_constructors(), grown_methods);
+    klass->replace_unnamed_constructors(remaining_constructors);
     auto remaining_factories = shake_methods(klass->factories(), grown_methods);
     klass->replace_factories(remaining_factories);
     klass->statics()->invalidate_resolution_map();

tests/mixin-field3-test.toit

@@ -0,0 +1,42 @@
+// Copyright (C) 2023 Toitware ApS.
+// Use of this source code is governed by a Zero-Clause BSD license that can
+// be found in the tests/LICENSE file.
+
+import expect show *
+
+foo [block]:
+  block.call
+
+mixin MixA:
+  field/int? := null
+  field2/int := 499
+
+  constructor:
+    field = 42
+
+mixin MixB extends MixA:
+  foo:
+    return field
+
+  bar:
+    return field2
+
+class ClassA extends Object with MixA:
+
+class ClassB extends Object with MixB:
+  b-field1 := "a"
+  b-field2 := "b"
+
+main:
+  a := ClassA
+  expect-equals 42 a.field
+  expect-equals 499 a.field2
+
+  b := ClassB
+  expect-equals 42 b.field
+  expect-equals 499 b.field2
+  expect-equals "a" b.b-field1
+  expect-equals "b" b.b-field2
+
+  expect-equals 42 b.foo
+  expect-equals 499 b.bar

tests/mixin-super5-test.toit

@@ -0,0 +1,34 @@
+// Copyright (C) 2023 Toitware ApS.
+// Use of this source code is governed by a Zero-Clause BSD license that can
+// be found in the tests/LICENSE file.
+
+import expect show *
+
+events := []
+
+mixin Mix1:
+  constructor:
+    events.add "Mix1"
+
+class A extends Object with Mix1:
+
+class B extends Object with Mix1:
+  constructor:
+    events.add "ClassB"
+
+class C extends Object with Mix1:
+  field := 499
+
+main:
+  a := A
+  expect-equals ["Mix1"] events
+  events.clear
+
+  b := B
+  expect-equals ["ClassB", "Mix1"] events
+  events.clear
+
+  c := C
+  expect-equals ["Mix1"] events
+  events.clear
+  expect-equals 499 c.field

tests/mixin-super6-test.toit

@@ -0,0 +1,37 @@
+// Copyright (C) 2023 Toitware ApS.
+// Use of this source code is governed by a Zero-Clause BSD license that can
+// be found in the tests/LICENSE file.
+
+import expect show *
+
+events := []
+
+mixin Mix1:
+  bool-field := false
+  int-field := 499
+
+mixin Mix2 extends Mix1:
+
+class A extends Object with Mix2:
+
+class B extends Object with Mix2:
+  constructor:
+    events.add "ClassB"
+
+class C extends Object with Mix2:
+  field := 499
+
+main:
+  a := A
+  expect-equals false a.bool-field
+  expect-equals 499 a.int-field
+
+  b := B
+  expect-equals ["ClassB"] events
+  expect-equals false b.bool-field
+  expect-equals 499 b.int-field
+
+  c := C
+  expect-equals false c.bool-field
+  expect-equals 499 c.int-field
+  expect-equals 499 c.field

tests/mixin-super7-test.toit

@@ -0,0 +1,15 @@
+// Copyright (C) 2023 Toitware ApS.
+// Use of this source code is governed by a Zero-Clause BSD license that can
+// be found in the tests/LICENSE file.
+
+import expect show *
+
+mixin Mix1:
+  field/int := 499
+
+class A extends Object with Mix1:
+  constructor.named:
+
+main:
+  a := A.named
+  expect-equals 499 a.field