-
Notifications
You must be signed in to change notification settings - Fork 748
Even more failing layout tests with SpiderMonkey bindings #573
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
Labels
Comments
More reduced test case: template <int>
struct UnusedIntTemplateParam {};
template <class>
class Outer {
static const long SIZE = 1;
UnusedIntTemplateParam<SIZE> i;
};
class AutoIdVector {
Outer<int> ar;
}; Interesting that removing the unused template parameter from |
fitzgen
added a commit
to fitzgen/rust-bindgen
that referenced
this issue
Mar 14, 2017
The old ToRustTy and ItemToRustTy conversion traits were problematic in that they assumed infallibity. That assumption is problematic because we are often dealing with C++ constructs for which Rust has no equivalent *and* we don't have a usable layout from which to generate an opaque blob in its stead. But, a usable layout is often "up the stack" if only we had a way to get up there. For example, Rust does not currently have an equivalent of const value template parameters, and libclang will not give us a layout for template definitions with const value template parameters. However, libclang will give us the layout for an instantiation of such a template definition. First, this commit separates the concepts of generating an equivalent Rust type from generating an opaque blob of the same size and alignment as an IR thing. This consolidates and DRYs up a *ton* of code involved in falling back to an opaque blob (and doing our best to get a Layout for the blob) when we can't generate an equivalent Rust type for some IR thing. Second, it separates fallible and infallible conversions, and provides a nice little framework for when to choose which. This gives us one single place where we do this whole dance: if could not generate equivalent Rust type: if we have a layout: return opaque blob based on layout else: return opaque blob based on a wish and a prayer The ToRustTy trait is replaced by the TryToOpaque, ToOpaque, TryToRustTyOrOpaque, and ToRustTyOrOpaque traits. The ItemToRustTy helper was just a way to avoid ToRustTy's Self::Extra parameter when it was not needed, and is simply removed without a replacement. We suck it up and pass `&()` at call sites now. We *could* introduce ItemTryToOpaque, ItemToOpaque, etc... traits, but the cost of the added boiler plate would outweigh the benefits of not passing `&()` at call sites, IMHO. In addition to being a nice code cleanup, this also fixes rust-lang#573.
fitzgen
added a commit
to fitzgen/rust-bindgen
that referenced
this issue
Mar 14, 2017
The old ToRustTy and ItemToRustTy conversion traits were problematic in that they assumed infallibity. That assumption is problematic because we are often dealing with C++ constructs for which Rust has no equivalent *and* we don't have a usable layout from which to generate an opaque blob in its stead. But, a usable layout is often "up the stack" if only we had a way to get up there. For example, Rust does not currently have an equivalent of const value template parameters, and libclang will not give us a layout for template definitions with const value template parameters. However, libclang will give us the layout for an instantiation of such a template definition. First, this commit separates the concepts of generating an equivalent Rust type from generating an opaque blob of the same size and alignment as an IR thing. This consolidates and DRYs up a *ton* of code involved in falling back to an opaque blob (and doing our best to get a Layout for the blob) when we can't generate an equivalent Rust type for some IR thing. Second, it separates fallible and infallible conversions, and provides a nice little framework for when to choose which. This gives us one single place where we do this whole dance: if could not generate equivalent Rust type: if we have a layout: return opaque blob based on layout else: return opaque blob based on a wish and a prayer The ToRustTy trait is replaced by the TryToOpaque, ToOpaque, TryToRustTyOrOpaque, and ToRustTyOrOpaque traits. The ItemToRustTy helper was just a way to avoid ToRustTy's Self::Extra parameter when it was not needed, and is simply removed without a replacement. We suck it up and pass `&()` at call sites now. We *could* introduce ItemTryToOpaque, ItemToOpaque, etc... traits, but the cost of the added boiler plate would outweigh the benefits of not passing `&()` at call sites, IMHO. In addition to being a nice code cleanup, this also fixes rust-lang#573.
fitzgen
added a commit
to fitzgen/rust-bindgen
that referenced
this issue
Mar 15, 2017
The old ToRustTy and ItemToRustTy conversion traits were problematic in that they assumed infallibity. That assumption is problematic because we are often dealing with C++ constructs for which Rust has no equivalent *and* we don't have a usable layout from which to generate an opaque blob in its stead. But, a usable layout is often "up the stack" if only we had a way to get up there. For example, Rust does not currently have an equivalent of const value template parameters, and libclang will not give us a layout for template definitions with const value template parameters. However, libclang will give us the layout for an instantiation of such a template definition. First, this commit separates the concepts of generating an equivalent Rust type from generating an opaque blob of the same size and alignment as an IR thing. This consolidates and DRYs up a *ton* of code involved in falling back to an opaque blob (and doing our best to get a Layout for the blob) when we can't generate an equivalent Rust type for some IR thing. Second, it separates fallible and infallible conversions, and provides a nice little framework for when to choose which. This gives us one single place where we do this whole dance: if could not generate equivalent Rust type: if we have a layout: return opaque blob based on layout else: return opaque blob based on a wish and a prayer The ToRustTy trait is replaced by the TryToOpaque, ToOpaque, TryToRustTyOrOpaque, and ToRustTyOrOpaque traits. The ItemToRustTy helper was just a way to avoid ToRustTy's Self::Extra parameter when it was not needed, and is simply removed without a replacement. We suck it up and pass `&()` at call sites now. We *could* introduce ItemTryToOpaque, ItemToOpaque, etc... traits, but the cost of the added boiler plate would outweigh the benefits of not passing `&()` at call sites, IMHO. In addition to being a nice code cleanup, this also fixes rust-lang#573.
fitzgen
added a commit
to fitzgen/rust-bindgen
that referenced
this issue
Mar 15, 2017
The old ToRustTy and ItemToRustTy conversion traits were problematic in that they assumed infallibity. That assumption is problematic because we are often dealing with C++ constructs for which Rust has no equivalent *and* we don't have a usable layout from which to generate an opaque blob in its stead. But, a usable layout is often "up the stack" if only we had a way to get up there. For example, Rust does not currently have an equivalent of const value template parameters, and libclang will not give us a layout for template definitions with const value template parameters. However, libclang will give us the layout for an instantiation of such a template definition. First, this commit separates the concepts of generating an equivalent Rust type from generating an opaque blob of the same size and alignment as an IR thing. This consolidates and DRYs up a *ton* of code involved in falling back to an opaque blob (and doing our best to get a Layout for the blob) when we can't generate an equivalent Rust type for some IR thing. Second, it separates fallible and infallible conversions, and provides a nice little framework for when to choose which. This gives us one single place where we do this whole dance: if could not generate equivalent Rust type: if we have a layout: return opaque blob based on layout else: return opaque blob based on a wish and a prayer The ToRustTy trait is replaced by the TryToOpaque, ToOpaque, TryToRustTyOrOpaque, and ToRustTyOrOpaque traits. The ItemToRustTy helper was just a way to avoid ToRustTy's Self::Extra parameter when it was not needed, and is simply removed without a replacement. We suck it up and pass `&()` at call sites now. We *could* introduce ItemTryToOpaque, ItemToOpaque, etc... traits, but the cost of the added boiler plate would outweigh the benefits of not passing `&()` at call sites, IMHO. In addition to being a nice code cleanup, this also fixes rust-lang#573.
bors-servo
pushed a commit
that referenced
this issue
Mar 15, 2017
…test-failures, r=emilio Refactor fallibility of conversions from IR to Rust types The old `ToRustTy` and `ItemToRustTy` conversion traits were problematic in that they assumed infallibity. That assumption is problematic because we are often dealing with C++ constructs for which Rust has no equivalent *and* we don't have a usable layout from which to generate an opaque blob in its stead. But, a usable layout is often "up the stack" if only we had a way to get up there. For example, Rust does not currently have an equivalent of const value template parameters, and libclang will not give us a layout for template definitions with const value template parameters. However, libclang will give us the layout for an instantiation of such a template definition. First, this commit separates the concepts of generating an equivalent Rust type from generating an opaque blob of the same size and alignment as an IR thing. This consolidates and DRYs up a *ton* of code involved in falling back to an opaque blob (and doing our best to get a `Layout` for the blob) when we can't generate an equivalent Rust type for some IR thing. Second, it separates fallible and infallible conversions, and provides a nice little framework for when to choose which. This gives us one single place where we do this whole dance: if could not generate equivalent Rust type: if we have a layout: return opaque blob based on layout else: return opaque blob based on a wish and a prayer The `ToRustTy` trait is replaced by the `TryToOpaque`, `ToOpaque`, `TryToRustTyOrOpaque`, and `ToRustTyOrOpaque` traits. The `ItemToRustTy` helper was just a way to avoid `ToRustTy`'s `Self::Extra` parameter when it was not needed, and is simply removed without a replacement. We suck it up and pass `&()` at call sites now. We *could* introduce `ItemTryToOpaque`, `ItemToOpaque`, etc... traits, but the cost of the added boiler plate would outweigh the benefits of not passing `&()` at call sites, IMHO. In addition to being a nice code cleanup, this also fixes #573. r? @emilio
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Input C/C++ Header
Bindgen Invokation
Actual Results
Emits these bindings:
Compiling with
--test
and running the layout tests results in this failure:Expected Results
We shouldn't fail layout tests. Even in the face of constructs for which we cannot meaningfully make Rust equivalents, we should generate opaque blobs of the correct size.
The text was updated successfully, but these errors were encountered: