-
Notifications
You must be signed in to change notification settings - Fork 18
Generic pseudo-instantiation macro for container wrapper types #102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
In general, I really like these macros. They're much nicer to use than those mumbo-jumbo generated ones! However: def f(pen : Qt::Pen)
# before
pen.dash_pattern = [3.0, 1.5, 1.0, 0.5]
# after
pen.dash_pattern = Qt::QVector.of(Float64).from [3.0, 1.5, 1.0, 0.5]
endI think the 'after' usage is too complicated. Bindgen strives to produce code that's nice to use to the lib-user (At the expense of ease-of-use as configuration goes at times). |
|
Understandable. That said, the breaking change only happens when passing containers to the wrapper; the wrapper should be free to return containers directly, i.e. class Pen
# before
def dash_pattern : Enumerable(Float64)
# after
def dash_pattern : QVector(Float64)
endThis might be sufficient to fix the second issue. For it not to break existing code |
|
module QVector(T)
macro of(*type_args)
{% types = type_args.map(&.resolve) %}
{% if types == {UInt32} %} {{ Container_QVector_unsigned_int_ }}
{% elsif types == {Point} %} {{ Container_QVector_QPoint_ }}
{% elsif types == {PointF} %} {{ Container_QVector_QPointF_ }}
{% elsif types == {Float64} %} {{ Container_QVector_double_ }}
{% elsif types == {TextLength} %} {{ Container_QVector_QTextLength_ }}
{% elsif types == {TextFormat} %} {{ Container_QVector_QTextFormat_ }}
{% elsif types == {LineF} %} {{ Container_QVector_QLineF_ }}
{% elsif types == {Line} %} {{ Container_QVector_QLine_ }}
{% elsif types == {RectF} %} {{ Container_QVector_QRectF_ }}
{% elsif types == {Rect} %} {{ Container_QVector_QRect_ }}
{% else %} {% raise "QVector(#{types.splat}) has not been instantiated" %}
{% end %}
end
end |
38e8449 to
04c1409
Compare
04c1409 to
0ee19c3
Compare
|
Rebased on top of master. Also there is now special logic to prefer container module names ( Please take a look and see if anything still needs to be changed. |
Couldn't this be done if each specialization would include these modules by themselves? |
|
More specifically it could break code that depends on that return type for type deduction, e.g. instance variables: https://play.crystal-lang.org/#/r/9z1p If |
|
Mh, this works fine in the playground: https://play.crystal-lang.org/#/r/9zap |
Currently, all instantiated containers must be referred to using their mangled names. This PR provides two ways to name those wrapper types:
Container(T): A module which contains no code. It is included by and only by the actual wrapper type with the same type arguments. It can be used as type restrictions.Container.of(T): A macro expression that produces the concrete, non-generic wrapper type. It can be used to construct containers in that wrapper type.In Qt5.cr only
QListandQVectorhave been wrapped so far. The.ofmacro for the latter looks like this:For the special case of sequential containers, these wrappers also gain a
.fromconstructor, which does exactly the same thing asBindgenHelper.wrap_container, except again no mangled names are used (see below for an example). The.ofmechanism is independent of sequential containers.There are at least two issues that need to be addressed in future PRs:
QRgbis an alias ofunsigned int, thenQVector<QRgb>is also an alias ofQVector<unsigned int>. So far the container processors do not consider these aliases, so they end up instantiating both vector types. (qrealshould be turned into an alias as well.) Thus, alias detection should be extended to template arguments.QVector<QVector<int>>can only be obtained withQVector.of(Enumerable(Int32)), not withQVector.of(QVector.of(Int32)), due to the special passing rules of sequential containers. This expression becomes ambiguous ifQVector<QList<int>>is also instantiated. Now that container types can be named by the user and that the.fromconstructor makesEnumerableconversion explicit, I suggest removing those passing rules after this PR, so Crystal arrays must be converted at the call site: (this removal would be a breaking change, so it needs to be accompanied by a version bump)