Skip to content

Bug in cxxbridge-cmd? Generated incorrect namespace #441

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

Closed
XiangpengHao opened this issue Nov 10, 2020 · 8 comments
Closed

Bug in cxxbridge-cmd? Generated incorrect namespace #441

XiangpengHao opened this issue Nov 10, 2020 · 8 comments

Comments

@XiangpengHao
Copy link
Contributor

Hi, I encountered this after upgrading the cxxbridge-cmd to 0.5.6:

// lib.rs
#[cxx::bridge(namespace = rust_part)]
mod ffi {
    extern "C++" {
        type Person;
        fn get_name(person: &Person) -> &CxxString;
    }
}

Run cxxbridge:

cxxbridge lib.rs --header
cxxbridge lib.rs

And get the header:

#pragma once
#include <string>

namespace rust_part {
  using Person = ::rust_part::Person;
}

and source:

#include <string>

namespace rust_part {
  using Person = ::rust_part::Person;
}

namespace rust_part {
extern "C" {
const ::std::string *rust_part$cxxbridge05$get_name(const ::rust_part::Person &person) noexcept {
  const ::std::string &(*get_name$)(const ::rust_part::Person &) = ::rust_part::get_name;
  return &get_name$(person);
}
} // extern "C"
} // namespace rust_part

I suppose the Person should not be in rust_part namespace, in fact (at least) in 0.5.2 it yields the following code and compiles for me.

#pragma once
#include <string>

namespace rust_part {

using Person = Person;

} // namespace rust_part

Am I missing something? This also relates to the problem discussed #434

@dtolnay
Copy link
Owner

dtolnay commented Nov 10, 2020

This looks like #416. Sorry, this was not intentional. :(

For now you can work around it by writing Person without a namespace attribute, like this:

#[cxx::bridge]
mod ffi {
    extern "C++" {
        type Person;
    }

    #[namespace = "rust_part"]
    extern "C++" {
        fn get_name(person: &Person) -> &CxxString;
    }
}

or like this:

#[cxx::bridge]
mod ffi {
    extern "C++" {
        type Person;

        #[namespace = "rust_part"]
        fn get_name(person: &Person) -> &CxxString;
    }
}

@XiangpengHao
Copy link
Contributor Author

Thanks for the reply!

But this doesn't work:

#[cxx::bridge]
mod ffi {
    #[namespace = "rust_part"]
    extern "Rust"{
        fn hello();
    }
}

fn hello() {
    println!("hello");
}

-->

#pragma once

void hello() noexcept;

I can only specify namespace on the function/type not the entire extern block, is that intended?

@dtolnay
Copy link
Owner

dtolnay commented Nov 11, 2020

Ah, I forgot that this was called out as not implemented yet in #370. I put up #444 with the implementation for namespace attributes on extern blocks and will publish 0.5.8 shortly.

@XiangpengHao
Copy link
Contributor Author

Great! Thanks for the support!

@dtolnay
Copy link
Owner

dtolnay commented Nov 11, 2020

Published 0.5.8:

$ cxxbridge --version
cxxbridge 0.5.8

$ echo '#[cxx::bridge] mod ffi { #[namespace = "rust_part"] extern "Rust" { fn hello(); } }' | cxxbridge --header -
#pragma once

namespace rust_part {
void hello() noexcept;
} // namespace rust_part

@XiangpengHao
Copy link
Contributor Author

How do you consider this case:

#[cxx::bridge]
mod ffi {
    #[namespace = "shared"]
    struct Color {
        r: u8,
        g: u8,
        b: u8
    }

    #[namespace = "rust_part"]
    extern "Rust"{
        fn is_white(self: &Color) -> bool;
    }
}

Note that the Color is in shared namespace while its member function is_white is in rust_part namespace

Probably it's better to just reject the code, but I haven't got a chance to look at the implementation.

@dtolnay
Copy link
Owner

dtolnay commented Nov 11, 2020

Looks like we currently generated something like the following, which does not compile. Good catch.

namespace shared {
struct Color final {
  uint8_t r;
  uint8_t g;
  uint8_t b;

  bool is_white() const noexcept;
};
} // namespace shared

namespace rust_part {
bool Color::is_white() const noexcept {
  ...
}
} // namespace rust_part
cxxbridge/sources/demo/src/main.rs.cc:25:6: error: ‘Color’ has not been declared
   25 | bool Color::is_white() const noexcept {
      |      ^~~~~

I wouldn't be opposed to accepting that code and just making it put the member function in the struct's namespace all the time, regardless of namespace attribute on the member function.

But if someone feels strongly this should be an error (with a better message), that would be reasonable too.

@dtolnay
Copy link
Owner

dtolnay commented Nov 12, 2020

I filed #460 to track the bug described in #441 (comment) of method with a different explicit namespace than the receiver type.

@dtolnay dtolnay closed this as completed Nov 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants