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

Mark BMV2 JSON scalars as 'pi_omit' so they won't be exposed in PI #209

Merged
merged 1 commit into from
Dec 23, 2016

Conversation

sethfowler
Copy link
Contributor

This patch marks the scalars header instance that we synthesize in the BMV2 JSON converter as "pi_omit". The plan is to not expose the scalars in PI, since they're an implementation detail and it doesn't really make sense to interact with them via PI directly.

@sethfowler sethfowler self-assigned this Dec 23, 2016
Copy link
Member

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

cool, I'll update the PI code to leverage this new attribute; I am not sure there is a use to update the bmv2 JSON doc right away as this attribute will be ignored by the bmv2 model

@sethfowler sethfowler merged commit afb9a65 into p4lang:master Dec 23, 2016
@sethfowler sethfowler deleted the exclude-scalars-from-pi branch December 23, 2016 01:37
@@ -1761,6 +1761,7 @@ void JsonConverter::addLocals() {
json->emplace("id", nextId("headers"));
json->emplace("header_type", scalarsName);
json->emplace("metadata", true);
json->emplace("pi_omit", true); // Don't expose scalars in PI.
Copy link
Contributor

Choose a reason for hiding this comment

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

I am using the same scalars struct in the output for storing local variables but also for scalar fields that may have been declared in the user metadata structure.

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.

3 participants