1613176 - Add a lint to avoid passing types with destructors through the FFI boundaries.

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, task)

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Passing these types behind a pointer or reference type should be OK, but passing them by-value may act poorly, due to differences in destructor semantics between C++ and Rust around function boundaries. The behaviour of this lint would want to be similar to the non_param annotation, but probably only impacting methods with an extern "C" linkage.

It's unclear whether we can make this completely implicit, based on linkage and trivial destructors, or whether we'd need to annotate cross-language methods or cross-language types due to false-positives.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

yeah, I'll try to give a shot to something like this, implicit first and then we'll see.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1613176 - Prove to the incoming clang plugin that our templates are indeed trivially copiable. r=boris,#style

Otherwise it'd error, which is not great. We could add a custom attribute in
there if needed or something, or fix cbindgen to generate these for us as
described in the issue linked from the comments.

Do this in ServoStyleConstsInlines as that's where we have the guarantee that
the templates have been defined, and that the file is included every time
ServoStyleConsts is included.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1613176 - Introduce NonTrivialTypeInFfiChecker. r=andi

This ensures that we don't pass non-trivially-copiable types through the FFI
boundary.

Depends on D61625

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1613176 - Enable -Wreturn-type-c-linkage. r=#build

JSAPI functions are not extern "C" anymore. This warning has a couple false
positives due to template specialization (see first patch of this bug), but we
were already working around them, because apparently some mingw builds use it.

Once I fix cbindgen to generate the specializations as needed we can remove
those workarounds, but this is green in the meantime and doesn't hurt.

Depends on D61626

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/879a5784bc60
Enable -Wreturn-type-c-linkage. r=froydnj

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/879a5784bc60

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5f9ed835ff4b
Prove to the incoming clang plugin that our templates are indeed trivially copiable. r=boris

Comment 9

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/5f9ed835ff4b

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/80d4c3f20004
Introduce NonTrivialTypeInFfiChecker. r=andi

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/80d4c3f20004