Skip to content
Open
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
26 changes: 19 additions & 7 deletions bitbybit/src/bitfield/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,14 @@ pub fn make_builder(
.filter(|def| def.setter_type.is_some());
let params = definitions
.clone()
.map(|def| syn::parse_str::<Ident>(format!("{}", def.field_name).as_str()).unwrap())
.map(|def| {
// Special handling for reserved identifiers prefix.
let mut field_name = def.field_name.to_string();
if field_name.starts_with("r#") {
field_name = field_name.strip_prefix("r#").unwrap().to_string();
}
syn::parse_str::<Ident>(&format!("__{}", field_name.to_uppercase())).unwrap()
Comment on lines +463 to +468
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this duplicated logic be moved to a separate function and called here and in 556?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

probably, yes..

})
.map(|name| quote! { const #name: bool })
.collect::<Vec<_>>();

Expand Down Expand Up @@ -545,19 +552,24 @@ pub fn make_builder(
result.push(quote!(false));
any_overlaps = true;
} else {
let name = syn::parse_str::<Ident>(format!("{}", def.field_name).as_str())
.unwrap();
params.push(quote! { const #name: bool });
names.push(quote!({ #name }));
result.push(quote!({ #name }));
// Special handling for reserved identifiers prefix.
let mut field_name = def.field_name.to_string();
if field_name.starts_with("r#") {
field_name = field_name.strip_prefix("r#").unwrap().to_string();
}
let const_generics =
syn::parse_str::<Ident>(&format!("__{}", field_name.to_uppercase()))
.unwrap();
params.push(quote! { const #const_generics: bool });
names.push(quote!({ #const_generics }));
result.push(quote!({ #const_generics }));
}
}
}
set_params.insert(builder_params);

let doc_comment = &field_definition.doc_comment;
new_with_builder_chain.push(quote! {
#[allow(non_camel_case_types)]
impl<#( #params, )*> #builder_struct_name<#( #names, )*> {
#(#doc_comment)*
pub const fn #with_name(&self, value: #argument_type) -> #builder_struct_name<#( #result, )*> {
Expand Down
10 changes: 10 additions & 0 deletions bitbybit/tests/builder_value_field.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// No issues with the private internal field value when a field is named value.
#[bitbybit::bitfield(u32, default = 0x0, debug)]
pub struct Data {
#[bit(15, rw)]
dparity: bool,
#[bits(0..=7, rw)]
value: u8,
}

pub fn main() {}
2 changes: 1 addition & 1 deletion bitbybit/tests/no_compile/invalid_field_range.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ error[E0599]: no method named `with_c` found for struct `PartialTest<true, true,
| ^^^^^^ method not found in `PartialTest<true, true, false>`
|
= note: the method was found for
- `PartialTest<false, b, false>`
- `PartialTest<false, __B, false>`
help: one of the expressions' fields has a method of the same name
|
30 | Test::builder().with_a(u4::new(1)).with_b(u4::new(1)).value.with_c(true).build();
Expand Down
2 changes: 1 addition & 1 deletion bitbybit/tests/no_compile/missing_fields_in_builder.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ error[E0599]: no method named `with_bar` found for struct `PartialTest<true, tru
| ^^^^^^^^ method not found in `PartialTest<true, true>`
|
= note: the method was found for
- `PartialTest<foo, false>`
- `PartialTest<__FOO, false>`
help: one of the expressions' fields has a method of the same name
|
15 | Test::builder().with_bar(1).with_foo(1).value.with_bar(2).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ error[E0599]: no method named `with_a` found for struct `PartialTestAllowed<true
| ^^^^^^ method not found in `PartialTestAllowed<true, true, false, false, false>`
|
= note: the method was found for
- `PartialTestAllowed<upper, false, c, b, false>`
- `PartialTestAllowed<__UPPER, false, __C, __B, false>`
Copy link
Collaborator

Choose a reason for hiding this comment

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

The original approach was explicitly to try and keep the output in the diagnostics as close to the user's code as possible, so changing their casing is unfortunate. We must address the issue with clashing with field names, of course, but we could handle that by exclusively changing value and nothing else, right? And at that point we'd have to check that the transformation target (lets say, VALUE) doesn't already exist as another const param...

Copy link
Collaborator Author

@robamu robamu Mar 13, 2026

Choose a reason for hiding this comment

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

that would be a solution, but I think the diagnostics are still sufficient to know what the issue is. I can still do the special case solution. alternatively: what do you think about renaming the value field itself to __value or __private_value?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@estebank could you have a look at #131

help: one of the expressions' fields has a method of the same name
|
48 | TestAllowed::builder().with_upper(u4::new(0)).with_lower(u12::new(0)).value.with_a(u2::new(0)).build();
Expand Down
1 change: 1 addition & 0 deletions bitbybit/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ fn all_tests() {
t.pass("tests/basic.rs");
t.pass("tests/with_fields.rs");
t.pass("tests/correct.rs");
t.pass("tests/builder_value_field.rs");

t.compile_fail("tests/no_compile/exhaustive_bitenum.rs");
t.compile_fail("tests/no_compile/non_exhaustive_bitenum.rs");
Expand Down
Loading