dtolnay / cxx

Safe interop between Rust and C++
https://cxx.rs
Apache License 2.0
5.82k stars 330 forks source link

Using g++ and -Wmissing-declarations produces warning in generated C++ code #1265

Open omoerbeek opened 1 year ago

omoerbeek commented 1 year ago

Steps tor reproduce: use g++ as compiler and take the tutorial and add

.flag("-Wmissing-declarations")

to build.rs. This produces the following warnings.

warning: /home/otto/cxx-demo/target/debug/build/cxx-demo-94d9b893a28bb988/out/cxxbridge/sources/cxx-demo/src/main.rs.cc:39:20: warning: no previous declaration for ‘BlobstoreClient* cxxbridge1$new_blobstore_client()’ [-Wmissing-declarations]
warning:    39 | ::BlobstoreClient *cxxbridge1$new_blobstore_client() noexcept {
warning:       |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
warning: /home/otto/cxx-demo/target/debug/build/cxx-demo-94d9b893a28bb988/out/cxxbridge/sources/cxx-demo/src/main.rs.cc:47:6: warning: no previous declaration for ‘void cxxbridge1$unique_ptr$BlobstoreClient$null(std::unique_ptr<BlobstoreClient>*)’ [-Wmissing-declarations]
warning:    47 | void cxxbridge1$unique_ptr$BlobstoreClient$null(::std::unique_ptr<::BlobstoreClient> *ptr) noexcept {
warning:       |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
warning: /home/otto/cxx-demo/target/debug/build/cxx-demo-94d9b893a28bb988/out/cxxbridge/sources/cxx-demo/src/main.rs.cc:50:6: warning: no previous declaration for ‘void cxxbridge1$unique_ptr$BlobstoreClient$raw(std::unique_ptr<BlobstoreClient>*, BlobstoreClient*)’ [-Wmissing-declarations]
warning:    50 | void cxxbridge1$unique_ptr$BlobstoreClient$raw(::std::unique_ptr<::BlobstoreClient> *ptr, ::BlobstoreClient *raw) noexcept {
warning:       |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
warning: /home/otto/cxx-demo/target/debug/build/cxx-demo-94d9b893a28bb988/out/cxxbridge/sources/cxx-demo/src/main.rs.cc:53:26: warning: no previous declaration for ‘const BlobstoreClient* cxxbridge1$unique_ptr$BlobstoreClient$get(const std::unique_ptr<BlobstoreClient>&)’ [-Wmissing-declarations]
warning:    53 | ::BlobstoreClient const *cxxbridge1$unique_ptr$BlobstoreClient$get(::std::unique_ptr<::BlobstoreClient> const &ptr) noexcept {
warning:       |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
warning: /home/otto/cxx-demo/target/debug/build/cxx-demo-94d9b893a28bb988/out/cxxbridge/sources/cxx-demo/src/main.rs.cc:56:20: warning: no previous declaration for ‘BlobstoreClient* cxxbridge1$unique_ptr$BlobstoreClient$release(std::unique_ptr<BlobstoreClient>&)’ [-Wmissing-declarations]
warning:    56 | ::BlobstoreClient *cxxbridge1$unique_ptr$BlobstoreClient$release(::std::unique_ptr<::BlobstoreClient> &ptr) noexcept {
warning:       |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
warning: /home/otto/cxx-demo/target/debug/build/cxx-demo-94d9b893a28bb988/out/cxxbridge/sources/cxx-demo/src/main.rs.cc:59:6: warning: no previous declaration for ‘void cxxbridge1$unique_ptr$BlobstoreClient$drop(std::unique_ptr<BlobstoreClient>*)’ [-Wmissing-declarations]
warning:    59 | void cxxbridge1$unique_ptr$BlobstoreClient$drop(::std::unique_ptr<::BlobstoreClient> *ptr) noexcept {
warning:       |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
dtolnay commented 1 year ago

Is it typical to run lints like this against third-party code?

I can't imagine that any significant fraction of C++ libraries in the wild are clean against -Wmissing-declarations. That lint seems like just a stylistic preference.

omoerbeek commented 1 year ago

The functions are not declared static, so they can be called from another compilation unit. That means that the prototype should be available somewhere in that compilation unit. Preferably in a header file so that it becomes more easy to make sure the prototype and implementation agree by also including that header file into the compilation unit that has the function definitions. If the functions are not called from outside they should be marked static.

dtolnay commented 1 year ago

They are called from Rust.

omoerbeek commented 1 year ago

I see. In that case they cannot be madestatic. I can see two options remaining: either introduce a prototype in the same file before the definition of the functions, just to silence the compiler, or leave it as-is and accept the warnings. For us specifically the drawback of that is that we aim for a warning-free compile (with pretty strict -W settings) and that goal becomes impossible to reach.