Update "Type Suffixes" coding guidelines

silabs-robin commented 2 years ago

Task Outcome

Should update the "Type Suffixes" table in the coding style guidelines.

Background information

The table prescribes suffixes to use, but we already have somewhat of a standard for some of these types. Hence, we should update the table to use our existing conventions where applicable.

Additional context

This was discussed on mattermost, in the "TWG: Verification" channel. I plan to do this task myself, but anyone else is of course welcome to do it, and this ticket is created as a reminder to get it done.

MikeOpenHWGroup commented 2 years ago

Thanks for bringing this up @silabs-robin. Perhaps it would be useful to use this issue document the "somewhat standard types" we currently have in use before updating the coding style guidelines themselves.

silabs-robin commented 2 years ago

This is what I could find:

_c           classes                      (same as guidelines)
""/_enum/_t  enumerated types             (inconsistent; what about typedefed enums?)
_pkg         packages                     (same)
_if          interface types              (different from guidelines)
_if          interface instances          (different; same as types themselves)
_vif         virtual interface instances  (different)
_t           typedefs                     (same; should depend on typedefed type?)
??           structs                      (we only have typedefed structs with "_t"?)
_cb          clocking blocks              (same)
_mp          modports                     (same)
""/_cons/_c  constraints                  (inconsistent)
cg_          covergroups                  (same... except is prefix)

MikeOpenHWGroup commented 2 years ago

Thanks for investigating this @silabs-robin. The delta between what we have and Brian Hunter's guidelines is not that large. Also, in most of the cases where there is a difference, we have little (or no) code that uses Hunter's suffixes. so it should not be too painful to close this. So how about the following?

Suffix	Type	Comment
`_c`	Classes
`_e` ,`_enum`	Enumerated Types	Either suffix is acceptable
`_te`, `_tenum`	Typedef'd enumerated types	Either suffix is acceptable
`_pkg`	Packages
`_if`	Interface Interfaces or Types
`_vif`	Virtual Interface Instances
`_st`	Structs
`_t`	Typedefs, Misc. Types
`_cb`	Clocking Blocks
`_mp`	Modports
`_cons`	Constraints
`_cg`	Covergroups	Some existing code uses `cg_` prefix

silabs-robin commented 2 years ago

I like your proposal.

Typo in "Interface Interfaces or Types"? Should be "Interface Instances or Types" or for example "Interface Types and Instances"?

I like the idea of _st for structs. But it seems like _t for tydefed structs is the most common suffix? (It also doesn't seem like non-typedefed structs are used at all.) Even the 1800-2017 LRM has some uses of _t for typedefed structs.

For enums, I think _e/_enum is sufficient, because it seems typedef is the only way to create an enum type, appart from directly creating a variables like enum {on, off, standby} mystate_d, mystate_q.

How about this:	Suffix	Type
_c	Classes
_pkg	Packages
_if	Interface Types and Instances
_vif	Virtual Interface Instances
_e, _enum	Typedef'd enumerated types	Either suffix is acceptable
_t	Typedefs, Misc. Types	All typedef'd types except enums
_cb	Clocking Blocks
_mp	Modports
_cons	Constraints
_cg	Covergroups	Some existing code uses cg_ prefix

Other thoughts:

We should ask for the opinion of more people.
_vif isn't really for a "type", it is for an instance, but it was in the original table so ok.
I am not sure whether enums should just use _t like all other typedef'd types.
Maybe there should be just 1 openhw systemverilog guideline, or maybe we could refer to some rtl-guidelines instead of having core-v-verif guidelines specify all of this.
Assert/assume/cover and generate statements also have a convention, but they are not types so it doesn't have to be part of this discussion.

MikeOpenHWGroup commented 2 years ago

Typo in "Interface Interfaces or Types"? Should be "Interface Instances or Types" or for example "Interface Types and Instances"?

Yes, thanks for the catch,

I like the idea of _st for structs. But it seems like _t for tydefed structs is the most common suffix?

I too like _st for structs, so I'll add it as a recommendation. You are right that _t is already widely used for typedefs (as you'd expect) and typedefed structs. I'm not perpared to police such finegrained suffixes, so I'm recommending we simple accept what is alreadyt in use.

For enums, I think _e/_enum is sufficient.

Agreed.

We should ask for the opinion of more people.

Agreed. I'll mention this discussion to the Verification channel on MatterMost. We should give people a week or so to comment before committing this to the coding style guidelines on GitHub. And of course, we are always free to update the guidelines at any time in the future.

openhwgroup / core-v-verif