Skip to content

Do not evaluate variadic template types #284

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
Nov 21, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 27 additions & 8 deletions libbindgen/src/clang.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ impl Cursor {
}

/// Try to evaluate this cursor.
pub fn evaluate(&self) -> EvalResult {
pub fn evaluate(&self) -> Option<EvalResult> {
EvalResult::new(*self)
}
}
Expand Down Expand Up @@ -1286,10 +1286,8 @@ pub struct EvalResult {
#[cfg(feature = "llvm_stable")]
impl EvalResult {
/// Create a dummy EvalResult.
pub fn new(_: Cursor) -> Self {
EvalResult {
x: ptr::null_mut(),
}
pub fn new(_: Cursor) -> Option<Self> {
None
}

/// Not useful in llvm 3.8.
Expand All @@ -1306,10 +1304,31 @@ impl EvalResult {
#[cfg(not(feature = "llvm_stable"))]
impl EvalResult {
/// Evaluate `cursor` and return the result.
pub fn new(cursor: Cursor) -> Self {
EvalResult {
x: unsafe { clang_Cursor_Evaluate(cursor.x) },
pub fn new(cursor: Cursor) -> Option<Self> {
// Clang has an internal assertion we can trigger if we try to evaluate
// a cursor containing a variadic template type reference. Triggering
// the assertion aborts the process, and we don't want that. Clang
// *also* doesn't expose any API for finding variadic vs non-variadic
// template type references, let alone whether a type referenced is a
// template type, instead they seem to show up as type references to an
// unexposed type. Our solution is to just flat out ban all
// `CXType_Unexposed` from evaluation.
let mut found_cant_eval = false;
cursor.visit(|c| {
if c.kind() == CXCursor_TypeRef && c.cur_type().kind() == CXType_Unexposed {
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW I don't think recursing here is needed given the callers. Probably something like:

if cursor.semantic_parent().kind() == CXCursor_ClassTemplate

Could be enough for our use case, but anyway it's fine.

Please add in the comment the issue number, and add a comment in new saying that this could be relatively expensive to call if mis-called.

r=me in any case. Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

The semantic parent ends up being a CXCursor_VarDecl and then that cursor's semantic parent is a CXCursor_ClassTemplate. I think the existing check is probably better than looking for a VarDecl -> ClassTemplate inheritence, although neither is satisfying.

found_cant_eval = true;
CXChildVisit_Break
} else {
CXChildVisit_Recurse
}
});
if found_cant_eval {
return None;
}

Some(EvalResult {
x: unsafe { clang_Cursor_Evaluate(cursor.x) },
})
}

fn kind(&self) -> Enum_CXEvalResultKind {
Expand Down
7 changes: 4 additions & 3 deletions libbindgen/src/ir/var.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,9 @@ impl ClangSubItemParser for Var {
_ => unreachable!(),
};

let mut val =
cursor.evaluate().as_int().map(|val| val as i64);
let mut val = cursor.evaluate()
.and_then(|v| v.as_int())
.map(|val| val as i64);
if val.is_none() || !kind.signedness_matches(val.unwrap()) {
let tu = ctx.translation_unit();
val = get_integer_literal_from_cursor(&cursor, tu);
Expand All @@ -225,7 +226,7 @@ impl ClangSubItemParser for Var {
})
} else if is_float {
cursor.evaluate()
.as_double()
.and_then(|v| v.as_double())
.map(VarType::Float)
} else {
None
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/* automatically generated by rust-bindgen */


#![allow(non_snake_case)]


#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct B<T> {
pub _address: u8,
pub _phantom_0: ::std::marker::PhantomData<T>,
}
8 changes: 8 additions & 0 deletions libbindgen/tests/headers/eval-variadic-template-parameter.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// bindgen-flags: -- -std=c++11

template <typename... T>
struct B {
// Can't generate anything meaningful in Rust for this, but we shouldn't
// trigger an assertion inside Clang.
static const long c = sizeof...(T);
};