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

Filterx: check if value is a macro during compilation #431

Conversation

bazsi
Copy link
Member

@bazsi bazsi commented Dec 30, 2024

This PR ended a bit more than what the title says:

  • the original goal was to check whether a variable is a hard macro and do this in the compilation phase, instead of during evaluation
  • then I removed the macro wrapper for $MSG, which was not really doing much anymore
  • this broke value-pairs
  • also broke light tests, which relied on $MSG being a string, even though it was set to a protobuf/bytes value
  • and I also noticed an read-side off-by-one for macro based variables
  • I ended up also adding a warning when using bytes or protobuf values in a template which does not render without a type hint.

With all that fixed is what we have in this branch.

@bazsi bazsi force-pushed the filterx-check-if-value-is-a-macro-during-compilation branch 2 times, most recently from ce6cd7b to 1871070 Compare December 31, 2024 09:37
@OverOrion OverOrion self-requested a review January 2, 2025 09:30
@bazsi bazsi force-pushed the filterx-check-if-value-is-a-macro-during-compilation branch 2 times, most recently from f8570fd to 9d55fc3 Compare January 5, 2025 17:40
@bazsi bazsi changed the title Filterx check if value is a macro during compilation Filterx: check if value is a macro during compilation Jan 5, 2025
bazsi added 7 commits January 6, 2025 16:58
The special handling for $MSG is long gone, avoid handling $MSG as a macro
as it requires extra copying in filterx and there's no functional
requirement anymore.

Signed-off-by: Balazs Scheidler <[email protected]>
Instead of doing this in the hot path. Also eliminate the related helper
function.

Signed-off-by: Balazs Scheidler <[email protected]>
… non-zero

To match how object-bytes/object-protobuf works.

Signed-off-by: Balazs Scheidler <[email protected]>
It took me a while to realize why the output is not rendered in our
light tests, once I removed $MSG as a macro. This message would have
helped me a lot.

Signed-off-by: Balazs Scheidler <[email protected]>
With the $MSG as a macro gone, $MSG became a typed value, so it does matter
what we set it to. Since setting it to a bytes/protobuf value means
that its not a string anymore, we can't render it as a part of a template
unless we explicitly ask for the right type hint.

This is what this patch changes in the broken testcases.

Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi bazsi force-pushed the filterx-check-if-value-is-a-macro-during-compilation branch from 9d55fc3 to f8e6cb6 Compare January 6, 2025 15:58
Copy link
Contributor

@OverOrion OverOrion left a comment

Choose a reason for hiding this comment

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

Thank you!

@OverOrion OverOrion merged commit d22a797 into axoflow:main Jan 7, 2025
22 checks passed
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