Skip to content

[WIP] Coding Style Guide #3026

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

soheilshahrouz
Copy link
Contributor

Addresses #2678

@github-actions github-actions bot added the docs Documentation label May 7, 2025
@soheilshahrouz
Copy link
Contributor Author

@amin1377
@AlexandreSinger
@AmirhosseinPoolad
@vaughnbetz

Any comments on what should be added to this guide is appreciated.

@AmirhosseinPoolad
Copy link
Contributor

Fully agree with the usage of auto as documented here. I guess we also should talk about the naming schemes we use. What I personally extrapolated from the code base and remember currently is that Classes are PascalCase, everything else is snake_case, structs are trivial and C-like and start with t_ but it should all be written down.

One more thing is saying that people should prioritize (but not blindly use in all circumstances) using vtr:: containers over STL and use the various LOG/ERROR macros over directly using printf/cout.

Copy link
Contributor

@AlexandreSinger AlexandreSinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Soheil, thank you so much for starting to add documentation on coding style! This is extremely important for such a large project such as VTR.

I think the coding style that we tell the students in ECE297 will be a good reference for a checklist of things to add to this documentation:
tutorial5_code_style.pdf

Specifically this slide:
image

I think the major things that are missing are:

  • The variable, function, and classes naming conventions that we use in VTR
  • Keeping function short (i.e. pulling out code into helper methods)
  • Self-checking code (i.e. proper use of VTR_ASSERT)

I think the three above are very important to document since we can point people to this page if they raise a PR that does not have proper style; since I believe these three things (as well as the things you have documented) would block a PR from being merged in my opinion!

I know our goal is to keep this style guide brief; but if you would like more things to add to the style guide, I can point you to some of the different style guides that I am familiar with!

Copy link
Contributor

@vaughnbetz vaughnbetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!


.. code-block:: cpp

class Example {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this convention is not followed very consistently today but we should move to it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we say that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe we just say that at the top of the style guide.

-------------

- Focus on explaining *why* the code exists or behaves in a certain way.
- Do not explain *what* the code is doing if it's already clear from the code itself.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.h file should say what the code does; if not obvious explain why the code behaves that way as well.
.cpp file should explain how the code works if it is not obvious.


Comment types and rules:

- Use `/* ... */` **only** for Doxygen documentation comments.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd weaken this a bit / explain the reasoning. Seem a bit strong on the /* vs. //





Copy link
Contributor

@vaughnbetz vaughnbetz May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other guidelines:

  • Prefer short functions, so they can be reused and their purpose can be more easily commented.
  • Avoid unnecessary use of complex language features; prefer easier-to-read code
  • Check the code and documentation at to see if a utility or routine already exists before making a new one
  • Group related data into structs or classes, and comment each data member and member function

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertion guidelines:

  • use assertions and data structure validators wherever possible.
  • In CPU-critical code, use VTR_ASSERT_SAFE for potentially expensive checks <link>

- Do not explain *what* the code is doing if it's already clear from the code itself.
- Keep comments up to date. Outdated comments are worse than no comments.
- Use Doxygen-style `/** ... */` or `///` for documenting APIs, classes, structs, members, and files.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow the instructions at <link> to add new utilities, APIs or internal functionality doxygen documentation to the html pages at <link>.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants