issue 1723

[smurfd]

Hey

This might be a start to fix this issue. Bare with me :)

I created a Bindings::generate_common() that i call from Bindings::generate()but im unsure if i used the right "parts" of generate() and possibly where we should call the Bindings::generate_common() to avoid BINDGEN_EXTRA_CLANG_ARGS being evaluated after dump-preprocessed-input()

[emilio]

Yeah, so this is a good start, but not quite what I meant (it might be that I goofed a bit on the description).

The main thing we have to do is making sure that the option shenanigans we do in Builder::generate:

rust-bindgen/src/lib.rs

Lines 1307 to 1333 in a467d3e

	// Add any extra arguments from the environment to the clang command line.
	if let Some(extra_clang_args) =
	env::var("BINDGEN_EXTRA_CLANG_ARGS").ok()
	{
	// Try to parse it with shell quoting. If we fail, make it one single big argument.
	if let Some(strings) = shlex::split(&extra_clang_args) {
	self.options.clang_args.extend(strings);
	} else {
	self.options.clang_args.push(extra_clang_args);
	};
	}
	// Transform input headers to arguments on the clang command line.
	self.options.input_header = self.input_headers.pop();
	self.options
	.clang_args
	.extend(self.input_headers.drain(..).flat_map(|header| {
	iter::once("-include".into()).chain(iter::once(header))
	}));
	self.options.input_unsaved_files.extend(
	self.input_header_contents
	.drain(..)
	.map(|(name, contents)| {
	clang::UnsavedFile::new(&name, &contents)
	}),
	);

get applied to dump_preprocessed_input. It may be easy enough to move those to a separate function, clone the current options in dump_preprocessed_input, and call that function.

Does that make sense?

[smurfd]

Hey. yeah that makes sense i think. ill remove the generate_common() then I do something like :

fn generate_options(self, mut opt: BindgenOptions) -> BindgenOptions { ....}

with this in it

rust-bindgen/src/lib.rs

Lines 1307 to 1333 in a467d3e

	// Add any extra arguments from the environment to the clang command line.
	if let Some(extra_clang_args) =
	env::var("BINDGEN_EXTRA_CLANG_ARGS").ok()
	{
	// Try to parse it with shell quoting. If we fail, make it one single big argument.
	if let Some(strings) = shlex::split(&extra_clang_args) {
	self.options.clang_args.extend(strings);
	} else {
	self.options.clang_args.push(extra_clang_args);
	};
	}
	// Transform input headers to arguments on the clang command line.
	self.options.input_header = self.input_headers.pop();
	self.options
	.clang_args
	.extend(self.input_headers.drain(..).flat_map(|header| {
	iter::once("-include".into()).chain(iter::once(header))
	}));
	self.options.input_unsaved_files.extend(
	self.input_header_contents
	.drain(..)
	.map(|(name, contents)| {
	clang::UnsavedFile::new(&name, &contents)
	}),
	);

generate looks:

    pub fn generate(mut self) -> Result<Bindings, ()> {
        Bindings::generate(self.generate_options(generate.options))
    }

call that from

rust-bindgen/src/lib.rs

Line 1404 in a467d3e

Where i pass the self.options as a parameter.

.... ofcourse i run into the whole : move occurs because value has type BindgenOptions, which does not implement the Copy trait
And im pretty sure you don't want the Copy,Clone traits implemented :)

Is it something similarly done else where that i can look at?

[smurfd]

Its easier to see what i was talking about if i show you... hence ^

[smurfd]

nooooooooooooooo i did something wrong :'(
please dont break github?!
--- edit ---
hmm i hope that the fact that i see all others commits above is a feature that github just enabled? *please, if so then i didnt do anything wrong :) *
--- edit2 ---
no i did something bad. long story short, new computer new git clone. git mojo not strong.
my branch must have become master somehow....

how do i fix? Sorry for this!!!!

[emilio]

I think what I'd do is:

git fetch upstream
git rebase upstream/master
# Hit conflicts? git mergetool, and solve them, then git rebase --continue
git push -f

[emilio]

Or alternatively:

git diff upstream/master > diff
git reset --hard upstream/master
git apply diff
git commit -am "Issue 1723"
git push -f

That should leave you with your changes, but just one commit on top of master.

[smurfd]

THANK YOU! phew , we are back to 1 commit again. :)
used the 2nd alternative. but got a problem when i pushed.

$ git push -f 
fatal: The current branch issue-1723 has no upstream branch.
To push the current branch and set the remote as upstream, use

    git push --set-upstream origin issue-1723

got some other problem when i ran that command. But after some searching i found this
$ git config --global push.default current

after that i was able to push :)

[bors-servo]

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

[smurfd]

same thing again :( ill fix

[smurfd]

for info, this latest patch might actually be what we are looking for.
It builds and passes the cargo test tests

[smurfd]

forgot to remove the generate_common() i had in my 1st patch.

[bors-servo]

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

[emilio]

I took the liberty of updating this. It needed a few changes because making dump_preprocessed_input take &mut would be a breaking change. But it ended up being simple enough.

Thanks a lot @smurfd, and sorry I ended up missing your previous update for so long.