-
Notifications
You must be signed in to change notification settings - Fork 444
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
Validate arguments to log_msg #2462
Conversation
Fixes #2201 |
auto argType = ctxt->typeMap->getType(arg1); | ||
if (auto ts = argType->to<IR::Type_List>()) { | ||
for (auto tf : ts->components) { | ||
if (!tf->is<IR::Type_Bits>()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it still work with SerEnum arguments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 134 from issue2201-bmv2.p4 tests for this case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the testdata/p4_16_samples/issue2201-bmv2.p4 is marked as XFAIL, I don't think anything is really tested as far as these backend changes are concerned? But if tf->is<IR::Type_Bits>()
is true for SerEnum types, that works for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a new test 2201-1-bmv2.p4 which tests this case.
return primitive; | ||
} | ||
|
||
auto val = ctxt->conv->convert(arg1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you call convert
(https://github.com/p4lang/p4c/blob/master/backends/bmv2/common/expression.h#L105) with convertBool
set to true
? Then it will work fine with boolean values as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did that, but since this is a list I had to iterate over the elements first.
* Validate arguments to log_msg
No description provided.