Skip to content

Add a Builder::header_contents method to allow passing source code as a string to a Builder #601

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 1 commit into from
Mar 27, 2017

Conversation

luser
Copy link
Contributor

@luser luser commented Mar 27, 2017

Currently the Builder API requires input as header files on disk. This allows
passing C source directly to clang using the existing UnsavedFile struct.

I'm going to take a shot at implementing a webapp that lets users paste in C source and get the bindgen output. It would make life easier if I didn't have to write the source out to a temp file. This patch allows passing the source in as a &str.

I wasn't quite sure where to put the new test, but it seems to work in tests.rs.

Copy link
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

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

Looks sensible to me, thanks for doing this! Only one nit, and this probably needs to wait to #600 to land, because our rustfmt setup in CI used to allow failures, and that changed recently.

Thanks Ted!

@@ -175,6 +175,12 @@ impl Builder {
self
}

/// Add `contents` as an input C/C++ header named `name`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps note that the files are automatically added to the clang flags?

tests/tests.rs Outdated
@@ -32,6 +32,7 @@ fn compare_generated_header(header: &PathBuf,
Err(_) => "".to_string(),
};


Copy link
Contributor

Choose a reason for hiding this comment

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

nit: stray newline?

@luser
Copy link
Contributor Author

luser commented Mar 27, 2017

Fixed those nits, thanks for the quick review!

src/clang.rs Outdated
@@ -1409,7 +1409,7 @@ impl Drop for Diagnostic {
/// A file which has not been saved to disk.
pub struct UnsavedFile {
x: CXUnsavedFile,
name: CString,
pub name: CString,
Copy link
Contributor

Choose a reason for hiding this comment

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

CI is failing because this field is public but it's not documented. Please add a line like:

/// The name of the unsaved file. We carry that over to avoid leaving dangling pointers in `CXUnsavedFile`

Or something like that.

…as a string to a Builder

Currently the Builder API requires input as header files on disk. This allows
passing C source directly to clang using the existing UnsavedFile struct.
@emilio
Copy link
Contributor

emilio commented Mar 27, 2017

@bors-servo r+

Thanks again!

@bors-servo
Copy link

📌 Commit 63de948 has been approved by emilio

@bors-servo
Copy link

⌛ Testing commit 63de948 with merge 7ed7c24...

bors-servo pushed a commit that referenced this pull request Mar 27, 2017
Add a `Builder::header_contents` method to allow passing source code as a string to a Builder

Currently the Builder API requires input as header files on disk. This allows
passing C source directly to clang using the existing UnsavedFile struct.

I'm going to take a shot at implementing a webapp that lets users paste in C source and get the bindgen output. It would make life easier if I didn't have to write the source out to a temp file. This patch allows passing the source in as a `&str`.

I wasn't quite sure where to put the new test, but it seems to work in tests.rs.
@bors-servo
Copy link

☀️ Test successful - status-travis
Approved by: emilio
Pushing 7ed7c24 to master...

@bors-servo bors-servo merged commit 63de948 into rust-lang:master Mar 27, 2017
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