dtolnay / cxx

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

include! uses relative path, which breaks for dependencies #367

Closed Bromeon closed 4 years ago

Bromeon commented 4 years ago

Let's assume project cxx-example has a bridge module, which sets up the C++ bindings. I took it mostly from the cxx-demo crate inside the main repo.

#[cxx::bridge(namespace = org::example)]
mod ffi {
    extern "C" {
        include!("include/demo.h");

        ...
    }
}

The include! is translated to verbatim #include "include/demo.h in C++. Which file this refers to, depends on the include paths on the cc::Build configuration in build.rs.

If I make another project cxx-depend which uses cxx-example as a dependency, then it fails with:

   Compiling cxx-example v0.1.0 (...\cxx-example)
error: failed to run custom build command for `cxx-example v0.1.0 (...\cxx-example)`

Caused by:
  process didn't exit successfully: `...\my_project\target\debug\build\cxx-example-6c41497ba3fecf2f\build-script-build` (exit code: 1)
  --- stderr

  CXX include path:
    ...\my_project\target\debug\build\cxx-example-2606dc5ab30a72a7\out\cxxbridge\include
    ...\my_project\target\debug\build\cxx-example-2606dc5ab30a72a7\out\cxxbridge\crate

  error occurred: Command 
"C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Community\\VC\\Tools\\MSVC\\14.27.29110\\bin\\HostX64\\x64\\cl.exe" 
  "-nologo" "-MD" "-Z7" "-Brepro" 
  "-I" "...\\my_project\\target\\debug\\build\\cxx-example-2606dc5ab30a72a7\\out\\cxxbridge\\include" 
  "-I" "...\\my_project\\target\\debug\\build\\cxx-example-2606dc5ab30a72a7\\out\\cxxbridge\\crate" 
  "-I" "...\\cxx-example" 
  "-W4"
  "-FoG:\\Rust\\Experiments\\my_project\\target\\debug\\build\\cxx-example-2606dc5ab30a72a7\\out\\src/demo.o" 
  "-c" "src/demo.cc"

Note that it fails at cxx-example (not the current crate cxx-depend), which I just successfully compiled.

In the error output, you see that the include paths refer to the current crate, although some included files (e.g. cxx-example/include/demo.h) are in the source tree of cxx-example. It seems like include!(...) does not take that into account.

The include directory is specified at https://github.com/dtolnay/cxx/blob/e37ce2f4ee65a261ee64189585a5615a459c6403/gen/build/src/lib.rs#L190, and is something like <absolutePath>/cxx/target/debug/buildcxx-test-suite-6970f6e2077e6b4d/out/cxxbridge/include, i.e. pointing to the files in the target directory.


Maybe a general question: is include! really the best name for this macro? It took me quite a while that this is not Rust's include!, but effectively equivalent to a C++ #include. Wouldn't something like cxx_include! be clearer?

dtolnay commented 4 years ago

This is supposed to work, though admittedly not well tested on Windows. Take a look at https://docs.rs/cxx-build/0.5.2/cxx_build/static.CFG.html for some discussion of how the include paths are arranged. In particular, when using cxx-build, your include! paths should always begin with a crate name (or the prefix set by the crate through cxx_build::CFG).

    include!("cxx-example/include/demo.h");

If it does not work that way, would it be possible for you to provide a minimal repro with the cxx-example and cxx-depend crates, as a small git repository or tarball?

dtolnay commented 4 years ago

Separately, I notice the following output in your error message: "-I" "...\\cxx-example". That is not something cxx-build put there, right? Your build script explicitly adds that directory itself? If so, probably either don't do that and use the crate name in the start of your import paths like in https://github.com/dtolnay/cxx/issues/367#issuecomment-713912747, or else use CFG.include_prefix = "" to remove enforcing a crate name in the import paths (discouraged).

Bromeon commented 4 years ago

True, forgot to mention I added the current crate's root directory in an attempt to fix this error.

    cxx_build::bridge("src/lib.rs")
        .include(env!("CARGO_MANIFEST_DIR")) // <- added by me
        .file("src/demo.cc")
        .flag_if_supported("-std=c++14")
        .compile("cxxbridge-demo");

Now I remove that extra include dir and change the include!("include/demo.h"); to include!("cxx-example/include/demo.h");, I get a different cc compile error:

"C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Community\\VC\\Tools\\MSVC\\14.27.29110\\bin\\HostX64\\x64\\cl.exe"
   "-nologo" "-MD" "-Z7" "-Brepro"
   "-I" "G:\\Rust\\Experiments\\cxx-example\\target\\debug\\build\\cxx-example-a838c7c80a157435\\out\\cxxbridge\\include"
   "-I" "G:\\Rust\\Experiments\\cxx-example\\target\\debug\\build\\cxx-example-a838c7c80a157435\\out\\cxxbridge\\crate"
   "-W4"
   "-FoG:\\Rust\\Experiments\\cxx-example\\target\\debug\\build\\cxx-example-a838c7c80a157435\\out\\src/demo.o" 
   "-c" "src/demo.cc"

demo.cc
...\cxx-example\target\debug\build\cxx-example-a838c7c80a157435\out\cxxbridge\crate\cxx-example/include/demo.h(9): error C2011: "org::example::ThingC": "class" Typneudefinition
...\cxx-example\src\../include/demo.h(9): note: Siehe Deklaration von "org::example::ThingC"
src/demo.cc(8): error C2027: Verwendung des undefinierten Typs "org::example::ThingC"
...\cxx-example\src\../include/demo.h(9): note: Siehe Deklaration von "org::example::ThingC"
src/demo.cc(8): error C4430: Fehlender Typspezifizierer - int wird angenommen. Hinweis: "default-int" wird von C++ nicht unterstützt.
src/demo.cc(8): error C2550: "org::example::{ctor}": Initialisierungslisten für Konstruktoren dürfen nur in Konstruktordefinition stehen
src/demo.cc(8): warning C4508: "org::example::{ctor}": Funktion sollte einen Wert zurückgeben; Ergebnistyp "void" angenommen
src/demo.cc(10): error C2027: Verwendung des undefinierten Typs "org::example::ThingC"
...\cxx-example\src\../include/demo.h(9): note: Siehe Deklaration von "org::example::ThingC"
src/demo.cc(10): error C4430: Fehlender Typspezifizierer - int wird angenommen. Hinweis: "default-int" wird von C++ nicht unterstützt.
src/demo.cc(10): warning C4508: "org::example::{dtor}": Funktion sollte einen Wert zurückgeben; Ergebnistyp "void" angenommen
src/demo.cc(16): error C2027: Verwendung des undefinierten Typs "org::example::ThingC"
...\cxx-example\src\../include/demo.h(9): note: Siehe Deklaration von "org::example::ThingC"

Translated, it means the type is re-defined, effectively the header file is compiled multiple times. I output the two files, they look like this (the first one is a symlink to the second):

cat "...\cxx-example\target\
      debug\build\cxx-example-a838c7c80a157435\out\cxxbridge\crate\cxx-example/include/demo.h"
cat "...\cxx-example\src\../include/demo.h"
#pragma once
#include "rust/cxx.h"
#include <memory>
#include <string>

namespace org {
namespace example {

class ThingC {
public:
  ThingC(std::string appname);
  ~ThingC();

  std::string appname;
};

struct SharedThing;

std::unique_ptr<ThingC> make_demo(rust::Str appname);
const std::string &get_name(const ThingC &thing);
void do_thing(SharedThing state);

} // namespace example
} // namespace org

To me, it looks a bit like the MSVC compiler cannot recognize #pragma once when symlinks are involved.


To verify that, I set up a small test in a new empty directory. First, create a sub-directory include and a symlink to it:

mkdir include
mklink /d symlink include

Then, add a .hpp and .cpp file:

// include/simple.hpp
#pragma once

class MyClass {};
// simple.cpp
#include "include/simple.hpp"
#include "symlink/simple.hpp"

int main() {
    MyClass m;
}

Now compile:

> cl -c simple.cpp
Microsoft (R) C/C++-Optimierungscompiler Version 19.27.29111 für x64
Copyright (C) Microsoft Corporation. Alle Rechte vorbehalten.

simple.cpp
G:\Others\symlink-pragma-once\symlink/simple.hpp(3): error C2011: "MyClass": "class" Typneudefinition
G:\Others\symlink-pragma-once\include/simple.hpp(3): note: Siehe Deklaration von "MyClass"
simple.cpp(5): error C2079: "m" verwendet undefiniertes class "MyClass"

Exactly the same problem.
So if a header is included multiple times through different paths, #pragma once will fail to recognize the uniqueness.

The obvious fix would be traditional header guards, but maybe it can be avoided to include the header through multiple paths in the first place? Because including through symlinks per se is not a problem on MSVC.

dtolnay commented 4 years ago

Would you be able to provide a repro as a git repo or tarball that I can just run cargo build in? I don't immediately see from your comment how "...\cxx-example\src\../include/demo.h" is being included because that directory is not one of the two on the include path, and I don't want to go by trying to come up with my own repro based on partial information because the toolchain behavior is different on your system than mine:

$ exa -T
.
├── include
│  └── simple.hpp
├── simple.cpp
└── symlink -> include

$ clang++ simple.cpp; echo $?
0

$ g++ simple.cpp; echo $?
0
Bromeon commented 4 years ago

I wrote an entire post explaining the errors, but eventually I could get it to work (working example in this repo). I had a lot of problems reproducing the errors. What surprised me is that incremental compilation was totally unreliable, several times a build failed or succeeded, only to do the opposite when deleting the target directory.

This could be related to sccache, which is anyway not working as nicely as I had hoped. But could also be that cxx's own generator is not run after the initial build. Would a println!("cargo:rerun-if-changed=<RustOrCppFile>"); be enough to trigger cxx to re-generate things?

In case someone is interested in the (now solved) errors ### #pragma once Looks like the `#pragma once` was not the actual problem, it was caused by changing `demo.cc`'s includes to ```cpp #include "../include/demo.h" // instead of #include "cxx-example/include/demo.h" #include "cxx-example/src/main.rs.h" #include ``` Nevertheless good to know that symlinks and `#pragma once` don't work together nicely on MSVC. Anyway, the problem still persists, I created https://github.com/Bromeon/cxx-example to reproduce it. I made it step-wise, first a working `cxx-example` using main.rs, then turning it into a lib.rs (so it can be used by another crate). Once we figure out that, the next step would be to call it from `cxx-depend` crate. ### Using lib instead of bin When changing `main.rs` to `lib.rs`, `cl` failed with: ```cpp src/demo.cc(2): fatal error C1083: Datei (Include) kann nicht geöffnet werden: "cxx-example/src/main.rs.h": No such file or directory ``` I adapted the references in `build.rs`, but forgot to change the include in `demo.cc`. A `#include "cxx-example/src/lib.rs.h"` fixed it.

Generally I'm struggling a bit with the many implicit things that are happening in cxx, such as:

There's probably a good reason for all those points, but it's quite involved to wire all the things together correctly 🙂

By the way, is there an easy way to show cc's stderr output? I only get the failed command and exit code, so I always have to copy the failing command, run it from a VS-enhanced shell and hope that the working directory and environment is the same.

dtolnay commented 4 years ago

I'll go ahead and close this since everything you described appears working as designed.

In the future please consider providing a simple repro up front when opening an issue. You changed the include to "../include/demo.h" or you renamed a file without fixing the places importing the file are probably not things that someone could have identified as the problem based on your comments.

We'll have detailed documentation of how things are wired together on https://cxx.rs just as soon as I get a chance to write the content; tracked in #179.

Bromeon commented 4 years ago

You're totally right, sorry for that and I'll keep it in mind. The changes you mentioned were early attempts to get it working, after your explanation of include! I should have made sure to start from a clean slate.

Nevertheless, thanks a lot for this, I learned a lot during this issue. Also glad to hear you're planning to write extended docs.

In order to prevent others from doing the same mistakes, would it make sense if include! checks whether its argument starts with the crate name (or is an absolute path)? If so, I could maybe help with a PR. There's also a couple of other errors that I might compile in a list, as a sort of "if X happens, you did Y wrong".

dtolnay commented 4 years ago

Sure thing -- that would be terrific.