Fix for issue #583 #587
Fix for issue #583 #587
Conversation
| /// - link header/metadata instances to the Type | ||
| /// - replace named references to global header or metadata instances with ConcreteHeaderRef | ||
| /// expressions that link directly to them. | ||
| /// - set type for Member and HeaderStackItemRefs | ||
| class TypeCheck::AssignInitialTypes : public Transform { |
There was a problem hiding this comment.
Is there any particular reason you're using nested classes rather than namespaces?
There was a problem hiding this comment.
I have only changed the comments here. This code was written by @ChrisDodd.
There was a problem hiding this comment.
I generally define nested visitors for a PassManager subclass as nested classes in the PassManager subclass; seems cleaner to me that defining a separate namespace in addition to the class.
| if (it.first->layout) { | ||
| auto type = types.get(it.first->layout); | ||
| if (type->is<IR::Type_Header>()) | ||
| createType(type, true, &converted); |
There was a problem hiding this comment.
Register layouts should always be treated as structs, not headers (should not have valid bits), They're only declared as header_types in P4_14, because that is the only way to define structs in P4_14 -- whether something is a header or metadata is determined by the instance declaration.
There was a problem hiding this comment.
Then let's not yet merge this yet.
There was a problem hiding this comment.
It may be that the best place to do the conversion to a struct is somewhere earlier, in the P4-14 typechecker. I will try to figure it out.
There was a problem hiding this comment.
Actually the layout is just a string, so that solution won't work.
Also, slightly improved comments and debugging output in the p4-14 typecheck file.