autocxx icon indicating copy to clipboard operation
autocxx copied to clipboard

Function with `wchar_t` generated using `std::uint32_t` instead of `wchar_t`

Open jjacobson93 opened this issue 3 years ago • 9 comments

Expected Behavior

Function with wchar_t as a parameter should generate successfully.

Actual Behavior

Function with wchar_t is generated using std::uint32_t instead of wchar_t, emitting a clang error.

Clang error

/Users/jjacobson/code/rust/autocxx-bad/target/debug/build/autocxx-bad-cd65df0090aaca31/out/autocxx-build-dir/cxx/gen0.cxx:75:21: error: cannot initialize a variable of type '::std::uint32_t (*)(::std::uint32_t)' (aka 'unsigned int (*)(unsigned int)') with an lvalue of type 'wchar_t (wchar_t)': type mismatch at 1st parameter ('::std::uint32_t' (aka 'unsigned int') vs 'wchar_t')
  ::std::uint32_t (*nextWchar$)(::std::uint32_t) = ::nextWchar;
                    ^                              ~~~~~~~~~~~
1 error generated.

Full error from running build

Caused by:
  process didn't exit successfully: `/Users/jjacobson/code/rust/autocxx-bad/target/debug/build/autocxx-bad-82a9b27e51324d18/build-script-build` (exit status: 1)
  --- stdout
  cargo:rerun-if-changed=src/bad.hpp
  TARGET = Some("aarch64-apple-darwin")
  OPT_LEVEL = Some("0")
  HOST = Some("aarch64-apple-darwin")
  CXX_aarch64-apple-darwin = None
  CXX_aarch64_apple_darwin = None
  HOST_CXX = None
  CXX = None
  CXXFLAGS_aarch64-apple-darwin = None
  CXXFLAGS_aarch64_apple_darwin = None
  HOST_CXXFLAGS = None
  CXXFLAGS = None
  CRATE_CC_NO_DEFAULTS = None
  DEBUG = Some("true")
  CARGO_CFG_TARGET_FEATURE = None
  CXX_aarch64-apple-darwin = None
  CXX_aarch64_apple_darwin = None
  HOST_CXX = None
  CXX = None
  CXXFLAGS_aarch64-apple-darwin = None
  CXXFLAGS_aarch64_apple_darwin = None
  HOST_CXXFLAGS = None
  CXXFLAGS = None
  CRATE_CC_NO_DEFAULTS = None
  CARGO_CFG_TARGET_FEATURE = None
  running: "c++" "-O0" "-ffunction-sections" "-fdata-sections" "-fPIC" "-g" "-fno-omit-frame-pointer" "-arch" "arm64" "-I" "src" "-I" "/Users/jjacobson/code/rust/autocxx-bad/target/debug/build/autocxx-bad-cd65df0090aaca31/out/autocxx-build-dir/include" "-Wall" "-Wextra" "-std=c++14" "-o" "/Users/jjacobson/code/rust/autocxx-bad/target/debug/build/autocxx-bad-cd65df0090aaca31/out/autocxx-build-dir/cxx/gen0.o" "-c" "/Users/jjacobson/code/rust/autocxx-bad/target/debug/build/autocxx-bad-cd65df0090aaca31/out/autocxx-build-dir/cxx/gen0.cxx"
  cargo:warning=/Users/jjacobson/code/rust/autocxx-bad/target/debug/build/autocxx-bad-cd65df0090aaca31/out/autocxx-build-dir/cxx/gen0.cxx:75:21: error: cannot initialize a variable of type '::std::uint32_t (*)(::std::uint32_t)' (aka 'unsigned int (*)(unsigned int)') with an lvalue of type 'wchar_t (wchar_t)': type mismatch at 1st parameter ('::std::uint32_t' (aka 'unsigned int') vs 'wchar_t')
  cargo:warning=  ::std::uint32_t (*nextWchar$)(::std::uint32_t) = ::nextWchar;
  cargo:warning=                    ^                              ~~~~~~~~~~~
  cargo:warning=1 error generated.
  exit status: 1

  --- stderr


  error occurred: Command "c++" "-O0" "-ffunction-sections" "-fdata-sections" "-fPIC" "-g" "-fno-omit-frame-pointer" "-arch" "arm64" "-I" "src" "-I" "/Users/jjacobson/code/rust/autocxx-bad/target/debug/build/autocxx-bad-cd65df0090aaca31/out/autocxx-build-dir/include" "-Wall" "-Wextra" "-std=c++14" "-o" "/Users/jjacobson/code/rust/autocxx-bad/target/debug/build/autocxx-bad-cd65df0090aaca31/out/autocxx-build-dir/cxx/gen0.o" "-c" "/Users/jjacobson/code/rust/autocxx-bad/target/debug/build/autocxx-bad-cd65df0090aaca31/out/autocxx-build-dir/cxx/gen0.cxx" with args "c++" did not execute successfully (status code exit status: 1).

Steps to Reproduce the Problem

  1. Create a project with the following (or pull from this repository: https://github.com/jjacobson93/autocxx-bad):

Cargo.toml

[package]
name = "autocxx-bad"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
autocxx = "0.22.3"
cxx = "1.0"

[build-dependencies]
autocxx-build = "0.22.3"
miette = { version="4.3", features=["fancy"] } # optional but gives nicer error messages!

build.rs

fn main() -> miette::Result<()> {
    let path = std::path::PathBuf::from("src"); // include path
    let mut b = autocxx_build::Builder::new("src/main.rs", &[&path]).build()?;
    // This assumes all your C++ bindings are in main.rs
    b.flag_if_supported("-std=c++14").compile("autocxx-bad"); // arbitrary library name, pick anything
    println!("cargo:rerun-if-changed=src/main.rs");
    // Add instructions to link to any C++ libraries you need.
    Ok(())
}

src/main.rs

use autocxx::prelude::*;

include_cpp! {
    #include "bad.hpp"
    safety!(unsafe)
    generate!("nextWchar")
}

fn main() {}

bad.hpp

wchar_t nextWchar(wchar_t c);

bad.cpp

#include "bad.hpp"

wchar_t nextWchar(wchar_t c) { return c + 1; }
  1. Run cargo build at the root of the project.
  2. See errors!

Specifications

cargo version -v

cargo 1.59.0 (49d8809dc 2022-02-10)
release: 1.59.0
commit-hash: 49d8809dc2d3e6e0d5ec634fcf26d8e2aab67130
commit-date: 2022-02-10
host: aarch64-apple-darwin
libgit2: 1.3.0 (sys:0.13.23 vendored)
libcurl: 7.64.1 (sys:0.4.51+curl-7.80.0 system ssl:(SecureTransport) LibreSSL/2.8.3)
os: Mac OS 11.5.2 [64-bit]

jjacobson93 avatar Jul 31 '22 17:07 jjacobson93

Thanks for the report. I'm currently on vacation, so I'm unlikely to have a chance to reproduce this myself within the next few weeks. If you're feeling generous, please could you raise a pull request with a new test case per these instructions? That way I'm more likely to take a look at fixing this sooner.

adetaylor avatar Aug 01 '22 15:08 adetaylor

@adetaylor Thanks for getting back so quickly. Here's the PR: https://github.com/google/autocxx/pull/1142

Hope you have a good rest of your vacation!

jjacobson93 avatar Aug 01 '22 19:08 jjacobson93

Thanks for the test case, that helps a lot.

A bit of investigation.

Here's a pure-bindgen test case, which does the right thing.

[package]
name = "test-proj"
version = "0.1.0"
edition = "2021"
mod bindgen_output;

fn main() {
  println!("Ade 1");
  unsafe { bindgen_output::next_wchar(3) };
  println!("Ade 2");
}
#include <stddef.h>
#include <cstdint>
wchar_t next_wchar(wchar_t c);
#include "header.hpp"

wchar_t next_wchar(wchar_t c) {
    return c;
}

build.rs:

fn main() {
    println!(r"cargo:rustc-link-lib=wchartest");
    println!(r"cargo:rustc-link-search=/Users/adetaylor/dev/bindgen/test-proj");
}

Built with:

bindgen header.hpp > bindgen_output.rs
clang++ -c -o lib.o impl.cpp
ar rc libwchartest.a ../lib.o 
cargo run

Bindgen's output is:

extern "C" {
    #[link_name = "\u{1}__Z10next_wcharw"]
    pub fn next_wchar(c: u32) -> u32;
}

This works because, at least on this specific platform, uint32_t seems to be ABI compatible with wchar_t. Although it's semantically different and therefore has a different mangled name, bindgen of course provides the specific mangled name to which Rust should link.

This doesn't work for autocxx since we generate C++ which calls into the existing C++.

I think therefore this will be another thing for the list on #124 where we need bindgen to output some extra metadata such that we can distinguish this case within autocxx.

Next step will be to look at how easy it is to spot this distinction within the bindgen code based on the clang output. It's possible we might run into https://github.com/jethrogb/rust-cexpr/issues/25 / https://github.com/rust-lang/rust-bindgen/issues/1745.

adetaylor avatar Aug 04 '22 20:08 adetaylor

bindgen has an option --use-distinct-char16-type which we should try, in case it also benefits char32.

adetaylor avatar Sep 14 '22 08:09 adetaylor

bindgen has an option --use-distinct-char16-type which we should try, in case it also benefits char32.

Oh, we were already doing that.

adetaylor avatar Sep 14 '22 18:09 adetaylor

I'm not going to attempt to make progress here until the linked bindgen issues have progress. @jjacobson93 thanks again for the test case PR, it's much appreciated, and sorry this hasn't yielded a quick fix.

adetaylor avatar Apr 02 '23 02:04 adetaylor