From ae58fa01e7b86e25c0ab39f5953bd252a34fab16 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Tue, 27 Oct 2020 14:43:03 +0800 Subject: [PATCH 1/5] allow class/module nodes to declare generics --- src/bindgen/graph/path.cr | 41 +++++++++++++++++++-------- src/bindgen/processor/sanity_check.cr | 12 ++++++++ 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/src/bindgen/graph/path.cr b/src/bindgen/graph/path.cr index 6306784..fc105ea 100644 --- a/src/bindgen/graph/path.cr +++ b/src/bindgen/graph/path.cr @@ -100,6 +100,11 @@ module Bindgen end end + # Returns a copy of this path with every part CamelCased. + def camelcase + Path.new(@parts.map(&.camelcase), global?) + end + # Gives the path as Crystal constants look-up path. def to_s(io) io << "::" if global? @@ -115,14 +120,16 @@ module Bindgen end end - # Returns a new `Path` on *path*. Supports generic parts. - def self.from(path : String) : Path + # Returns a new `Path` on *path*. All generic type arguments are removed, + # unless *generic* is true. + def self.from(path : String, *, generic : Bool = false) : Path if path == "" self_path elsif path == "::" global_root else - parts = path.gsub(Util::BALANCED_PARENS_RX, "").split("::") + path = remove_generics(path) unless generic + parts = path.split("::") if global = parts.first?.try(&.empty?) parts.shift end @@ -132,22 +139,27 @@ module Bindgen end # :ditto: - def self.from(path : Path) : Path - new(path.parts.dup, path.global?) + def self.from(path : Path, *, generic : Bool = false) : Path + if generic + new(path.parts.dup, path.global?) + else + new(path.parts.map(&->remove_generics(String)), path.global?) + end end - # Returns a new `Path` formed by concatenating the given paths. An empty - # collection produces a self-path. - def self.from(paths : Enumerable(String | Path)) : Path + # Returns a new `Path` formed by concatenating the given *paths*. Each + # path in the collection may correspond to more than one namespace level; + # an empty collection produces a self-path. + def self.from(paths : Enumerable(String | Path), *, generic : Bool = false) : Path paths.reduce(self_path) do |path, other| - path.join!(other.is_a?(Path) ? other : from(other)) + path.join!(from(other, generic: generic)) end end # :ditto: - def self.from(first_path : String | Path, *remaining : String | Path) : Path - remaining.reduce(from(first_path)) do |path, other| - path.join!(other.is_a?(Path) ? other : from(other)) + def self.from(first_path : String | Path, *remaining : String | Path, generic : Bool = false) : Path + remaining.reduce(from(first_path, generic: generic)) do |path, other| + path.join!(from(other, generic: generic)) end end @@ -266,6 +278,11 @@ module Bindgen nil # Not found. end + # Removes the generic types from the given *path*. + private def self.remove_generics(path) + path.gsub(Util::BALANCED_PARENS_RX, "") + end + # Finds the last common element in the lists *a* and *b*, and returns it. private def self.last_common(a, b) found = nil diff --git a/src/bindgen/processor/sanity_check.cr b/src/bindgen/processor/sanity_check.cr index 10342aa..95eefec 100644 --- a/src/bindgen/processor/sanity_check.cr +++ b/src/bindgen/processor/sanity_check.cr @@ -27,6 +27,9 @@ module Bindgen # Regular expression for a camel-cased symbol CAMEL_CASE_RX = /^[A-Z_][A-Za-z0-9_]*$/ + # Regular expression for a type declaration, possibly generic + TYPE_DECL_RX = /^[A-Z_][A-Za-z0-9_]*(?:\(\*?[A-Z_][A-Za-z0-9_]*(?:, \*?[A-Z_][A-Za-z0-9_]*)*\))?$/ + # Regular expression describing a method name METHOD_NAME_RX = /^[a-z_][A-Za-z0-9_]*[?!=]?$/ @@ -205,6 +208,8 @@ module Bindgen private def check_node_name!(node) if node.is_a?(Graph::Constant) check_valid_constant_name!(node) + elsif node.is_a?(Graph::Class) || node.is_a?(Graph::Namespace) + check_valid_type_name!(node) elsif node.constant? check_valid_camel_case_name!(node) else @@ -219,6 +224,13 @@ module Bindgen end end + # Checks if *node* has a valid type name. If not, adds an error. + private def check_valid_type_name!(node) + unless TYPE_DECL_RX.matches?(node.name) + add_error(node, "Invalid #{node.kind_name.downcase} name #{node.name.inspect}") + end + end + # Checks if *node* has a valid constant name. If not, adds an error. private def check_valid_constant_name!(node) unless CONSTANT_RX.matches?(node.name) From 318db57217f61a5b5a1abf2d025430de1bf08610 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Tue, 27 Oct 2020 15:31:51 +0800 Subject: [PATCH 2/5] container modules, pseudo-generic selection --- assets/glue.cr | 23 ++++-- .../call_builder/crystal_container_of.cr | 73 +++++++++++++++++++ src/bindgen/crystal/typename.cr | 17 ++++- src/bindgen/parser/method.cr | 8 +- .../processor/instantiate_containers.cr | 41 ++++++++++- 5 files changed, 151 insertions(+), 11 deletions(-) create mode 100644 src/bindgen/call_builder/crystal_container_of.cr diff --git a/assets/glue.cr b/assets/glue.cr index c35741d..038cd91 100644 --- a/assets/glue.cr +++ b/assets/glue.cr @@ -75,12 +75,11 @@ module BindgenHelper end # Wraps a *list* into a container *wrapper*, if it's not already one. - macro wrap_container(wrapper, list) - %instance = {{ list }} - if %instance.is_a?({{ wrapper }}) - %instance + def self.wrap_container(wrapper : T.class, list) : T forall T + if list.is_a?(T) + list else - {{wrapper}}.new.concat(%instance) + wrapper.new.concat(list) end end @@ -119,5 +118,19 @@ module BindgenHelper to_a.inspect(io) io << ">" end + + # Note: Crystal has no dependent types, so this module must refer to its own + # type parameter `T`. + private module ClassMethods(T) + # Wraps the list of *values* into a container of this type, if it's not + # already one. + def from(values : Enumerable(T)) : self + BindgenHelper.wrap_container(self, values) + end + end + + macro included + extend ClassMethods(T) + end end end diff --git a/src/bindgen/call_builder/crystal_container_of.cr b/src/bindgen/call_builder/crystal_container_of.cr new file mode 100644 index 0000000..bd0693a --- /dev/null +++ b/src/bindgen/call_builder/crystal_container_of.cr @@ -0,0 +1,73 @@ +module Bindgen + module CallBuilder + # Builder for the `.of` macro of container modules. + class CrystalContainerOf + def initialize(@db : TypeDatabase) + end + + def build(method : Parser::Method, container : Configuration::Container) : Call + raise "not a macro" unless method.macro? + + result = Call::Result.new( + type: Parser::Type::EMPTY, + type_name: "", + reference: false, + pointer: 0, + ) + + Call.new( + origin: method, + name: method.name, + arguments: [] of Call::Argument, + result: result, + body: Body.new(@db, container), + ) + end + + class Body < Call::Body + def initialize(@db : TypeDatabase, @container : Configuration::Container) + end + + def to_code(call : Call, _platform : Graph::Platform) : String + %[macro #{call.name}(*type_args)\n] \ + %[#{macro_body}\n] \ + %[end] + end + + private def macro_body + instantiations = @container.instantiations.map do |inst| + inst.map { |t| @db.resolve_aliases(t).full_name } + end.uniq + + if instantiations.empty? + return %[ {% raise "\#{self} has no instantiations" %}] + end + + pass = Crystal::Pass.new(@db) + typer = Crystal::Typename.new(@db) + cpp_typer = Cpp::Typename.new + + branches = instantiations.map_with_index do |inst, i| + type_name = cpp_typer.template_class(@container.class, inst) + templ_type = Parser::Type.parse(type_name) + templ_args = templ_type.template.not_nil!.arguments + + klass_name = "Container_#{templ_type.mangled_name}" + arg_list = templ_args.join(", ") do |t| + typer.full(pass.to_wrapper(t), expects_type: false) + end + + %[ {% #{i == 0 ? "if" : "elsif"} types == {#{arg_list}} %} {{ #{klass_name} }}\n] + end + + module_name = Graph::Path.from(@container.class).last_part.camelcase + + %[ {% types = type_args.map(&.resolve) %}\n] \ + %[#{branches.join}] \ + %[ {% else %} {% raise "#{module_name}(\#{types.splat}) has not been instantiated" %}\n] \ + %[ {% end %}] + end + end + end + end +end diff --git a/src/bindgen/crystal/typename.cr b/src/bindgen/crystal/typename.cr index 0f271ca..1e4a13c 100644 --- a/src/bindgen/crystal/typename.cr +++ b/src/bindgen/crystal/typename.cr @@ -6,13 +6,22 @@ module Bindgen def initialize(@db : TypeDatabase) end - # Returns the full Crystal type-name of *result*. - def full(result : Call::Expression) + # Returns the full Crystal type-name of *result*. If *expects_type* is + # false, the type-name may appear in regular code, which affects how + # pointers are formatted. + def full(result : Call::Expression, *, expects_type = true) ptr = result.pointer ptr += 1 if result.reference - stars = "*" * ptr nilable = "?" if result.nilable? - "#{result.type_name}#{stars}#{nilable}" + + if expects_type + stars = "*" * ptr if ptr > 0 + "#{result.type_name}#{stars}#{nilable}" + else + prefix = "Pointer(" * ptr if ptr > 0 + suffix = ")" * ptr if ptr > 0 + "#{prefix}#{result.type_name}#{suffix}#{nilable}" + end end # The type-name of *type* for use in a wrapper. diff --git a/src/bindgen/parser/method.cr b/src/bindgen/parser/method.cr index 4ff04a4..bb39431 100644 --- a/src/bindgen/parser/method.cr +++ b/src/bindgen/parser/method.cr @@ -25,6 +25,10 @@ module Bindgen # Qt signal Signal + # Crystal macro. Only internally used by Bindgen to indicate that a + # macro is none of the other method types. + Macro + # Is this one of the constructors? def any_constructor? : Bool constructor? || aggregate_constructor? || copy_constructor? @@ -127,7 +131,7 @@ module Bindgen delegate constructor?, aggregate_constructor?, copy_constructor?, any_constructor?, member_method?, member_getter?, member_setter?, static?, static_method?, static_getter?, static_setter?, signal?, - operator?, conversion_operator?, destructor?, to: @type + operator?, conversion_operator?, destructor?, macro?, to: @type delegate public?, protected?, private?, to: @access def_equals_and_hash @type, @name, @class_name, @access, @arguments, @@ -328,6 +332,8 @@ module Bindgen "clone" when .destructor? "finalize" + when .macro? + name else raise "BUG: No #crystal_name implementation for type #{@type}" end diff --git a/src/bindgen/processor/instantiate_containers.cr b/src/bindgen/processor/instantiate_containers.cr index 7ab9120..05d1325 100644 --- a/src/bindgen/processor/instantiate_containers.cr +++ b/src/bindgen/processor/instantiate_containers.cr @@ -18,10 +18,37 @@ module Bindgen root = graph.as(Graph::Container) @config.containers.each do |container| + build_container_module(container, root) instantiate_container(container, root) end end + # Builds the wrapper module under *root* for the given *container*. The + # module is responsible for naming instantiated container wrappers. + private def build_container_module(container, root) + sizes = container.instantiations.map(&.size) + unless sizes.uniq.size == 1 + raise "All instantiations of #{container.class} must have the same number of arguments" + end + arg_count = sizes.first + + formal_args = arg_count == 1 ? %w[T] : Array(String).new(arg_count) { |i| "T#{i + 1}" } + crystal_name = "#{container.class}(#{formal_args.join(", ")})" + path = Graph::Path.from(crystal_name, generic: true).camelcase + + builder = Graph::Builder.new(@db) + parent = builder.get_or_create_path(root, path).as(Graph::Container) + call = CallBuilder::CrystalContainerOf.new(@db) + + macro_method = of_macro(container) + macro_node = Graph::Method.new( + origin: macro_method, + name: macro_method.name, + parent: parent.platform_specific(Graph::Platform::Crystal), + ) + macro_node.calls[Graph::Platform::Crystal] = call.build(macro_method, container) + end + # Instantiates the *container* (Of any type), placing the built classes # into *root*. private def instantiate_container(container, root) @@ -66,6 +93,7 @@ module Bindgen graph = builder.build_class(klass, klass.name, root) graph.set_tag(Graph::Class::FORCE_UNWRAP_VARIABLE_TAG) graph.included_modules << container_module(SEQUENTIAL_MODULE, templ_args) + graph.included_modules << container_module(container.class, templ_args) end # Generates the C++ template name of a container class. @@ -80,7 +108,7 @@ module Bindgen typer = Crystal::Typename.new(@db) args = types.map { |t| typer.full pass.to_wrapper(t) }.join(", ") - "#{kind}(#{args})" + "#{Graph::Path.from(kind).camelcase}(#{args})" end # Adds a `typedef Container Container_T...` for C++. @@ -217,6 +245,17 @@ module Bindgen crystal_name: "size", # `Indexable#size` ) end + + # Builds the `.of` macro of the given *container* module. + private def of_macro(container) + Parser::Method.build( + type: Parser::Method::Type::Macro, + class_name: container.class, + name: "of", + return_type: Parser::Type::EMPTY, + arguments: [] of Parser::Argument, + ) + end end end end From 0ee19c3ab20d30af40a8a8de9a1cbd83c7eeea40 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Tue, 27 Oct 2020 17:02:49 +0800 Subject: [PATCH 3/5] specs for container modules --- SPEC.md | 63 +++++++++++++++++++++++++++++ spec/integration/containers_spec.cr | 5 +++ 2 files changed, 68 insertions(+) diff --git a/SPEC.md b/SPEC.md index 63ba982..889479c 100644 --- a/SPEC.md +++ b/SPEC.md @@ -577,6 +577,12 @@ module Container(T) values.each { |v| self << v } self end + + # Wrapper selection macro + macro of(*type_args) end + + # General conversion method (§2.6.1) + # abstract def self.from(values : Enumerable(T)) : Container(T) end ``` @@ -585,6 +591,63 @@ appear in method argument types or return types; explicit instantiations may be configured with the `containers` section. Aliases to complete container types and container type arguments are both supported. +It is possible to store containers as Crystal instance variables using the +container module. However, the generic module does not name the actual wrapper +type instantiated by Bindgen. To do this, the `.of` macro should be used: + +```crystal +class Foo + # `Container(Int32)` is the module type + # `Container.of(Int32)` produces the concrete, non-generic wrapper type + # corresponding to the `Int32` instantiation + # For all `T`, `Container.of(T) < Container(T)`, and no other types include + # the same instantiated module + @x : Container(Int32) = Container.of(Int32).new + + # Referring to a non-instantiated type is a compilation error + # @x = Container.of(NoReturn).new + + # If the wrapper type is stored in a constant, it can be used in array literal + # initializers + BitArray = Container.of(Bool) + SECRET = BitArray{true, false, false, true} + + # Type restrictions, like instance variables, can only use module types due to + # grammar restrictions + def matches_secret?(bits : Container(Bool)) + bits.to_a == SECRET.to_a + end +end +``` + +### §2.6.1 Automatic container conversion + +Every container wrapper class defines a `.from` class method, which takes any +matching enumerable and converts it into that wrapper type. Enumerables passed +to C++ are automatically converted using the same method. In both cases, the +argument is reused if it is already of the wrapper type. + +```cpp +struct Foo { + static double sum(const Container &values); +}; +``` + +```crystal +class Foo + def self.sum(values : Enumerable(Float64)) : Float64 + # converted = Container.of(Float64).from(values) + # ... + end +end + +Foo.sum([1.2, 3.4]) # => 4.6 +Foo.sum({1.2, 3.4}) # => 4.6 + +# No copies are generated +Foo.sum(Container.of(Float64).from [1.2, 3.4]) # => 4.6 +``` + ## §3. Crystal bindings ### §3.1 Naming scheme diff --git a/spec/integration/containers_spec.cr b/spec/integration/containers_spec.cr index 7dbbfb6..15de617 100644 --- a/spec/integration/containers_spec.cr +++ b/spec/integration/containers_spec.cr @@ -15,6 +15,11 @@ describe "container instantiation feature" do it "works with auto instantiated container (argument)" do list = [1.5, 2.5] Test::Containers.new.sum(list).should eq(4.0) + Test::Containers.new.sum(Test::Std::Vector.of(Float64).from(list)).should eq(4.0) + + list = {3.5, 4.5} + Test::Containers.new.sum(list).should eq(8.0) + Test::Containers.new.sum(Test::Std::Vector.of(Float64).from(list)).should eq(8.0) end it "works with auto instantiated container (aliased container)" do From a01a25f6cfc77cc6c57cf5ee9b32e3299e4e7eb7 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Sun, 15 Nov 2020 01:11:03 +0800 Subject: [PATCH 4/5] use exact container names in generic selection macro --- src/bindgen/call_builder/crystal_container_of.cr | 2 +- src/bindgen/processor/instantiate_containers.cr | 7 ++++--- src/bindgen/type_database.cr | 7 ++++++- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/bindgen/call_builder/crystal_container_of.cr b/src/bindgen/call_builder/crystal_container_of.cr index bd0693a..dbdec83 100644 --- a/src/bindgen/call_builder/crystal_container_of.cr +++ b/src/bindgen/call_builder/crystal_container_of.cr @@ -54,7 +54,7 @@ module Bindgen klass_name = "Container_#{templ_type.mangled_name}" arg_list = templ_args.join(", ") do |t| - typer.full(pass.to_wrapper(t), expects_type: false) + @db.try_or(t, typer.full(pass.to_wrapper(t), expects_type: false), &.container_type) end %[ {% #{i == 0 ? "if" : "elsif"} types == {#{arg_list}} %} {{ #{klass_name} }}\n] diff --git a/src/bindgen/processor/instantiate_containers.cr b/src/bindgen/processor/instantiate_containers.cr index 05d1325..a955d48 100644 --- a/src/bindgen/processor/instantiate_containers.cr +++ b/src/bindgen/processor/instantiate_containers.cr @@ -88,7 +88,7 @@ module Bindgen klass = build_sequential_class(container, templ_type) add_cpp_typedef(root, templ_type, klass.name) - set_sequential_container_type_rules(klass, templ_type) + set_sequential_container_type_rules(container, klass, templ_type) graph = builder.build_class(klass, klass.name, root) graph.set_tag(Graph::Class::FORCE_UNWRAP_VARIABLE_TAG) @@ -132,10 +132,10 @@ module Bindgen ) end - # Updates the rules of the sequential container *klass*, whose + # Updates the rules of the sequential *container* *klass*, whose # instantiated type is *templ_type*. The rules are changed to convert # from and to the binding type. - private def set_sequential_container_type_rules(klass : Parser::Class, templ_type) + private def set_sequential_container_type_rules(container, klass : Parser::Class, templ_type) rules = @db.get_or_add(templ_type.full_name) type_args = templ_type.template.not_nil!.arguments @@ -143,6 +143,7 @@ module Bindgen rules.wrapper_pass_by = TypeDatabase::PassBy::Value rules.binding_type = klass.name rules.crystal_type ||= container_module("Enumerable", type_args) + rules.container_type ||= container_module(container.class, type_args) rules.cpp_type ||= klass.name if rules.to_crystal.no_op? diff --git a/src/bindgen/type_database.cr b/src/bindgen/type_database.cr index 429fd6d..effc3a2 100644 --- a/src/bindgen/type_database.cr +++ b/src/bindgen/type_database.cr @@ -129,6 +129,11 @@ module Bindgen @[YAML::Field(ignore: true)] property graph_node : Graph::Node? + # The name of the Crystal container module this type includes, if any. + # Used by `CrystalContainerOf`. + @[YAML::Field(ignore: true)] + property container_type : String? + def initialize( @crystal_type = nil, @cpp_type = nil, @binding_type = nil, @from_cpp = Template::None.new, @to_cpp = Template::None.new, @converter = nil, @@ -140,7 +145,7 @@ module Bindgen @builtin = false, @ignore_methods = [] of String, @superclass_ignore_methods = Util::FAIL_RX, @instance_variables = InstanceVariableConfig::Collection.new, - @graph_node = nil + @graph_node = nil, @container_type = nil ) end From 10edb0c6393d8c182b99798c863e823addc1bffc Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Sun, 15 Nov 2020 01:24:00 +0800 Subject: [PATCH 5/5] update specs for nested container selection --- SPEC.md | 16 ++++++++++++++++ spec/integration/containers.cpp | 16 ++++++++++++++++ spec/integration/containers_spec.cr | 10 +++++++++- 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/SPEC.md b/SPEC.md index 889479c..f858f3c 100644 --- a/SPEC.md +++ b/SPEC.md @@ -617,6 +617,22 @@ class Foo def matches_secret?(bits : Container(Bool)) bits.to_a == SECRET.to_a end + + # Module types can be nested + def transpose(ary : Container(Container(Float64))) + Container.of(Container(Float64)).new + end + + # One level of nested `Enumerable`s can be automatically wrapped in an array + # literal initializer + FloatMatrix = Container.of(Container(Float32)) + def skew(x : Float32, y : Float32, z : Float32) + FloatMatrix{ + [0_f32, -z, y], + [ z, 0_f32, -x], + [ -y, x, 0_f32], + } + end end ``` diff --git a/spec/integration/containers.cpp b/spec/integration/containers.cpp index db9cd2a..b69b73e 100644 --- a/spec/integration/containers.cpp +++ b/spec/integration/containers.cpp @@ -35,4 +35,20 @@ class Containers { return d; } + + std::vector> transpose(const std::vector> &mat) { + std::size_t height = mat.size(); + std::size_t width = mat[0].size(); + + std::vector> trsp; + for (std::size_t x = 0; x < width; ++x) { + std::vector row; + for (std::size_t y = 0; y < height; ++y) { + row.push_back(mat[y][x]); + } + trsp.push_back(std::move(row)); + } + + return trsp; + } }; diff --git a/spec/integration/containers_spec.cr b/spec/integration/containers_spec.cr index 15de617..03c63de 100644 --- a/spec/integration/containers_spec.cr +++ b/spec/integration/containers_spec.cr @@ -3,6 +3,8 @@ require "./spec_helper" describe "container instantiation feature" do it "works" do build_and_run("containers") do + FloatMatrix = Test::Std::Vector.of(Test::Std::Vector(Float64)) + context "sequential container" do it "works with explicitly instantiated container" do Test::Containers.new.integers.to_a.should eq([1, 2, 3]) @@ -31,7 +33,13 @@ describe "container instantiation feature" do end it "works with nested containers" do - Test::Containers.new.grid.to_a.map(&.to_a).should eq([[1, 4], [9, 16]]) + Test::Containers.new.grid.map(&.to_a).should eq([[1, 4], [9, 16]]) + + mat = FloatMatrix{ + [1.2, 3.4], + [5.6, 7.8], + } + Test::Containers.new.transpose(mat).map(&.to_a).should eq([[1.2, 5.6], [3.4, 7.8]]) end end end