Skip to content

Commit a00db04

Browse files
author
bors-servo
authored
Auto merge of #799 - emilio:comment-indent, r=fitzgen
codegen: Make comments indentation-aware. This commit moves comment processing to a central place (well, two, because of field docs, but that's fine). Also, it makes comments indentation aware, so multiline comments don't appear garbled. Finally, it also fixes an out-of-bounds panic when processing an empty multiline comment.
2 parents 02afb5b + 96304f9 commit a00db04

File tree

13 files changed

+281
-81
lines changed

13 files changed

+281
-81
lines changed

‎Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ name = "bindgen"
1313
readme = "README.md"
1414
repository = "https://github.com/servo/rust-bindgen"
1515
documentation = "https://docs.rs/bindgen"
16-
version = "0.26.3"
16+
version = "0.27.0"
1717
build = "build.rs"
1818

1919
exclude = [

‎src/codegen/helpers.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ pub mod attributes {
3030
aster::AstBuilder::new().attr().word("inline")
3131
}
3232

33-
pub fn doc(comment: &str) -> ast::Attribute {
34-
aster::AstBuilder::new().attr().doc(comment)
33+
pub fn doc(comment: String) -> ast::Attribute {
34+
aster::AstBuilder::new().attr().doc(&*comment)
3535
}
3636

3737
pub fn link_name(name: &str) -> ast::Attribute {

‎src/codegen/mod.rs

Lines changed: 24 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,14 @@ use aster::struct_field::StructFieldBuilder;
1111
use ir::annotations::FieldAccessorKind;
1212
use ir::comp::{Base, BitfieldUnit, Bitfield, CompInfo, CompKind, Field,
1313
FieldData, FieldMethods, Method, MethodKind};
14+
use ir::comment;
1415
use ir::context::{BindgenContext, ItemId};
1516
use ir::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault};
1617
use ir::dot;
1718
use ir::enum_ty::{Enum, EnumVariant, EnumVariantValue};
1819
use ir::function::{Abi, Function, FunctionSig};
1920
use ir::int::IntKind;
20-
use ir::item::{IsOpaque, Item, ItemAncestors, ItemCanonicalName,
21-
ItemCanonicalPath, ItemSet};
21+
use ir::item::{IsOpaque, Item, ItemCanonicalName, ItemCanonicalPath, ItemSet};
2222
use ir::item_kind::ItemKind;
2323
use ir::layout::Layout;
2424
use ir::module::Module;
@@ -42,23 +42,13 @@ use syntax::ptr::P;
4242
// Name of type defined in constified enum module
4343
pub static CONSTIFIED_ENUM_MODULE_REPR_NAME: &'static str = "Type";
4444

45-
fn root_import_depth(ctx: &BindgenContext, item: &Item) -> usize {
46-
if !ctx.options().enable_cxx_namespaces {
47-
return 0;
48-
}
49-
50-
item.ancestors(ctx)
51-
.filter(|id| ctx.resolve_item(*id).is_module())
52-
.fold(1, |i, _| i + 1)
53-
}
54-
5545
fn top_level_path(ctx: &BindgenContext, item: &Item) -> Vec<ast::Ident> {
5646
let mut path = vec![ctx.rust_ident_raw("self")];
5747

5848
if ctx.options().enable_cxx_namespaces {
5949
let super_ = ctx.rust_ident_raw("super");
6050

61-
for _ in 0..root_import_depth(ctx, item) {
51+
for _ in 0..item.codegen_depth(ctx) {
6252
path.push(super_.clone());
6353
}
6454
}
@@ -616,10 +606,8 @@ impl CodeGenerator for Type {
616606
let rust_name = ctx.rust_ident(&name);
617607
let mut typedef = aster::AstBuilder::new().item().pub_();
618608

619-
if ctx.options().generate_comments {
620-
if let Some(comment) = item.comment() {
621-
typedef = typedef.attr().doc(comment);
622-
}
609+
if let Some(comment) = item.comment(ctx) {
610+
typedef = typedef.with_attr(attributes::doc(comment));
623611
}
624612

625613
// We prefer using `pub use` over `pub type` because of:
@@ -839,6 +827,7 @@ trait FieldCodegen<'a> {
839827
fn codegen<F, M>(&self,
840828
ctx: &BindgenContext,
841829
fields_should_be_private: bool,
830+
codegen_depth: usize,
842831
accessor_kind: FieldAccessorKind,
843832
parent: &CompInfo,
844833
anon_field_names: &mut AnonFieldNames,
@@ -857,6 +846,7 @@ impl<'a> FieldCodegen<'a> for Field {
857846
fn codegen<F, M>(&self,
858847
ctx: &BindgenContext,
859848
fields_should_be_private: bool,
849+
codegen_depth: usize,
860850
accessor_kind: FieldAccessorKind,
861851
parent: &CompInfo,
862852
anon_field_names: &mut AnonFieldNames,
@@ -872,6 +862,7 @@ impl<'a> FieldCodegen<'a> for Field {
872862
Field::DataMember(ref data) => {
873863
data.codegen(ctx,
874864
fields_should_be_private,
865+
codegen_depth,
875866
accessor_kind,
876867
parent,
877868
anon_field_names,
@@ -884,6 +875,7 @@ impl<'a> FieldCodegen<'a> for Field {
884875
Field::Bitfields(ref unit) => {
885876
unit.codegen(ctx,
886877
fields_should_be_private,
878+
codegen_depth,
887879
accessor_kind,
888880
parent,
889881
anon_field_names,
@@ -903,6 +895,7 @@ impl<'a> FieldCodegen<'a> for FieldData {
903895
fn codegen<F, M>(&self,
904896
ctx: &BindgenContext,
905897
fields_should_be_private: bool,
898+
codegen_depth: usize,
906899
accessor_kind: FieldAccessorKind,
907900
parent: &CompInfo,
908901
anon_field_names: &mut AnonFieldNames,
@@ -945,8 +938,9 @@ impl<'a> FieldCodegen<'a> for FieldData {
945938

946939
let mut attrs = vec![];
947940
if ctx.options().generate_comments {
948-
if let Some(comment) = self.comment() {
949-
attrs.push(attributes::doc(comment));
941+
if let Some(raw_comment) = self.comment() {
942+
let comment = comment::preprocess(raw_comment, codegen_depth + 1);
943+
attrs.push(attributes::doc(comment))
950944
}
951945
}
952946

@@ -1153,6 +1147,7 @@ impl<'a> FieldCodegen<'a> for BitfieldUnit {
11531147
fn codegen<F, M>(&self,
11541148
ctx: &BindgenContext,
11551149
fields_should_be_private: bool,
1150+
codegen_depth: usize,
11561151
accessor_kind: FieldAccessorKind,
11571152
parent: &CompInfo,
11581153
anon_field_names: &mut AnonFieldNames,
@@ -1197,6 +1192,7 @@ impl<'a> FieldCodegen<'a> for BitfieldUnit {
11971192
for bf in self.bitfields() {
11981193
bf.codegen(ctx,
11991194
fields_should_be_private,
1195+
codegen_depth,
12001196
accessor_kind,
12011197
parent,
12021198
anon_field_names,
@@ -1277,6 +1273,7 @@ impl<'a> FieldCodegen<'a> for Bitfield {
12771273
fn codegen<F, M>(&self,
12781274
ctx: &BindgenContext,
12791275
_fields_should_be_private: bool,
1276+
_codegen_depth: usize,
12801277
_accessor_kind: FieldAccessorKind,
12811278
parent: &CompInfo,
12821279
_anon_field_names: &mut AnonFieldNames,
@@ -1409,10 +1406,8 @@ impl CodeGenerator for CompInfo {
14091406
let mut attributes = vec![];
14101407
let mut needs_clone_impl = false;
14111408
let mut needs_default_impl = false;
1412-
if ctx.options().generate_comments {
1413-
if let Some(comment) = item.comment() {
1414-
attributes.push(attributes::doc(comment));
1415-
}
1409+
if let Some(comment) = item.comment(ctx) {
1410+
attributes.push(attributes::doc(comment));
14161411
}
14171412
if self.packed() {
14181413
attributes.push(attributes::repr_list(&["C", "packed"]));
@@ -1545,9 +1540,11 @@ impl CodeGenerator for CompInfo {
15451540

15461541
let mut methods = vec![];
15471542
let mut anon_field_names = AnonFieldNames::default();
1543+
let codegen_depth = item.codegen_depth(ctx);
15481544
for field in self.fields() {
15491545
field.codegen(ctx,
15501546
fields_should_be_private,
1547+
codegen_depth,
15511548
struct_accessor_kind,
15521549
self,
15531550
&mut anon_field_names,
@@ -2367,10 +2364,8 @@ impl CodeGenerator for Enum {
23672364
builder = builder.with_attr(attributes::repr("C"));
23682365
}
23692366

2370-
if ctx.options().generate_comments {
2371-
if let Some(comment) = item.comment() {
2372-
builder = builder.with_attr(attributes::doc(comment));
2373-
}
2367+
if let Some(comment) = item.comment(ctx) {
2368+
builder = builder.with_attr(attributes::doc(comment));
23742369
}
23752370

23762371
if !is_constified_enum {
@@ -3069,10 +3064,8 @@ impl CodeGenerator for Function {
30693064

30703065
let mut attributes = vec![];
30713066

3072-
if ctx.options().generate_comments {
3073-
if let Some(comment) = item.comment() {
3074-
attributes.push(attributes::doc(comment));
3075-
}
3067+
if let Some(comment) = item.comment(ctx) {
3068+
attributes.push(attributes::doc(comment));
30763069
}
30773070

30783071
if let Some(mangled) = mangled_name {

‎src/ir/comment.rs

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
//! Utilities for manipulating C/C++ comments.
22
3+
use std::iter;
4+
35
/// The type of a comment.
46
#[derive(Debug, PartialEq, Eq)]
57
enum Kind {
@@ -10,11 +12,12 @@ enum Kind {
1012
/// entire block ends with `*/`.
1113
MultiLine,
1214
}
15+
1316
/// Preprocesses a C/C++ comment so that it is a valid Rust comment.
14-
pub fn preprocess(comment: String) -> String {
17+
pub fn preprocess(comment: &str, indent: usize) -> String {
1518
match self::kind(&comment) {
16-
Some(Kind::SingleLines) => preprocess_single_lines(&comment),
17-
Some(Kind::MultiLine) => preprocess_multi_line(&comment),
19+
Some(Kind::SingleLines) => preprocess_single_lines(comment, indent),
20+
Some(Kind::MultiLine) => preprocess_multi_line(comment, indent),
1821
None => comment.to_owned(),
1922
}
2023
}
@@ -30,38 +33,59 @@ fn kind(comment: &str) -> Option<Kind> {
3033
}
3134
}
3235

36+
fn make_indent(indent: usize) -> String {
37+
const RUST_INDENTATION: usize = 4;
38+
39+
iter::repeat(' ').take(indent * RUST_INDENTATION).collect()
40+
}
41+
3342
/// Preprocesses mulitple single line comments.
3443
///
3544
/// Handles lines starting with both `//` and `///`.
36-
fn preprocess_single_lines(comment: &str) -> String {
37-
assert!(comment.starts_with("//"), "comment is not single line");
45+
fn preprocess_single_lines(comment: &str, indent: usize) -> String {
46+
debug_assert!(comment.starts_with("//"), "comment is not single line");
3847

48+
let indent = make_indent(indent);
49+
let mut is_first = true;
3950
let lines: Vec<_> = comment.lines()
40-
.map(|l| l.trim_left_matches('/').trim())
41-
.map(|l| format!("/// {}", l).trim().to_owned())
42-
.collect();
51+
.map(|l| l.trim_left_matches('/').trim())
52+
.map(|l| {
53+
let indent = if is_first { "" } else { &*indent };
54+
is_first = false;
55+
let maybe_space = if l.is_empty() { "" } else { " " };
56+
format!("{}///{}{}", indent, maybe_space, l)
57+
})
58+
.collect();
4359
lines.join("\n")
4460
}
4561

46-
fn preprocess_multi_line(comment: &str) -> String {
62+
fn preprocess_multi_line(comment: &str, indent: usize) -> String {
4763
let comment = comment.trim_left_matches('/')
48-
.trim_left_matches("*")
49-
.trim_left_matches("!")
64+
.trim_left_matches('*')
65+
.trim_left_matches('!')
5066
.trim_right_matches('/')
5167
.trim_right_matches('*')
5268
.trim();
5369

70+
let indent = make_indent(indent);
5471
// Strip any potential `*` characters preceding each line.
72+
let mut is_first = true;
5573
let mut lines: Vec<_> = comment.lines()
5674
.map(|line| line.trim().trim_left_matches('*').trim())
5775
.skip_while(|line| line.is_empty()) // Skip the first empty lines.
58-
.map(|line| format!("/// {}", line).trim().to_owned())
76+
.map(|line| {
77+
let indent = if is_first { "" } else { &*indent };
78+
is_first = false;
79+
let maybe_space = if line.is_empty() { "" } else { " " };
80+
format!("{}///{}{}", indent, maybe_space, line)
81+
})
5982
.collect();
6083

61-
// Remove the trailing `*/`.
62-
let last_idx = lines.len() - 1;
63-
if lines[last_idx].is_empty() {
64-
lines.remove(last_idx);
84+
// Remove the trailing line corresponding to the `*/`.
85+
let last_line_is_empty = lines.last().map_or(false, |l| l.is_empty());
86+
87+
if last_line_is_empty {
88+
lines.pop();
6589
}
6690

6791
lines.join("\n")
@@ -79,16 +103,16 @@ mod test {
79103

80104
#[test]
81105
fn processes_single_lines_correctly() {
82-
assert_eq!(preprocess("/// hello".to_owned()), "/// hello");
83-
assert_eq!(preprocess("// hello".to_owned()), "/// hello");
106+
assert_eq!(preprocess("/// hello", 0), "/// hello");
107+
assert_eq!(preprocess("// hello", 0), "/// hello");
84108
}
85109

86110
#[test]
87111
fn processes_multi_lines_correctly() {
88-
assert_eq!(preprocess("/** hello \n * world \n * foo \n */".to_owned()),
112+
assert_eq!(preprocess("/** hello \n * world \n * foo \n */", 0),
89113
"/// hello\n/// world\n/// foo");
90114

91-
assert_eq!(preprocess("/**\nhello\n*world\n*foo\n*/".to_owned()),
115+
assert_eq!(preprocess("/**\nhello\n*world\n*foo\n*/", 0),
92116
"/// hello\n/// world\n/// foo");
93117
}
94118
}

‎src/ir/comp.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//! Compound types (unions and structs) in our intermediate representation.
22
33
use super::annotations::Annotations;
4-
use super::comment;
54
use super::context::{BindgenContext, ItemId};
65
use super::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault};
76
use super::dot::DotAttributes;
@@ -1102,7 +1101,7 @@ impl CompInfo {
11021101
Some(potential_id),
11031102
ctx);
11041103

1105-
let comment = cur.raw_comment().map(comment::preprocess);
1104+
let comment = cur.raw_comment();
11061105
let annotations = Annotations::new(&cur);
11071106
let name = cur.spelling();
11081107
let is_mutable = cursor.is_mutable_field();

‎src/ir/enum_ty.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
//! Intermediate representation for C/C++ enumerations.
22
3-
use super::comment;
43
use super::context::{BindgenContext, ItemId};
54
use super::item::Item;
65
use super::ty::TypeKind;
@@ -114,7 +113,7 @@ impl Enum {
114113
})
115114
});
116115

117-
let comment = cursor.raw_comment().map(comment::preprocess);
116+
let comment = cursor.raw_comment();
118117
variants.push(EnumVariant::new(name,
119118
comment,
120119
val,

‎src/ir/function.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
//! Intermediate representation for C/C++ functions and methods.
22
3-
use super::comment;
43
use super::context::{BindgenContext, ItemId};
54
use super::dot::DotAttributes;
65
use super::item::Item;
@@ -406,7 +405,7 @@ impl ClangSubItemParser for Function {
406405
mangled_name = None;
407406
}
408407

409-
let comment = cursor.raw_comment().map(comment::preprocess);
408+
let comment = cursor.raw_comment();
410409

411410
let function = Self::new(name, mangled_name, sig, comment);
412411
Ok(ParseResult::New(function, Some(cursor)))

0 commit comments

Comments
 (0)