Contributing guidelines, code style

[ratijas]

I feel the need for contributing guidelines and code style. Code style should cover both formatting rules (and tools used for automation) and general patterns for hand-written bindings.

For example, I like how it is done in in winapi crate: https://github.com/retep998/winapi-rs/blob/0.3/CONTRIBUTING.md

Things to consider, based on the articles about previous attempts to generate C++ bindings for rust:

• Method overload renaming rules

  For example, in Qt documentation they are differentiated by suffix number rather than by signature.

  • Constructor overloads:

    • single-argument constructors are expressed using From trait.
    • multiple-argument constructors wrapped in dedicated factory function with some sensible name like impl AB { fn from_a_b(a: A, b: B) -> Self {...} } — what is the naming pattern for them?

  • Method overloads:

    • In Rust, method overloading trait-based.
      It works best on single-argument overloads, while multiple arguments may be stacked in tuple.
      For example, see ToSocketAddrs.

• Operators overload
  Probably, the easiest part, since in Rust they are generic and single-argumented.

• Default arguments
  In C++ omitted default arguments are substituted at compile time with what was defined in header file. It can be mapped to Options in Rust. Then, for each None argument, a separate cpp! macro invocation would need to be done to retrieve the corresponding default value. Finally, call C++ function with all arguments supplied.

• C-style enums with multiple variants mapping to same values. Sounds like the right job for a bitflags-style macro.
  Or just don't use enum at all — that's what winapi is doing: every define becomes just a const.
  Then again, it may be a place for two-layered system, with low-level raw bindings and high-level rusty ones.

• Short-circuiting on Rust side
  This one is about re-implementing basic things like QRect in native Rust rather than calling into Qt.

  • Is it acceptable?
  • If so, what are the rules?

• Imitating inheritance?

• Feature-flagging classes and methods added in Qt x.y

  • Chain features such that e.g. qt_5_12 enables qt_5_11 and so on.
    Even though current 5.15 is the last in Qt5 branch, feature-flagging still would apply in Qt6.

• Licensing and copyright management rules

What else?

[ogoffart]

My goal is to make a nice and "idomatic" rust API

in Qt documentation they are differentiated by suffix number rather than by signature.

I would not consider number as an atractive way to dinstinguish overload. I think either a name base on the type. Or perhaps taking a trait that can convert from the different expected parameters.

Default arguments

Either an option, or no default, or a different overload.

C-style enums with multiple variants mapping to same values

Again it depends, sometimes this is not an enum at all but just some constant, then some const in a module will do. Sometimes it is just an alias because the name is deprecated: Then don't use the deprecated name.
Other solution may be to use different value but mask the higher bits.

Short-circuiting on Rust side

For small types like QRect i think it is acceptable. I'd say type whose member are public.

What else?

I must admit that this was one of my first rust project and i did not have much experience, so there is some things that are fundamentally wrong. For example the way we accept to take a &mut self for the callback (see #11 )
Also the whole pinning was done before Pin was in rust and it should be re-done.

[ratijas]

I would not consider number as an atractive way to dinstinguish overload.

Numbers are arguably the worst way to overload, but for brainstorming step it would do. I like the whole idea of PEPs, RFCs etc. where people write down their thoughts and all considered options — even the worst ones. Just so that other people who come later would see that such option was considered and rejected/accepted for a reason.

Or perhaps taking a trait that can convert from the different expected parameters.

If it is going to trait-based, then how about dynamic calls? Like having two methods:

• fn very_overloaded<T: VeryOverloaded>(arg: T) -> T::R;
• fn very_overloaded_dyn(arg: Box<dyn VeryOverloaded>) -> ???

I don't feel like it would well work with return types.

sometimes this is not an enum at all but just some constant, then some const in a module will do.

So, defining a separate module for each such 'enum' then.

I'd say type whose member are public.

Even for QRect there are getters and setters, x/y are not a public members. Could you elaborate on that, please?

Also the whole pinning was done before Pin was in rust and it should be re-done.

Pinning definitely should be considered as high priority. I'm not even sure whether it would make sense to write more wrappers until then.

---

Now what about documentation and links to 'upstream' Qt docs?

Writing manually is quite repetitive and error-prone. It may be a job for a macro, but I'm not sure declarative rust macro is able to work on sub-string level with doc comments. Generally, I'd like to see the pattern similar to what I introduced in the 9397478 commit.

Also: wrapper vs bindings. I don't think they are the same, albeit not sure what's the difference. Exact terminology use cases should be stated explicitly.

[ogoffart]

Regarding overload, i think the best is to find good names with the name of function they do. Depending on the function, other methodology can be used.

I'd say type whose member are public.

Even for QRect there are getters and setters, x/y are not a public members. Could you elaborate on that, please?

Ah, you are right.
Then i'll reformulate:
For small structure whose access would be inline in C++, we can also try to get it "inline" in rust by replicating the same field when it make sense. (so no d_ptr, but just plain field)
If the field can be accessed with inline method, changing them would break the ABI, so we can rely on them being there.
If the rust structure is layout-compatible, casting is well defined https://en.cppreference.com/w/cpp/language/data_members#Standard_layout

Writing manually is quite repetitive and error-prone. It may be a job for a macro, but I'm not sure declarative rust macro is able to work on sub-string level with doc comments.

I believe it is:

something like

#[qt_doc_comment]
/// Wrapper around [`QByteArray`][qt:QByteArray] class
struct QByteArray ...

The procedural macro can just replace the doc comments with one that does some simple search and replace to add the link.
Is that what you mean?

[ratijas]

A note to consider:
If overloaded methods will end up renamed, then it is extremely important to set up documentation pattern such that it contains original method's name and signature. Ideally, copy-paste full signature, so than it could be easily grepped for.

d_ptr

I seriously need to research more about Qt internals. Reading log for me: https://wiki.qt.io/D-Pointer

#[qt_doc_comment]

Genius! Seriously, it's almost perfect. Let me extrapolate the idea:

#[qt_doc_comment] for any occasion

// class
#[qt_doc_comment(qt = "qbytearray.html", cls = "QByteArray")]
// ctor
#[qt_doc_comment(qt = "qbytearray.html#QByteArray-2", ctor = "QByteArray(int size, char ch)")]
// method
#[qt_doc_comment(qt = "qbytearray.html#append-2", method = "QByteArray &QByteArray::append(int count, char ch)")]
// function
#[qt_doc_comment(qt = "qbytearray.html#append-2", method = "QByteArray &QByteArray::append(int count, char ch)")]
// signal
#[qt_doc_comment(qt = "qcompleter.html#activated-1", signal = "void QCompleter::activated(const QModelIndex &index)")]
// slot
#[qt_doc_comment(qt = "qcompleter.html#complete", slot = "void QCompleter::complete(const QRect &rect = QRect())")]
// operator
#[qt_doc_comment(qt = "qbytearray.html#operator-5b-5d", op = "QByteRef QByteArray::operator[](int i)")]
#[qt_doc_comment(qt = "qbytearray.html#operator-5b-5d-1", op = "char QByteArray::operator[](int i) const")]
// Static Public Members
#[qt_doc_comment(qt = "qbytearray.html#fromHex", member = "QByteArray QByteArray::fromHex(const QByteArray &hexEncoded)")]
// Related Non-Members
#[qt_doc_comment(qt = "qbytearray.html#qstrnlen", related = "uint qstrnlen(const char *str, uint maxlen)")]
// typedef
#[qt_doc_comment(qt = "qbytearray.html#const_iterator-typedef", typedef = "QByteArray::const_iterator")]

This tool may be useful to parse such attribute macros: https://github.com/TedDriggs/darling

As can be seen, with this macro doc comment is not required at all. However, should it be needed, it can also be written, in which case proc macro handler should append header # Wrapper-specific implementation followed by the content of provided docstring.

Type of an item is determined from keyword argument (other than qt = ). Only one of them must be provided.

The link is generated by prepending https://doc.qt.io/qt-5/ prefix.

TODO: what about future-proof versioning? After all, some day Qt6 will become a thing.

Hidden default arguments

Argument's default value may be hidden from Qt docs. What to do in that case?

For example: https://doc.qt.io/qt-5/qstring.html#section

QString QString::section(QChar sep, int start, int end = ..., QString::SectionFlags flags = SectionDefault) const

Related Non-Members

How they are defined and do they belong to? Could it happened that one such entity is listed under two different classes / namespaces? If so, then where to implement its wrapper?

[ogoffart]

I think maybe i should be explicit that i do not have the vision to expose the full Qt API.
A lot of things are redundant with the Rust API. And the core class should only expose the API that allow to interoperate with QML.
For example, we wouldn't want to export section, we would recommend to use the rust idom: something like

my_qsting.to_slice().split('*').split(2).take(3)

TODO: what about future-proof versioning? After all, some day Qt6 will become a thing.

Let's see about Qt6 when it will be there. Can be an env variable or a feature detected by the build script.

Hidden default arguments

If we wanted to expose section, we would make the last parameter mendatory, or have section and section_with_flags

Related Non-Members

They could just be normal function in the module, or associated function to the struct.

[ratijas]

i do not have the vision to expose the full Qt API.

That's completely understandable. Qt is huge, and porting it is not something I could've done in a span of a year.

But nerdy people like me will be passing by from time to time, and the better guideline there will be — the more chances that they will make meaningful contribution. And from the maintainers' point of view, they better have damn strict guidelines and rich infrastructure like #[qt_doc_comment] to keep the code base consistent.

I'd say, commonly used fundamental classes should have nice bindings for most (if not all) their members. Forget the rest. QCompleter from the doc string example above was just a randomly chosen class which apparently had slots and signals, nothing more.

Let's see about Qt6 when it will be there.

Ok. At that point, doc strings probably would be the least of all troubles anyway.

But hey, they are damn serious about upcoming stuff: https://www.qt.io/blog/new-qml-language-features-in-qt-5.15

They could just be normal function in the module

So, do they appear on the documentation page of a class just because they were defined in the same physical file? (I shouldn't be asking, it's too easy to google up)

[ratijas]

Update: added bullet-points about feature-flagging and licensing to the top post.

[ratijas]

All mentions of C++ types in cpp! macro and other places should follow Qt conventions: put a space in front of stars * and ampersands &. For example, take a look at QObject#findChildren method.

const QString &name = QString(), ...
QList<QWidget *> widgets = parentWidget.findChildren<QWidget *>("widgetname");
const QMetaObject *QObject::metaObject() const;

[ratijas]

unsafe blocks should wrap as little as possible, in fact they only necessary around calls to unsafe functions and cpp! macros invocations.

Whenever cpp! is the only unsafe thing in the block, place unsafe marker inside the macro as following: cpp!(unsafe [] -> () {...}):

pub fn to_number(&self) -> f64 {
    cpp!(unsafe [self as "const QJSValue *"] -> f64 as "double" {
        return self->toNumber();
    })
}

But if the are unsafe pre- or post-processing of arguments and/or return values, then wrap it all inside a standard unsafe { } block as follow:

// Unsafe dereference of a raw pointer
unsafe {
    &*cpp!([]-> *const QObjectDescriptor as "RustQObjectDescriptor const *" {
        return RustQObjectDescriptor::instance<Rust_QQmlExtensionPlugin>();
    })
}

// Call to unsafe function CStr::from_ptr
unsafe {
    let x = cpp!([self as "QMessageLogContext *"] -> *const c_char as "const char *" {
        return self->function;
    });
    if x.is_null() {
        return "";
    }
    std::ffi::CStr::from_ptr(x).to_str().unwrap()
}