From 6187c35b26f2ee87462531826132fd733eaf574a Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 17 Jul 2017 10:03:55 -0700 Subject: [PATCH 1/5] Preserve comments when dumping preprocessed input headers The -C flag tells Clang's preprocessor not to strip comments, but unfortunately Clang will only accept it when dumping the preprocessed file to stdout, which is the -E flag. This means that we need to capture the child process's stdout and then copy it to a file ourselves now. --- src/lib.rs | 25 ++++++++++++++++++------- tests/headers/arg_keyword.hpp | 2 ++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 312155a51d..f1f2fcb1e4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -91,7 +91,7 @@ use std::fs::{File, OpenOptions}; use std::iter; use std::io::{self, Write}; use std::path::{Path, PathBuf}; -use std::process::Command; +use std::process::{Command, Stdio}; use std::sync::Arc; use syntax::ast; @@ -870,19 +870,30 @@ impl Builder { let mut cmd = Command::new(&clang.path); cmd.arg("-save-temps") + .arg("-E") + .arg("-C") .arg("-c") - .arg(&wrapper_path); + .arg(&wrapper_path) + .stdout(Stdio::piped()); for a in &self.options.clang_args { cmd.arg(a); } - if cmd.spawn()?.wait()?.success() { - Ok(()) - } else { - Err(io::Error::new(io::ErrorKind::Other, - "clang exited with non-zero status")) + let mut child = cmd.spawn()?; + if !child.wait()?.success() { + return Err(io::Error::new(io::ErrorKind::Other, + "clang exited with non-zero status")); } + + let mut preprocessed = child.stdout.take().unwrap(); + let mut file = File::create(if is_cpp { + "__bindgen.ii" + } else { + "__bindgen.i" + })?; + io::copy(&mut preprocessed, &mut file)?; + Ok(()) } } diff --git a/tests/headers/arg_keyword.hpp b/tests/headers/arg_keyword.hpp index 9f0af85030..283fcf2306 100644 --- a/tests/headers/arg_keyword.hpp +++ b/tests/headers/arg_keyword.hpp @@ -1 +1,3 @@ +// This comment exists to ensure that `--dump-preprocessed-input` doesn't strip +// comments. void foo(const char* type); From b56edd0e7facc8578381e13b2287972186b75db8 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 17 Jul 2017 10:08:40 -0700 Subject: [PATCH 2/5] Remove warning about stripped annotations that is now obsolete --- CONTRIBUTING.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 56f75ab1f8..1eb6f76bdb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -283,11 +283,6 @@ Afterwards, there should be a `__bindgen.i` or `__bindgen.ii` file containing the combined and preprocessed input headers, which is usable as an isolated, standalone test case. -Note that the preprocessor likely removed all comments, so if the bug you're -trying to pin down involves source annotations (for example, `/**
*/`), then you will have to manually reapply them to the -preprocessed file. - ### Writing a Predicate Script Writing a `predicate.sh` script for a `bindgen` test case is fairly From c29e5bc0164c014f355850044749cf24e8f88bea Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 17 Jul 2017 10:18:35 -0700 Subject: [PATCH 3/5] Mention dumping preprocessed input headers in the issue template --- .github/ISSUE_TEMPLATE.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE.md b/.github/ISSUE_TEMPLATE.md index 02fe3cdec5..a6be94975b 100644 --- a/.github/ISSUE_TEMPLATE.md +++ b/.github/ISSUE_TEMPLATE.md @@ -7,7 +7,11 @@ // files, and therefore any `#include` harms reproducibility. Additionally, // the test case isn't minimal since the included file almost assuredly // contains things that aren't necessary to reproduce the bug, and makes -// tracking it down much more difficult. If necessary, see +// tracking it down much more difficult. +// +// Use the `--dump-preprocessed-input` flag or the +// `bindgen::Builder::dump_preprocessed_input` method to make your test case +// standalone and without `#include`s, and then use C-Reduce to minimize it: // https://github.com/servo/rust-bindgen/blob/master/CONTRIBUTING.md#using-creduce-to-minimize-test-cases ``` From 159fe962b2e389dc834bb7e51c66a14cb905015c Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 17 Jul 2017 10:21:24 -0700 Subject: [PATCH 4/5] Clear up common misconception about RUST_LOG=bindgen --- .github/ISSUE_TEMPLATE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/ISSUE_TEMPLATE.md b/.github/ISSUE_TEMPLATE.md index a6be94975b..ca168c0d31 100644 --- a/.github/ISSUE_TEMPLATE.md +++ b/.github/ISSUE_TEMPLATE.md @@ -63,8 +63,8 @@ bindings is missing a field that exists in the C/C++ struct, note that here.
``` -Insert debug logging when running bindgen with the `RUST_LOG=bindgen` environment -variable set. +Insert debug logging when running bindgen (not when compiling bindgen's output) +with the `RUST_LOG=bindgen` environment variable set. ```
From 02da6536e25a0c45dd8ee77ee37d7993721666e8 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 17 Jul 2017 12:54:51 -0700 Subject: [PATCH 5/5] Drain the child processes stdout before waiting For very large logs to stdout, this prevents deadlocks. --- src/lib.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f1f2fcb1e4..700ca7e9c9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -881,10 +881,6 @@ impl Builder { } let mut child = cmd.spawn()?; - if !child.wait()?.success() { - return Err(io::Error::new(io::ErrorKind::Other, - "clang exited with non-zero status")); - } let mut preprocessed = child.stdout.take().unwrap(); let mut file = File::create(if is_cpp { @@ -893,7 +889,13 @@ impl Builder { "__bindgen.i" })?; io::copy(&mut preprocessed, &mut file)?; - Ok(()) + + if child.wait()?.success() { + Ok(()) + } else { + Err(io::Error::new(io::ErrorKind::Other, + "clang exited with non-zero status")) + } } }