Skip to content

codegen: Fix typedef re-export in namespaces when bindings aren't at the root. #436

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

Merged
merged 4 commits into from
Jan 24, 2017

Conversation

emilio
Copy link
Contributor

@emilio emilio commented Jan 24, 2017

No description provided.

@emilio
Copy link
Contributor Author

emilio commented Jan 24, 2017

r? @fitzgen

@emilio
Copy link
Contributor Author

emilio commented Jan 24, 2017

Fixes #433

The problem with rust-lang#425 was the following:

We were parsing the methods after reaching the JS::Value definition.

Those methods contained a JSWhyMagic that we hadn't seen, so we parsed it as
being in the JS:: module.
There's just no advantage in doing so.
@fitzgen
Copy link
Member

fitzgen commented Jan 24, 2017

Can you also add this test case:

namespace JS {
class Value;
}
typedef enum JSWhyMagic {} JSWhyMagic;
namespace JS {
class Value {
public:
  void a(JSWhyMagic);
};
}

@emilio
Copy link
Contributor Author

emilio commented Jan 24, 2017

That's the exact test case that is in tree already (for #410)

@fitzgen
Copy link
Member

fitzgen commented Jan 24, 2017

That's the exact test case that is in tree already (for #410)

:-/

When I try and compile the bindings generated from this test case I get:

$ rustc --crate-type lib js.rs
error[E0412]: cannot find type `JSWhyMagic` in module `root::JS`
  --> js.rs:22:34
   |
22 |                            arg1: root::JS::JSWhyMagic);
   |                                  ^^^^^^^^^^^^^^^^^^^^ not found in `root::JS`
   |
   = help: possible candidate is found in another module, you can import it into scope:
             `use root::JSWhyMagic;`

error[E0412]: cannot find type `JSWhyMagic` in module `root::JS`
  --> js.rs:29:46
   |
29 |             pub unsafe fn a(&mut self, arg1: root::JS::JSWhyMagic) {
   |                                              ^^^^^^^^^^^^^^^^^^^^ not found in `root::JS`
   |
   = help: possible candidate is found in another module, you can import it into scope:
             `use root::JSWhyMagic;`

error: aborting due to 2 previous errors

@fitzgen
Copy link
Member

fitzgen commented Jan 24, 2017

I am getting this same error in the compilation of the SpiderMonkey bindings

@emilio
Copy link
Contributor Author

emilio commented Jan 24, 2017

In tree it happens to compile because root is at the top of the module.

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see what was going on now. Thanks!


for _ in 0..root_import_depth(ctx, item) {
path.push(super_.clone());
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole function body could be

let super_ = ctx.rust_ident_raw("super");
let root = ctx.root_module().canonical_name(ctx);
let root_ident = ctx.rust_ident(&root);

iter::once(ctx.rust_ident_raw("self"))
    .chain(iter::repeat(super_.clone()).take(root_import_depth(ctx, item)))
    .chain(iter::once(root_ident))
    .collect()

@fitzgen
Copy link
Member

fitzgen commented Jan 24, 2017

@bors-servo r+

@bors-servo
Copy link

📌 Commit d6fc044 has been approved by fitzgen

@fitzgen
Copy link
Member

fitzgen commented Jan 24, 2017

@bors-servo r-

@fitzgen
Copy link
Member

fitzgen commented Jan 24, 2017

@bors-servo r+

@bors-servo
Copy link

📌 Commit d6fc044 has been approved by fitzgen

@bors-servo
Copy link

⚡ Test exempted - status

@bors-servo bors-servo merged commit d6fc044 into rust-lang:master Jan 24, 2017
bors-servo pushed a commit that referenced this pull request Jan 24, 2017
codegen: Fix typedef re-export in namespaces when bindings aren't at the root.
@emilio emilio deleted the fix-ns-typedef branch January 24, 2017 21:32
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

Successfully merging this pull request may close these issues.

4 participants