p4c icon indicating copy to clipboard operation
p4c copied to clipboard

match_kind identifiers acts interesting and seems not to follow the spec

Open apinski-cavium opened this issue 2 years ago • 5 comments

Take:

const int t = 1;
match_kind {
  t,
  b
}
const int tt = t;

I would have expected an error message that t has been redeclared as a different type in the toplevel scope but no the above code is accepted and the match_kind t is hidden instead.

The spec contains:

All identifiers are inserted into the top-level namespace.
It is an error to declare the same `match_kind` identifier multiple times.

apinski-cavium avatar Apr 08 '22 02:04 apinski-cavium

Here is another testcase which I would assume would get an error message on too:

match_kind {
  t,
  b
}
const match_kind t = t;

Or at least a warning. I do get a warning as expected for:

match_kind {
  t,
  b
}
void func()
{
  bit<1> t;
}

apinski-cavium avatar Apr 08 '22 03:04 apinski-cavium

This is an interesting one. I could argue that we have to modify the spec to handle this case elegantly. I don't think we actually want the match_kind identifiers in the global namespace, they should always be referred to using match_kind.t, with the exception of table keys, where they can be used without the prefix.

mihaibudiu avatar Apr 08 '22 20:04 mihaibudiu

This is an interesting one. I could argue that we have to modify the spec to handle this case elegantly. I don't think we actually want the match_kind identifiers in the global namespace, they should always be referred to using match_kind.t, with the exception of table keys, where they can be used without the prefix.

Right, I agree match_kind should be act similar to error (keyword/type) here and would be consistent too. I don't know the history of the two (error and match_kind) to know why they are different.

apinski-cavium avatar Apr 08 '22 21:04 apinski-cavium

They are different because we don't want to say match_kind.exact in every table key.

mihaibudiu avatar Apr 08 '22 21:04 mihaibudiu

I actually tried to improve this, but there is a problem: this change would really break backwards compatibility with the P4 language as defined now. For example, match_kinds are also used within "@match" annotations for value_sets, without a prefix. So we could also special-case that, but it becomes ugly. I will think about it a bit more.

mihaibudiu avatar Apr 08 '22 22:04 mihaibudiu