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

Remove some unnecessary dynamic vectors/strings #7862

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Dave-Lowndes
Copy link
Contributor

Eliminate dynamic vector & string storage where there's a simpler alternative.

Code tweak in prepareForEvaluation to use logical OR rather than bitwise and const the initial bool variable.

…ernative.

Code tweak in prepareForEvaluation to use logical OR rather than bitwise and const the initial bool variable.
@baconpaul
Copy link
Collaborator

So this change is in the first half correct but I'm not sure I like it. Naming that white list and table set makes the code quite a bit clearer i thought.

The second change to a constexpr initializer list is, I think, a c++ 20 thing. At least it doesn't compile in ci with clang for mac.

@mkruselj
Copy link
Collaborator

mkruselj commented Nov 21, 2024

That whitelist should've been behind a // clang-format off (even if it's a valid clang-format that we use), for further clarity I think. It would signify we really want it to be formatted that way.

David Lowndes added 2 commits November 21, 2024 21:30
More ref usage.
2 functions can be static.
@Dave-Lowndes
Copy link
Contributor Author

How's this one looking now?

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

Successfully merging this pull request may close these issues.

3 participants