Removes the unused braces warning in the autogenerated From implementations. #68
Conversation
Previously, `From` implementations may have expanded to code similar to the
following:
```
...
where
[(); { Size }]:
::modular_bitfield::private::IsU32Compatible,
...
```
Where `Size` is a constant expression that reduces to a `usize`.
With `Size` being an expression and the context in which it is used, the braces
around it are unneeded, producing a warning.
The braces are inserted in `impl::bitfield::expand::generate_bitfield_size`,
surrounding the returned sum-expression.
To avoid the warning, `generate_bitfield_size` was modified to return a naked
expression.
Places where brackets were needed to avoid an incorrect reading of the
expression were modified to add the brackets in place.
Similarly, places where the brackets were not needed were simplified.
diseraluca
changed the title
The modular bitfield crate produces an unneccesary braces warning when `From` implementations are generated. See: Robbepop/modular-bitfield#66 A pull request ( Robbepop/modular-bitfield#68 ) to resolve this was provided. Since it is not yet accepted mainstream, the `modular-bitfield` dependency was temporarily moved to a custom fork to move forward with the removal of compilation warnings.
There was a problem hiding this comment.
Thanks for the PR!
I think it is really important to silence this annoying warning.
Could you please try to add a parentheses around #lhs + #rhs in the generate_bitfield_size function just to be sure about expression ordering. This should ideally not result in warnings anymore but still be more explicit about expression ordering (not relying on implicit orderings).
Furthermore, the documentation for the function was modified to be consistently aligned.
Sorry I've been sloppy with this as I wasn't at my usual computer and tested only on a small I've now tested with my library and, indeed, the parenthesis generate a warning. This was, for now, reverted in b9b81df. A similar case to the one for the from implementation was identified in Furthermore, testing in another library I identified a case that was broken by the original change, in I expect a warning to be generated by the new parenthesization when |
Recently, `generate_bitfield_size` was modified to return an expression not contained in a block, to avoid generating an `unused_braces` warning. See: Robbepop#66 To simplify the reading of the generated code and avoid ambiguities with operators' precedence, further parenthesization was added to the expression itself. The new parenthesization generates an `unused_parenthesis` warning and is now removed.
Similarly to the recent changes to `generate_bitfield_size`, the block surrounding the expression returned by `next_divisible_by_8` was removed to avoid the generation of an `unused_braces` warning.
After the recent changes to `generate_bitfield_size`, the returned expression is not enclosed in a block. In `expand_byte_conversion_impls`, when `filled` is not enabled, an expression of the following form is generated: ``` if bytes[...#next_divisible_by_8 - #size))) ``` Where `#size` is a value returned by `generate_bitfield_size`. Since `#size` may take the form of a sum, it is now possible to generate an expression of the form `#next_divisible_by_8 - size_1 + size_2 + ... + size_n` which is not equivalent to the previous behavior, where `#size` was first reduced to a single value and the subtracted, and results in a bigger result. To avoid this issue, `#size` is now parenthesized in the expression to perform the correct reduction.
|
I can't find a way to suppress this short of adding There are also unused warnings generated. For this I had to add For now I'll use the bitfield crate instead. I would be happy to use this instead if it looks like it is being actively maintained. I think the API is quite good. |
Resolves #66.
Previously,
Fromimplementations may have expanded to code similar to thefollowing:
Where
Sizeis a constant expression that reduces to ausize.With
Sizebeing an expression and the context in which it is used, the bracesaround it are unneeded, producing a warning.
The braces are inserted in
impl::bitfield::expand::generate_bitfield_size,surrounding the returned sum-expression.
To avoid the warning,
generate_bitfield_sizewas modified to return a nakedexpression.
Places where brackets were needed to avoid an incorrect reading of the
expression were modified to add the brackets in place.
Similarly, places where the brackets were not needed were simplified.
Tests, even on a clean download of the repository, seems to be currently broken, at least on my system, with multiple failing testes and a certain amount of panics.
This means that I was unable to use the tests to avoid regressions or prescribed errors. I cannot, hence, be sure about the correctness of each case.
If this is intended to be accepted, please let me know what I should do meet quality control, be it adding tests or something else.