Skip to content
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

TemplateParameters do not return Option #1305

Merged
merged 5 commits into from
Apr 10, 2018
Merged

TemplateParameters do not return Option #1305

merged 5 commits into from
Apr 10, 2018

Conversation

tamird
Copy link
Contributor

@tamird tamird commented Apr 7, 2018

Fixes #960.
Closes #1245.

I found it useful to do this incrementally, changing each method in a separate commit and ensuring the tests continue to pass unchanged.

r? @emilio
/cc @ericho

@emilio
Copy link
Contributor

emilio commented Apr 7, 2018

Looks good, thank you very much.

Sorry for not seeing the last commit in #1245 @ericho :(.

This one looks ready to land while that one still seemed to change behavior, so let's land this instead.

@bors-servo r+

@bors-servo
Copy link

📌 Commit ca48146 has been approved by emilio

@bors-servo
Copy link

🔒 Merge conflict

@bors-servo
Copy link

☔ The latest upstream changes (presumably #1303) made this pull request unmergeable. Please resolve the merge conflicts.

@tamird
Copy link
Contributor Author

tamird commented Apr 7, 2018 via email

@ericho
Copy link

ericho commented Apr 7, 2018

No problem @emilio I was stuck anyway.

This is great I can learn from @tamird changes 😄

@tamird
Copy link
Contributor Author

tamird commented Apr 7, 2018

@emilio could I get another r+?

@emilio
Copy link
Contributor

emilio commented Apr 8, 2018

@bors-servo r+

Sure, thanks both!

@bors-servo
Copy link

📌 Commit 9b69f3e has been approved by emilio

@bors-servo
Copy link

⌛ Testing commit 9b69f3e with merge 1f9bcbe...

bors-servo pushed a commit that referenced this pull request Apr 8, 2018
TemplateParameters do not return Option

Fixes #960.
Closes #1245.

I found it useful to do this incrementally, changing each method in a separate commit and ensuring the tests continue to pass unchanged.

r? @emilio
/cc @ericho
@bors-servo
Copy link

💔 Test failed - status-travis

@tamird
Copy link
Contributor Author

tamird commented Apr 8, 2018

I bisected this down to the last first commit - the others all pass. @emilio any ideas?

--- stderr
cpp/Test.h:1:9: warning: #pragma once in main file [-Wpragma-once-outside-header], err: false
thread 'main' panicked at '"::" is not a valid Term', /Users/tamird/.cargo/registry/src/github.com-1ecc6299db9ec823/proc-macro2-0.3.6/src/stable.rs:463:9
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at libstd/sys_common/backtrace.rs:59
             at libstd/panicking.rs:380
   3: std::panicking::default_hook
             at libstd/panicking.rs:396
   4: std::panicking::begin_panic
             at libstd/panicking.rs:576
   5: std::panicking::begin_panic
             at libstd/panicking.rs:537
   6: std::panicking::try::do_call
             at libstd/panicking.rs:521
   7: proc_macro2::imp::validate_term
             at /Users/tamird/.cargo/registry/src/github.com-1ecc6299db9ec823/proc-macro2-0.3.6/src/stable.rs:463
   8: proc_macro2::imp::Term::new
             at /Users/tamird/.cargo/registry/src/github.com-1ecc6299db9ec823/proc-macro2-0.3.6/src/stable.rs:406
   9: proc_macro2::Term::_new
             at /Users/tamird/.cargo/registry/src/github.com-1ecc6299db9ec823/proc-macro2-0.3.6/src/lib.rs:424
  10: bindgen::codegen::root_import
             at /Users/tamird/src/rust-bindgen/src/codegen/mod.rs:78
  11: <bindgen::ir::module::Module as bindgen::codegen::CodeGenerator>::codegen::{{closure}}
             at /Users/tamird/src/rust-bindgen/src/codegen/mod.rs:424
  12: bindgen::codegen::CodegenResult::inner
             at /Users/tamird/src/rust-bindgen/src/codegen/mod.rs:212
  13: alloc::str::<impl alloc::slice::SliceConcatExt<str> for [S]>::concat::{{closure}}
             at /Users/tamird/src/rust-bindgen/src/codegen/mod.rs:423
  14: <std::ffi::c_str::CString as core::ops::deref::Deref>::deref
             at /Users/tamird/src/rust-bindgen/src/codegen/mod.rs:360
  15: bindgen::codegen::codegen::{{closure}}
             at /Users/tamird/src/rust-bindgen/src/codegen/mod.rs:3441
  16: bindgen::ir::context::BindgenContext::gen
             at /Users/tamird/src/rust-bindgen/src/ir/context.rs:1222
  17: bindgen::codegen::codegen
             at /Users/tamird/src/rust-bindgen/src/codegen/mod.rs:3419
  18: bindgen::Bindings::generate
             at /Users/tamird/src/rust-bindgen/src/lib.rs:1605
  19: core::ptr::<impl *const T>::offset
             at /Users/tamird/src/rust-bindgen/src/lib.rs:1072
  20: build_script_build::main
             at ./build.rs:36
  21: std::rt::lang_start::{{closure}}
             at /Users/travis/build/rust-lang/rust/src/libstd/rt.rs:74
  22: std::panicking::try::do_call
             at libstd/rt.rs:59
             at libstd/panicking.rs:479
  23: panic_unwind::dwarf::eh::read_encoded_pointer
             at libpanic_unwind/lib.rs:102
  24: rust_panic
             at libstd/panicking.rs:458
             at libstd/panic.rs:358
             at libstd/rt.rs:58
  25: std::rt::lang_start
             at /Users/travis/build/rust-lang/rust/src/libstd/rt.rs:74
  26: <build_script_build::MacroCallback as core::fmt::Debug>::fmt

@tamird
Copy link
Contributor Author

tamird commented Apr 8, 2018

Oops, the first bad commit is the first commit. Did something wrong the first time.

@tamird
Copy link
Contributor Author

tamird commented Apr 8, 2018

Oh, hang on. This also failed on master https://travis-ci.org/rust-lang-nursery/rust-bindgen/builds/363772130.

I think this is because we silently picked up proc-macro 0.3.6 which includes dtolnay/proc-macro2@489c642. I'm going to pin to 0.3.5 and see what happens.

tamird referenced this pull request in dtolnay/proc-macro2 Apr 8, 2018
@tamird
Copy link
Contributor Author

tamird commented Apr 8, 2018

@emilio can I get another run, please?

@emilio
Copy link
Contributor

emilio commented Apr 10, 2018

Yeah, sorry for the lag. Thanks again!

@bors-servo r+

@bors-servo
Copy link

📌 Commit 8660c09 has been approved by emilio

@bors-servo
Copy link

⌛ Testing commit 8660c09 with merge 56d6e7e...

bors-servo pushed a commit that referenced this pull request Apr 10, 2018
TemplateParameters do not return Option

Fixes #960.
Closes #1245.

I found it useful to do this incrementally, changing each method in a separate commit and ensuring the tests continue to pass unchanged.

r? @emilio
/cc @ericho
@bors-servo
Copy link

💔 Test failed - status-travis

@emilio emilio merged commit af54e58 into rust-lang:master Apr 10, 2018
@emilio
Copy link
Contributor

emilio commented Apr 10, 2018

Only one test timed out.

@tamird tamird deleted the remove-option branch April 10, 2018 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants