Skip to content
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

Revisit use of static/const/constexpr keywords on variables #3958

Open
neobrain opened this issue Aug 15, 2024 · 1 comment
Open

Revisit use of static/const/constexpr keywords on variables #3958

neobrain opened this issue Aug 15, 2024 · 1 comment

Comments

@neobrain
Copy link
Member

We currently apply different, not strictly consistent idioms of these throughout the code base. Instead of derailing PR reviews with discussions on this, let's settle for a few guidelines that should be easy to understand and apply while not introducing performance hazards.

@neobrain
Copy link
Member Author

neobrain commented Aug 15, 2024

Here are some unfinished thoughts (for personal reference)

Variables with constant values:

  • By default: const (global constants (constexpr?), local constants)
  • Large, local mathematically constant data: static constexpr (also for class members)
  • Large, global mathematically constant data: constexpr static constexpr [1]

Variables that aren't constant

  • No extra keywords by default
  • Global variables only used in the current file: static

Rare special cases (don't apply without knowing what you do):

  • static const (one-time initialization of complex data)
  • static (persisting variables, e.g. for caching)
  • constexpr (compile-time programming)
  • Large local data (const or non-const): Likely shouldn't live on the stack
  • Large global non-const data with constant initializer: constinit
  • TODO: inline constexpr?

Not covered:

  • functions

Rationale:

  • Make rules easy to apply consistently: For common cases, we shouldn't have a gazillion of rules. Doing something excep should require reading the fine print, and the main rules should make it obvious what's an exceptional case (e.g. by omitting the specific keywords that would be needed for exceptional cases)
  • Avoid state bugs from accidental static constexpr -> static const refactors. This could happen e.g. when adding a parameter dependency to a computation and subsequently dropping constexpr for const. The could would seemingly work but you could easily forget dropping the static
  • Rule for large data:
    • In functions: otherwise data gets copied to the stack each time => performance hazard
    • [1] Globally: Otherwise computed data might not get baked into the binary and instead get initialized at runtime. static is redundant, but using it allows for one fewer rule.

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

No branches or pull requests

1 participant