Implement Consistent Commenting Style in backend #4568
Conversation
sumit7754
changed the title
| /// @param max_length maximum length for a header with varbit fields; | ||
| /// if 0 header does not contain varbit fields | ||
| /// @param fields a JsonArray for the fields in the header | ||
|
|
There was a problem hiding this comment.
You added an additional space here between the comment and the C++ code. Please remove it for all instances.
| @@ -166,7 +165,7 @@ bool CFG::checkImplementable() const { | |||
| namespace { | |||
| class CFGBuilder : public Inspector { | |||
| CFG *cfg; | |||
| /// predecessors of current CFG node | |||
There was a problem hiding this comment.
This is not correct. Members in header files also will have triple slashes. We only use double slashes for "internal" comments within functions, for example.
There was a problem hiding this comment.
Please fix this for all instances.
There was a problem hiding this comment.
/// Create a header type in json.
/// @param name header name
/// @param type header type
/// @param max_length maximum length for a header with varbit fields;
/// if 0 header does not contain varbit fields
/// @param fields a JsonArray for the fields in the header
Are you referring to a gap in these comments?
There was a problem hiding this comment.
You can format Github comments using triple '`', it will make your comment more readable: https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/creating-and-highlighting-code-blocks
Yes, in all the comments where there is a newline between the comments and the actual function.
| @@ -213,4 +212,4 @@ void ActionConverter::postorder(const IR::P4Action *action) { | |||
| ctxt->structure->ids.emplace(action, id); | |||
| } | |||
|
|
|||
| } // namespace BMV2 | |||
There was a problem hiding this comment.
for the namespace should I use /// or // ?
There was a problem hiding this comment.
They are internal, use two slashes here.
There was a problem hiding this comment.
Actually, another helpful contribution is to write this all down. Very good to have! We should have done this much earlier.
There was a problem hiding this comment.
Sure we can do this.
There was a problem hiding this comment.
If you elaborate more then that will be helpful
There was a problem hiding this comment.
Basically, all these details we have discussed (comment style for namespaces, triple slashes vs star comments, newlines before functions) should be written down in text. This way we can refer to our "style document" whenever these questions come up.
There was a problem hiding this comment.
/**
* Convert an expression into JSON
* @param e expression to convert
* @param doFixup Insert masking operations for operands to ensure that the result
* matches the specification. BMv2 does arithmetic using unbounded
* precision, but the spec requires fixed precision, specified by the types.
* @param wrap Wrap the result into an additiona JSON expression block.
* See the BMv2 JSON spec.
* @param convertBool Wrap the result into a cast from boolean to data (b2d JSON).
*/ For this type of comments, what kind of formatting should we use?
|
Closing this in favor of smaller pull requests per back end folder. Like #4586. |
Fixes #4567
This pull request addresses the issue #4567, which discusses the need for a consistent commenting style in the core parts of the compiler. Following the discussions in the issue, I have made the following changes:
Used triple slashes (///) for commenting functions and classes.
Used double slashes (//) for comments within the code.