Skip to content
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

Bitfield improvements #1492

Merged
merged 9 commits into from
Aug 22, 2023
Merged

Bitfield improvements #1492

merged 9 commits into from
Aug 22, 2023

Conversation

rsmmr
Copy link
Member

@rsmmr rsmmr commented Aug 4, 2023

This addresses #1032, #1468, and #1484.

@rsmmr rsmmr force-pushed the topic/robin/bitfield-improvements branch 5 times, most recently from de71633 to 32dd7fa Compare August 11, 2023 10:20
The `isEqual` method was implemented in the base class, which led to
wrong template types being used.

A subsequent commit will depend on this working correctly, including a test
breaking if not.
This prepares for making `bitfield` a fully-supported HILTI type. This
change only moves existing code over to HILTI without breaking
Spicy-side functionality. There are still HILTI-side pieces missing
for providing full support, which will come next.
We keep representing bitfields as a C++ tuple behind the scenes, but
no longer map it into a HILTI-side tuple as well. Instead `Bitfield`
is now a regular type with corresponding operators and runtime
functionality.

The one change to the C++ tuple representation is that we add an
additional hidden tuple element at the end that stores the original
integer value. That allows for displaying it (which we will do for now
only inside Spicy debug output) and could later also facilitate modifying
fields and reconstructing an integer.
Closes #1032. Using a typedef with bitfields works now.
Closes #1484. `&convert` can now refer to individual bitfields.
@rsmmr rsmmr force-pushed the topic/robin/bitfield-improvements branch from 32dd7fa to b681a4d Compare August 14, 2023 13:45
@rsmmr rsmmr changed the title [WIP] Bitfield improvements. Bitfield improvements Aug 14, 2023
@rsmmr rsmmr marked this pull request as ready for review August 14, 2023 13:47
@rsmmr rsmmr requested a review from bbannier August 14, 2023 13:48
tests/spicy/types/bitfield/anonymous-fail.spicy Outdated Show resolved Hide resolved
spicy/toolchain/src/compiler/visitors/normalizer.cc Outdated Show resolved Hide resolved
spicy/toolchain/bin/spicy-dump/printer-text.cc Outdated Show resolved Hide resolved
This works now:

    type Foo = unit {
      : bitfield(8) {
        x: 0..3;
        y: 4..7;
      };

      on %done {
        print self.x, self.y;
      }
};

Closes #1468.
…s anonymous.

This preservers the information if a field had a name even though
internally we do carry a generated dummy name around in that case. For
now, we just adapt `spicy-dump` to not print the dummy name in that
case in its text output. Later, Zeek will leverage this when
auto-generating record types.
We now render them as key/value form in both text and JSON output.
We were including offsets into spicy-dumps JSON output, but there was
no information in there saying which offsets belong to what field.
Turning `__offsets` from an array into a map indexed by field name now.
Spicy-dump now renders items of anonymous bitfields at the parent
level. Offsets are added accordingly as well if requested.
@rsmmr rsmmr force-pushed the topic/robin/bitfield-improvements branch from 51dfa0d to 33f5855 Compare August 22, 2023 10:04
@rsmmr rsmmr requested a review from bbannier August 22, 2023 10:04
Copy link
Member

@bbannier bbannier left a comment

Choose a reason for hiding this comment

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

🚀

@rsmmr rsmmr merged commit 79a47c9 into main Aug 22, 2023
22 checks passed
@rsmmr rsmmr deleted the topic/robin/bitfield-improvements branch August 22, 2023 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants