p4RuntimeSerializer does not support stack field references in match keys

[antoninbas]

The attached program produces the following output:

antonin@ubuntu:~/Documents/p4lang/p4c/build$ ./p4c-bm2-ss --p4-14 stack_ref.p4.txt --p4runtime-file i.bin
stack_ref.p4.txt(11): error: Expression '[]' is too complicated to resolve to a header field
table t { reads { stack[0].f0 : ternary; } }
                  ^^^^^^^^
stack_ref.p4.txt(11): error: Table 't': Match field '[].f0' is too complicated to represent in P4Runtime
table t { reads { stack[0].f0 : ternary; } }

stack_ref.p4.txt

[antoninbas]

To observe this output, this fix is needed: #290. Otherwise the compiler crashes.

[sethfowler]

This is a known problem; header stack references in match keys were something I left for future work in an effort to get p4RuntimeSerializer merged more quickly. I'll cook up a patch.

[sethfowler]

That error should also refer to stack[0].f0 rather than [].f0. I'll see if I can't fix that, too, though it's not immediately obvious to me why it's happening.

[mihaibudiu]

In P4-16 array indexes do not have to be constant values.
If you just use the toString on an array index expression it will skip the index.

[sethfowler]

And that's what ::error() is using internally?

[mihaibudiu]

Yes, ::error calls toString.
BUG calls dbprint
This is documented in the powerpoint in docs/.

[sethfowler]

OK. So I suppose this is a general problem with any compiler error we report involving a header stack, then. Should we change toString()'s behavior to print the index expression, or is there a reason to prefer the current behavior?

[ChrisDodd]

Arguably the default Expression::toString() method should just call dbprint into stringbuffer and return the resulting string. We kind-of have the convertion that dbprint is for logging while toString is for error messages, so in general dbprint might include compiler internal info that would be confusing to end-users, but at least as far as expressions are concerned, dbprint just prints the expression in source-equivalent form.

…

On Thu, Feb 9, 2017 at 2:16 PM, Seth Fowler ***@***.***> wrote: OK. So I suppose this is a general problem with any compiler error we report involving a header stack, then. Should we change toString()'s behavior to print the index expression, or is there a reason to prefer the current behavior? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#291 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AD4c8a86Kishg4qLKw-PvZ4EiCiBddibks5ra5AhgaJpZM4L8gAU> .

[mihaibudiu]

This is a problem of usability; if the index expression is complex the error message may be more obscure.
dbprint prints compiler internals, we cannot use that for user errors.

[sethfowler]

The consequence of the current approach is that errors messages refer to code that isn't valid P4 and doesn't actually appear in the program anywhere, though. =\

[mihaibudiu]

If the error refers to an IR node with source position the source code will be quoted exactly.
The rest of the error message should be as informative to the human as possible.

[sethfowler]

It sounds like it would be simplest not to address that issue as part of this PR. =)