Constant variable improvements. #260

emilio · 2016-11-15T13:30:50Z

Fixes #256.

r? @fitzgen

highfive · 2016-11-15T13:30:53Z

Warning

These commits modify unsafe code. Please review it carefully!

fitzgen · 2016-11-15T17:36:54Z

@emilio CI is failing, going to hold off on reviewing this until you fix it. Ping me when its ready :)

emilio · 2016-11-15T17:40:58Z

@fitzgen CI is failing due to the functions not present in llvm3.8. I didn't want to bring back the python infra having #204 so close, so I'm waiting for that :)

bors-servo · 2016-11-15T18:11:06Z

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

bors-servo · 2016-11-15T20:00:48Z

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

emilio · 2016-11-15T20:08:10Z

This should be ready for review @fitzgen

bors-servo · 2016-11-15T23:44:18Z

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

emilio · 2016-11-16T09:17:34Z

Ok, CI is green after removing nightly builds.

emilio · 2016-11-16T13:17:29Z

This includes the fix for #271, given without the changes in this PR to the test suite adding new tests correctly is a pain.

…o USR. Apparently MSVC isn't that good at giving us USRs... Fixes rust-lang#271

fitzgen

LGTM, thanks for breaking this up into nice little commits :)

fitzgen · 2016-11-16T16:56:16Z

libbindgen/build.rs

-                    let _ = writeln!(dst, "test_header!(header_{}, {:?});",
-                        func, entry.path());
+                    writeln!(dst, "test_header!(header_{}, {:?});",
+                             func, entry.path()).unwrap();


Did you rustfmt this? I'd think it would put each parameter on its own line if they are too long to fit on one line.

I didn't because cargo fmt doesn't detect the build.rs file apparently. Probably worth making it recognise it as a followup.

fitzgen · 2016-11-16T16:57:11Z

libbindgen/src/clang.rs

+    /// Create a dummy EvalResult.
+    pub fn new(_: Cursor) -> Self {
+        EvalResult {
+            x: ::std::ptr::null_mut(),


use std::ptr at the top?

fitzgen · 2016-11-16T16:59:38Z

libbindgen/src/codegen/mod.rs

+                        }
+                        Err(..) => {
+                            const_item
+                                .build(helpers::ast_ty::byte_array_expr(bytes))


I wonder if we should always make C strings a byte array...

It's just because the bytestring reads better, but should be effectively equivalent.

@emilio

It's just because the bytestring reads better, but should be effectively equivalent

actually bindgen generates i8 byte arrays for literal strings in processor definitions. u8 arrays would IMHO be a more useful resp. *const c_char compatible representation.

Could you file an issue with a test-case? Thanks!

i wrote an issue ticket in the meanwhile: #1401

but i think it's more an issue, which rather needs contemplation than any example, where it leads to unpleasant results.

fitzgen · 2016-11-16T17:00:24Z

libbindgen/src/ir/ty.rs

@@ -123,6 +123,14 @@ impl Type {
        Self::new(Some(name), None, kind, false)
    }

+    /// Is this an floating point type?


"an" -> "a"

fitzgen · 2016-11-16T20:51:53Z

libbindgen/src/clang.rs

-            c_str.to_string_lossy().into_owned()
+            let ret = c_str.to_string_lossy().into_owned();
+            clang_disposeString(self);
+            ret


... lol ...

It's not a big deal because they're refcounted, but still...

emilio · 2016-11-16T21:12:50Z

@bors-servo r=fitzgen

Thanks for the review Nick, and sorry for this patch pile :/

bors-servo · 2016-11-16T21:12:51Z

📌 Commit 19d36d4 has been approved by fitzgen

bors-servo · 2016-11-16T21:12:53Z

⌛ Testing commit 19d36d4 with merge e74acce...

@fitzgen

Constant variable improvements. Fixes #256. r? @fitzgen

bors-servo · 2016-11-16T21:28:35Z

☀️ Test successful - status-travis

highfive assigned fitzgen Nov 15, 2016

highfive added the S-awaiting-review label Nov 15, 2016

emilio force-pushed the macro-str branch from 432da20 to 3326621 Compare November 15, 2016 13:36

emilio mentioned this pull request Nov 15, 2016

clang: Evaluate more complex constant expressions in variables. #259

Closed

emilio changed the title ~~Generate macro string constants.~~ Constant variable improvements. Nov 15, 2016

emilio mentioned this pull request Nov 15, 2016

Split off bindgen library into a sub-crate #204

Merged

highfive added the S-needs-rebase label Nov 15, 2016

emilio force-pushed the macro-str branch from b5f5ec6 to 9bddf21 Compare November 15, 2016 19:58

highfive added the S-needs-rebase label Nov 15, 2016

emilio force-pushed the macro-str branch 2 times, most recently from dfc1141 to fe26aea Compare November 15, 2016 20:05

emilio force-pushed the macro-str branch from fe26aea to 38a4561 Compare November 15, 2016 20:10

emilio removed the S-needs-rebase label Nov 15, 2016

emilio force-pushed the macro-str branch 5 times, most recently from ea35be8 to 1536cf7 Compare November 15, 2016 21:28

highfive added the S-needs-rebase label Nov 15, 2016

emilio added 4 commits November 16, 2016 00:46

clangll: Add bindings to CXEvalResult

239d62b

clang: Don't leak all the strings in the world.

f9422a4

clang: Evaluate more complex constant expressions in variables.

d516168

reformat.

91faa76

emilio added 3 commits November 16, 2016 00:47

Multiple constant generation evaluation improvements.

824f99a

Don't ignore errors in the test suite.

b348e73

ir: var: Missing docs.

9380ed5

emilio force-pushed the macro-str branch from 1536cf7 to 9380ed5 Compare November 15, 2016 23:47

emilio removed the S-needs-rebase label Nov 16, 2016

emilio mentioned this pull request Nov 16, 2016

const field inside class is not handled correctly on Windows #271

Closed

emilio force-pushed the macro-str branch from b4fdf72 to 9920bc7 Compare November 16, 2016 13:38

ir: context: fall back to use the declaration as a key when there's n…

2468fb2

…o USR. Apparently MSVC isn't that good at giving us USRs... Fixes rust-lang#271

emilio force-pushed the macro-str branch from 9920bc7 to 2468fb2 Compare November 16, 2016 13:55

This was referenced Nov 16, 2016

macros defining integer constants using expressions containing enum values don't work #258

Open

Support for macro defined string constants #256

Closed

fitzgen approved these changes Nov 16, 2016

View reviewed changes

Address review comments.

19d36d4

highfive removed the S-awaiting-review label Nov 16, 2016

highfive added the S-awaiting-merge label Nov 16, 2016

bors-servo pushed a commit that referenced this pull request Nov 16, 2016

Auto merge of #260 - emilio:macro-str, r=fitzgen

e74acce

Constant variable improvements. Fixes #256. r? @fitzgen

bors-servo merged commit 19d36d4 into rust-lang:master Nov 16, 2016

highfive removed the S-awaiting-merge label Nov 16, 2016

emilio deleted the macro-str branch November 16, 2016 21:29

Manishearth unassigned fitzgen Jul 25, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Constant variable improvements. #260

Constant variable improvements. #260

emilio commented Nov 15, 2016

highfive commented Nov 15, 2016

fitzgen commented Nov 15, 2016

emilio commented Nov 15, 2016 •

edited

Loading

bors-servo commented Nov 15, 2016

bors-servo commented Nov 15, 2016

emilio commented Nov 15, 2016

bors-servo commented Nov 15, 2016

emilio commented Nov 16, 2016

emilio commented Nov 16, 2016

fitzgen left a comment

fitzgen Nov 16, 2016

emilio Nov 16, 2016

fitzgen Nov 16, 2016

fitzgen Nov 16, 2016

emilio Nov 16, 2016

mash-graz Sep 24, 2018

emilio Sep 24, 2018

emilio Sep 24, 2018

mash-graz Sep 26, 2018 •

edited

Loading

fitzgen Nov 16, 2016

fitzgen Nov 16, 2016

emilio Nov 16, 2016

emilio commented Nov 16, 2016

bors-servo commented Nov 16, 2016

bors-servo commented Nov 16, 2016

bors-servo commented Nov 16, 2016

Constant variable improvements. #260

Constant variable improvements. #260

Conversation

emilio commented Nov 15, 2016

highfive commented Nov 15, 2016

fitzgen commented Nov 15, 2016

emilio commented Nov 15, 2016 • edited Loading

bors-servo commented Nov 15, 2016

bors-servo commented Nov 15, 2016

emilio commented Nov 15, 2016

bors-servo commented Nov 15, 2016

emilio commented Nov 16, 2016

emilio commented Nov 16, 2016

fitzgen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mash-graz Sep 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emilio commented Nov 16, 2016

bors-servo commented Nov 16, 2016

bors-servo commented Nov 16, 2016

bors-servo commented Nov 16, 2016

emilio commented Nov 15, 2016 •

edited

Loading

mash-graz Sep 26, 2018 •

edited

Loading