From 9668ddf1f74e94ebcd9dc6c5fadaa6795b07534c Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Mon, 29 Aug 2022 11:37:01 -0500 Subject: [PATCH 01/12] add `is_divergent` field --- src/ir/function.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ir/function.rs b/src/ir/function.rs index e8e2c2dfed..61a9e4ee0e 100644 --- a/src/ir/function.rs +++ b/src/ir/function.rs @@ -228,6 +228,7 @@ pub struct FunctionSig { /// Whether this function is variadic. is_variadic: bool, + is_divergent: bool, /// Whether this function's return value must be used. must_use: bool, @@ -358,6 +359,7 @@ impl FunctionSig { return_type: TypeId, argument_types: Vec<(Option, TypeId)>, is_variadic: bool, + is_divergent: bool, must_use: bool, abi: Abi, ) -> Self { @@ -365,6 +367,7 @@ impl FunctionSig { return_type, argument_types, is_variadic, + is_divergent, must_use, abi, } @@ -528,7 +531,7 @@ impl FunctionSig { warn!("Unknown calling convention: {:?}", call_conv); } - Ok(Self::new(ret, args, ty.is_variadic(), must_use, abi)) + Ok(Self::new(ret, args, ty.is_variadic(), false, must_use, abi)) } /// Get this function signature's return type. From f3e29def485ba57a285357cacc35bcc015185d06 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Wed, 31 Aug 2022 14:38:26 -0500 Subject: [PATCH 02/12] check for noreturn attribute --- src/clang.rs | 18 ++++++++++++++++++ src/codegen/mod.rs | 4 ++++ src/ir/function.rs | 14 +++++++++++++- 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/clang.rs b/src/clang.rs index 00716a1bd5..2ff0ed3f7f 100644 --- a/src/clang.rs +++ b/src/clang.rs @@ -645,6 +645,24 @@ impl Cursor { self.has_attr("warn_unused_result", Some(CXCursor_WarnUnusedResultAttr)) } + pub fn has_no_return_attr(&self) -> bool { + let mut found_attr = false; + self.visit(|cur| { + found_attr = cur.kind() == CXCursor_UnexposedAttr && + cur.tokens().iter().any(|t| { + t.kind == CXToken_Keyword && t.spelling() == b"_Noreturn" + }); + + if found_attr { + CXChildVisit_Break + } else { + CXChildVisit_Continue + } + }); + + found_attr + } + /// Does this cursor have the given attribute? /// /// `name` is checked against unexposed attributes. diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index 77e9ba950d..ecfbb6ad43 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -4704,6 +4704,10 @@ pub mod utils { ctx: &BindgenContext, sig: &FunctionSig, ) -> proc_macro2::TokenStream { + if sig.is_divergent() { + return quote! { -> ! }; + } + let return_item = ctx.resolve_item(sig.return_type()); if let TypeKind::Void = *return_item.kind().expect_type().kind() { quote! {} diff --git a/src/ir/function.rs b/src/ir/function.rs index 61a9e4ee0e..37bafc0eb0 100644 --- a/src/ir/function.rs +++ b/src/ir/function.rs @@ -381,6 +381,7 @@ impl FunctionSig { ) -> Result { use clang_sys::*; debug!("FunctionSig::from_ty {:?} {:?}", ty, cursor); + let is_divergent = cursor.has_no_return_attr(); // Skip function templates let kind = cursor.kind(); @@ -531,7 +532,14 @@ impl FunctionSig { warn!("Unknown calling convention: {:?}", call_conv); } - Ok(Self::new(ret, args, ty.is_variadic(), false, must_use, abi)) + Ok(Self::new( + ret, + args, + ty.is_variadic(), + is_divergent, + must_use, + abi, + )) } /// Get this function signature's return type. @@ -578,6 +586,10 @@ impl FunctionSig { matches!(self.abi, Abi::C | Abi::Unknown(..)) } + + pub(crate) fn is_divergent(&self) -> bool { + self.is_divergent + } } impl ClangSubItemParser for Function { From 6922bf256430e7222f795df048947fec8898a477 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Wed, 31 Aug 2022 14:44:42 -0500 Subject: [PATCH 03/12] add tests --- tests/expectations/tests/noreturn.rs | 13 +++++++++++++ tests/headers/noreturn.h | 3 +++ 2 files changed, 16 insertions(+) create mode 100644 tests/expectations/tests/noreturn.rs create mode 100644 tests/headers/noreturn.h diff --git a/tests/expectations/tests/noreturn.rs b/tests/expectations/tests/noreturn.rs new file mode 100644 index 0000000000..fa81ee7621 --- /dev/null +++ b/tests/expectations/tests/noreturn.rs @@ -0,0 +1,13 @@ +#![allow( + dead_code, + non_snake_case, + non_camel_case_types, + non_upper_case_globals +)] + +extern "C" { + pub fn f() -> !; +} +extern "C" { + pub fn g(); +} diff --git a/tests/headers/noreturn.h b/tests/headers/noreturn.h new file mode 100644 index 0000000000..9ce68518f0 --- /dev/null +++ b/tests/headers/noreturn.h @@ -0,0 +1,3 @@ +_Noreturn void f(void); +// TODO (pvdrz): figure out how to handle this case. +__attribute__((noreturn)) void g(void); From e6ccc73411ae83b9fb116f8c884d2b27d5169aad Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Thu, 1 Sep 2022 10:54:48 -0500 Subject: [PATCH 04/12] add `CxTokenKind` argument to `has_attr` --- src/clang.rs | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/src/clang.rs b/src/clang.rs index 2ff0ed3f7f..d561386a30 100644 --- a/src/clang.rs +++ b/src/clang.rs @@ -642,39 +642,34 @@ impl Cursor { pub fn has_warn_unused_result_attr(&self) -> bool { // FIXME(emilio): clang-sys doesn't expose this (from clang 9). const CXCursor_WarnUnusedResultAttr: CXCursorKind = 440; - self.has_attr("warn_unused_result", Some(CXCursor_WarnUnusedResultAttr)) + self.has_attr( + "warn_unused_result", + Some(CXCursor_WarnUnusedResultAttr), + CXToken_Identifier, + ) } + /// Check wether this cursor has the `_Noreturn` attribute. pub fn has_no_return_attr(&self) -> bool { - let mut found_attr = false; - self.visit(|cur| { - found_attr = cur.kind() == CXCursor_UnexposedAttr && - cur.tokens().iter().any(|t| { - t.kind == CXToken_Keyword && t.spelling() == b"_Noreturn" - }); - - if found_attr { - CXChildVisit_Break - } else { - CXChildVisit_Continue - } - }); - - found_attr + self.has_attr("_Noreturn", None, CXToken_Keyword) } /// Does this cursor have the given attribute? /// /// `name` is checked against unexposed attributes. - fn has_attr(&self, name: &str, clang_kind: Option) -> bool { + fn has_attr( + &self, + name: &str, + clang_kind: Option, + token_kind: CXTokenKind, + ) -> bool { let mut found_attr = false; self.visit(|cur| { let kind = cur.kind(); found_attr = clang_kind.map_or(false, |k| k == kind) || (kind == CXCursor_UnexposedAttr && cur.tokens().iter().any(|t| { - t.kind == CXToken_Identifier && - t.spelling() == name.as_bytes() + t.kind == token_kind && t.spelling() == name.as_bytes() })); if found_attr { From 63dee05798afbb5042ea86d41ec2220a70c550dc Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Thu, 1 Sep 2022 10:59:06 -0500 Subject: [PATCH 05/12] gate `_Noreturn` detection behind `--enable-fucntion-attribute-detection` --- src/ir/function.rs | 4 +++- tests/headers/noreturn.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ir/function.rs b/src/ir/function.rs index 37bafc0eb0..3c4ff2fbc9 100644 --- a/src/ir/function.rs +++ b/src/ir/function.rs @@ -381,7 +381,9 @@ impl FunctionSig { ) -> Result { use clang_sys::*; debug!("FunctionSig::from_ty {:?} {:?}", ty, cursor); - let is_divergent = cursor.has_no_return_attr(); + + let is_divergent = ctx.options().enable_function_attribute_detection && + cursor.has_no_return_attr(); // Skip function templates let kind = cursor.kind(); diff --git a/tests/headers/noreturn.h b/tests/headers/noreturn.h index 9ce68518f0..0e4819e2a4 100644 --- a/tests/headers/noreturn.h +++ b/tests/headers/noreturn.h @@ -1,3 +1,4 @@ +// bindgen-flags: --enable-function-attribute-detection _Noreturn void f(void); // TODO (pvdrz): figure out how to handle this case. __attribute__((noreturn)) void g(void); From 5de42d57978e0eb8430d347e47bb4952363e3ce2 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Thu, 1 Sep 2022 11:26:47 -0500 Subject: [PATCH 06/12] find all attributes in a single pass --- src/clang.rs | 83 ++++++++++++++++++++++++++-------------------- src/ir/function.rs | 14 ++++---- 2 files changed, 55 insertions(+), 42 deletions(-) diff --git a/src/clang.rs b/src/clang.rs index d561386a30..4b946b9201 100644 --- a/src/clang.rs +++ b/src/clang.rs @@ -12,6 +12,27 @@ use std::hash::Hasher; use std::os::raw::{c_char, c_int, c_longlong, c_uint, c_ulong, c_ulonglong}; use std::{mem, ptr, slice}; +pub struct Attribute { + name: &'static [u8], + kind: CXCursorKind, + token_kind: CXTokenKind, +} + +impl Attribute { + pub const MUST_USE: Self = Self { + name: b"warn_unused_result", + // FIXME(emilio): clang-sys doesn't expose `CXCursor_WarnUnusedResultAttr` (from clang 9). + kind: 440, + token_kind: CXToken_Identifier, + }; + + pub const NO_RETURN: Self = Self { + name: b"_Noreturn", + kind: CXCursor_UnexposedAttr, + token_kind: CXToken_Keyword, + }; +} + /// A cursor into the Clang AST, pointing to an AST node. /// /// We call the AST node pointed to by the cursor the cursor's "referent". @@ -638,48 +659,38 @@ impl Cursor { } } - /// Whether this cursor has the `warn_unused_result` attribute. - pub fn has_warn_unused_result_attr(&self) -> bool { - // FIXME(emilio): clang-sys doesn't expose this (from clang 9). - const CXCursor_WarnUnusedResultAttr: CXCursorKind = 440; - self.has_attr( - "warn_unused_result", - Some(CXCursor_WarnUnusedResultAttr), - CXToken_Identifier, - ) - } - - /// Check wether this cursor has the `_Noreturn` attribute. - pub fn has_no_return_attr(&self) -> bool { - self.has_attr("_Noreturn", None, CXToken_Keyword) - } - - /// Does this cursor have the given attribute? - /// - /// `name` is checked against unexposed attributes. - fn has_attr( + pub fn has_attrs( &self, - name: &str, - clang_kind: Option, - token_kind: CXTokenKind, - ) -> bool { - let mut found_attr = false; + attrs: &[Attribute; N], + ) -> [bool; N] { + let mut found_attrs = [false; N]; + let mut found_count = 0; + self.visit(|cur| { let kind = cur.kind(); - found_attr = clang_kind.map_or(false, |k| k == kind) || - (kind == CXCursor_UnexposedAttr && - cur.tokens().iter().any(|t| { - t.kind == token_kind && t.spelling() == name.as_bytes() - })); - - if found_attr { - CXChildVisit_Break - } else { - CXChildVisit_Continue + for (idx, attr) in attrs.iter().enumerate() { + let found_attr = &mut found_attrs[idx]; + if !*found_attr { + if kind == attr.kind && + cur.tokens().iter().any(|t| { + t.kind == attr.token_kind && + t.spelling() == attr.name + }) + { + *found_attr = true; + found_count += 1; + + if found_count == N { + return CXChildVisit_Break; + } + } + } } + + CXChildVisit_Continue }); - found_attr + found_attrs } /// Given that this cursor's referent is a `typedef`, get the `Type` that is diff --git a/src/ir/function.rs b/src/ir/function.rs index 3c4ff2fbc9..f6b2d4c02b 100644 --- a/src/ir/function.rs +++ b/src/ir/function.rs @@ -6,7 +6,7 @@ use super::dot::DotAttributes; use super::item::Item; use super::traversal::{EdgeKind, Trace, Tracer}; use super::ty::TypeKind; -use crate::clang; +use crate::clang::{self, Attribute}; use crate::parse::{ ClangItemParser, ClangSubItemParser, ParseError, ParseResult, }; @@ -382,9 +382,6 @@ impl FunctionSig { use clang_sys::*; debug!("FunctionSig::from_ty {:?} {:?}", ty, cursor); - let is_divergent = ctx.options().enable_function_attribute_detection && - cursor.has_no_return_attr(); - // Skip function templates let kind = cursor.kind(); if kind == CXCursor_FunctionTemplate { @@ -453,8 +450,13 @@ impl FunctionSig { } }; - let must_use = ctx.options().enable_function_attribute_detection && - cursor.has_warn_unused_result_attr(); + let [must_use, is_divergent] = + if ctx.options().enable_function_attribute_detection { + cursor.has_attrs(&[Attribute::MUST_USE, Attribute::NO_RETURN]) + } else { + Default::default() + }; + let is_method = kind == CXCursor_CXXMethod; let is_constructor = kind == CXCursor_Constructor; let is_destructor = kind == CXCursor_Destructor; From fe4b1c02fb95ca8f76f41e094f59865cf9f0b461 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Thu, 1 Sep 2022 12:16:43 -0500 Subject: [PATCH 07/12] document `Attribute` --- src/clang.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/clang.rs b/src/clang.rs index 4b946b9201..d1a72f6763 100644 --- a/src/clang.rs +++ b/src/clang.rs @@ -12,6 +12,10 @@ use std::hash::Hasher; use std::os::raw::{c_char, c_int, c_longlong, c_uint, c_ulong, c_ulonglong}; use std::{mem, ptr, slice}; +/// Type representing a clang attribute. +/// +/// Values of this type can be used to check for different attributes using the `has_attrs` +/// function. pub struct Attribute { name: &'static [u8], kind: CXCursorKind, @@ -19,6 +23,7 @@ pub struct Attribute { } impl Attribute { + /// A `warn_unused_result` attribute. pub const MUST_USE: Self = Self { name: b"warn_unused_result", // FIXME(emilio): clang-sys doesn't expose `CXCursor_WarnUnusedResultAttr` (from clang 9). @@ -26,6 +31,7 @@ impl Attribute { token_kind: CXToken_Identifier, }; + /// A `_Noreturn` attribute. pub const NO_RETURN: Self = Self { name: b"_Noreturn", kind: CXCursor_UnexposedAttr, From d15c37c3cfcd4dfc87e1e7c9770e9d784e3a82be Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Thu, 1 Sep 2022 12:17:36 -0500 Subject: [PATCH 08/12] document `has_attrs` --- src/clang.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/clang.rs b/src/clang.rs index d1a72f6763..896283ac64 100644 --- a/src/clang.rs +++ b/src/clang.rs @@ -665,6 +665,7 @@ impl Cursor { } } + /// Does this cursor have the given attributes? pub fn has_attrs( &self, attrs: &[Attribute; N], From 0c74f4cfe1bba67beb86f79123dfdb7094b6311c Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Thu, 1 Sep 2022 12:36:11 -0500 Subject: [PATCH 09/12] bring back optional cursor kind --- src/clang.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/clang.rs b/src/clang.rs index 896283ac64..b2164284b3 100644 --- a/src/clang.rs +++ b/src/clang.rs @@ -18,7 +18,7 @@ use std::{mem, ptr, slice}; /// function. pub struct Attribute { name: &'static [u8], - kind: CXCursorKind, + kind: Option, token_kind: CXTokenKind, } @@ -27,14 +27,14 @@ impl Attribute { pub const MUST_USE: Self = Self { name: b"warn_unused_result", // FIXME(emilio): clang-sys doesn't expose `CXCursor_WarnUnusedResultAttr` (from clang 9). - kind: 440, + kind: Some(440), token_kind: CXToken_Identifier, }; /// A `_Noreturn` attribute. pub const NO_RETURN: Self = Self { name: b"_Noreturn", - kind: CXCursor_UnexposedAttr, + kind: None, token_kind: CXToken_Keyword, }; } @@ -678,11 +678,13 @@ impl Cursor { for (idx, attr) in attrs.iter().enumerate() { let found_attr = &mut found_attrs[idx]; if !*found_attr { - if kind == attr.kind && - cur.tokens().iter().any(|t| { - t.kind == attr.token_kind && - t.spelling() == attr.name - }) + // `attr.name` and` attr.token_kind` are checked against unexposed attributes only. + if attr.kind.map_or(false, |k| k == kind) || + (kind == CXCursor_UnexposedAttr && + cur.tokens().iter().any(|t| { + t.kind == attr.token_kind && + t.spelling() == attr.name + })) { *found_attr = true; found_count += 1; From 363d341e3ab859627cca19794bd20fc2295cbc5c Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Thu, 1 Sep 2022 15:59:37 -0500 Subject: [PATCH 10/12] handle c++ `[[noreturn]]` attribute --- src/clang.rs | 7 +++++++ src/ir/function.rs | 9 +++++++-- tests/expectations/tests/noreturn.rs | 10 ++++++++-- tests/headers/{noreturn.h => noreturn.hpp} | 1 + 4 files changed, 23 insertions(+), 4 deletions(-) rename tests/headers/{noreturn.h => noreturn.hpp} (86%) diff --git a/src/clang.rs b/src/clang.rs index b2164284b3..1cf4dd0d0d 100644 --- a/src/clang.rs +++ b/src/clang.rs @@ -37,6 +37,13 @@ impl Attribute { kind: None, token_kind: CXToken_Keyword, }; + + /// A `[[noreturn]]` attribute. + pub const NO_RETURN_CPP: Self = Self { + name: b"noreturn", + kind: None, + token_kind: CXToken_Identifier, + }; } /// A cursor into the Clang AST, pointing to an AST node. diff --git a/src/ir/function.rs b/src/ir/function.rs index f6b2d4c02b..b76f066a93 100644 --- a/src/ir/function.rs +++ b/src/ir/function.rs @@ -450,9 +450,14 @@ impl FunctionSig { } }; - let [must_use, is_divergent] = + let (must_use, is_divergent) = if ctx.options().enable_function_attribute_detection { - cursor.has_attrs(&[Attribute::MUST_USE, Attribute::NO_RETURN]) + let [must_use, no_return, no_return_cpp] = cursor.has_attrs(&[ + Attribute::MUST_USE, + Attribute::NO_RETURN, + Attribute::NO_RETURN_CPP, + ]); + (must_use, no_return || no_return_cpp) } else { Default::default() }; diff --git a/tests/expectations/tests/noreturn.rs b/tests/expectations/tests/noreturn.rs index fa81ee7621..1ffce9c060 100644 --- a/tests/expectations/tests/noreturn.rs +++ b/tests/expectations/tests/noreturn.rs @@ -6,8 +6,14 @@ )] extern "C" { - pub fn f() -> !; + #[link_name = "\u{1}_Z1fv"] + pub fn f() -> !; } extern "C" { - pub fn g(); + #[link_name = "\u{1}_Z1gv"] + pub fn g(); +} +extern "C" { + #[link_name = "\u{1}_Z1hv"] + pub fn h() -> !; } diff --git a/tests/headers/noreturn.h b/tests/headers/noreturn.hpp similarity index 86% rename from tests/headers/noreturn.h rename to tests/headers/noreturn.hpp index 0e4819e2a4..deaa3b1ace 100644 --- a/tests/headers/noreturn.h +++ b/tests/headers/noreturn.hpp @@ -2,3 +2,4 @@ _Noreturn void f(void); // TODO (pvdrz): figure out how to handle this case. __attribute__((noreturn)) void g(void); +[[noreturn]] void h(void); From 7679e53f2f33f6da55f4df305d3b8ae1ebc2d20f Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Thu, 1 Sep 2022 16:11:05 -0500 Subject: [PATCH 11/12] handle `__attribute__((noreturn))` attribute --- src/ir/function.rs | 7 ++++++- tests/expectations/tests/noreturn.rs | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/ir/function.rs b/src/ir/function.rs index b76f066a93..48ad73e8f0 100644 --- a/src/ir/function.rs +++ b/src/ir/function.rs @@ -450,7 +450,7 @@ impl FunctionSig { } }; - let (must_use, is_divergent) = + let (must_use, mut is_divergent) = if ctx.options().enable_function_attribute_detection { let [must_use, no_return, no_return_cpp] = cursor.has_attrs(&[ Attribute::MUST_USE, @@ -462,6 +462,11 @@ impl FunctionSig { Default::default() }; + // This looks easy to break but the clang parser keeps the type spelling clean even if + // other attributes are added. + is_divergent = + is_divergent || ty.spelling().contains("__attribute__((noreturn))"); + let is_method = kind == CXCursor_CXXMethod; let is_constructor = kind == CXCursor_Constructor; let is_destructor = kind == CXCursor_Destructor; diff --git a/tests/expectations/tests/noreturn.rs b/tests/expectations/tests/noreturn.rs index 1ffce9c060..1945749518 100644 --- a/tests/expectations/tests/noreturn.rs +++ b/tests/expectations/tests/noreturn.rs @@ -11,7 +11,7 @@ extern "C" { } extern "C" { #[link_name = "\u{1}_Z1gv"] - pub fn g(); + pub fn g() -> !; } extern "C" { #[link_name = "\u{1}_Z1hv"] From df5caad09c76e59561e48254a9adbcdac3123ed2 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Thu, 1 Sep 2022 16:24:38 -0500 Subject: [PATCH 12/12] add the `-- -std=c++11` flag --- tests/headers/noreturn.hpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/headers/noreturn.hpp b/tests/headers/noreturn.hpp index deaa3b1ace..4ce1e11e3f 100644 --- a/tests/headers/noreturn.hpp +++ b/tests/headers/noreturn.hpp @@ -1,5 +1,4 @@ -// bindgen-flags: --enable-function-attribute-detection +// bindgen-flags: --enable-function-attribute-detection -- -std=c++11 _Noreturn void f(void); -// TODO (pvdrz): figure out how to handle this case. __attribute__((noreturn)) void g(void); [[noreturn]] void h(void);