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

Update loglevels.hpp #496

Merged
merged 2 commits into from
May 20, 2023
Merged

Update loglevels.hpp #496

merged 2 commits into from
May 20, 2023

Conversation

CryptoManiac
Copy link
Contributor

See #495 for details

Issue is not triggered if __cplusplus is less than 202002L on your system.

@KjellKod
Copy link
Owner

@SzGaa please review this.

@KjellKod
Copy link
Owner

I think this is fine. Running tests now. constexpr vs const isn't really a huge win regardless outside of making it more up-to-date. I'm planning to merge this once it's all gone through CI

@KjellKod
Copy link
Owner

Thank you @CryptoManiac for this insight and update

src/g3log/loglevels.hpp Outdated Show resolved Hide resolved
src/g3log/loglevels.hpp Outdated Show resolved Hide resolved
@CryptoManiac
Copy link
Contributor Author

CryptoManiac commented May 20, 2023

I think this is fine. Running tests now. constexpr vs const isn't really a huge win regardless outside of making it more up-to-date. I'm planning to merge this once it's all gone through CI

This testing doesn't have a lot of sense because const vs constexpr have fundamental differences which have nothing to do with either performance or being up to date. The const identifiers are variables. Read-only, yes, but still allocated as usual. The same can't be said about constexpr expressions, they only exist at compile-time and there is no such thing in the generated code. Therefore, there is a lot of limitations on what you can or can't do with them. For constexpr identifier any input data must be known at compile time, meaning that there is no way to allocate dynamic memory either for them or for their input. In simpler terms, all input values must be literals or other constexpr expressions. That is why they're good for switches and math equations or other things like that. For the same reason you can use the result of constexpr expression as a template argument, for example, while the same can't be done with const identifiers.

@SzGaa
Copy link
Contributor

SzGaa commented May 20, 2023

The const identifiers are variables. Read-only, yes, but still allocated as usual. The same can't be said about constexpr expressions, they only exist at compile-time and there is no such thing in the generated code.

Compiler is not forced to use constexpr in compile-time context only (also not guaranteed that it will use that way even if could be possible), just allows it to be used in places where only compile time constants would be applicable.
Also constexpr implies constness.

constexpr specifier

@CryptoManiac
Copy link
Contributor Author

CryptoManiac commented May 20, 2023

Compiler is not forced to use constexpr in compile-time context only (also not guaranteed that it will use that way even if could be possible), just allows it to be used in places where only compile time constants would be applicable.

It allows them to be used in places like that because it does provide a guarantee that the expression is deterministic and the required data is known at compile-time. Therefore it may be evaluated in compile-time. Of course, for the sake of common sense, compiler won't actually evaluate expressions unless they're something very simple. But this has no visible side effects since the said limitations still stand. No heap allocations, no exceptions and only literal value types. It's also kind of understandable why goto is not supported, since though they're quite possible to implement, it would be really hard to actually do such a thing.

Also constexpr implies constness.

Well, Shakespeare was born in England, he wrote in english and spoke in english as well. Therefore, Shakespeare was english. Of course they are const since they're deterministic and their input data is const. :)

@KjellKod KjellKod merged commit 7817fd3 into KjellKod:master May 20, 2023
@CryptoManiac CryptoManiac deleted the patch-1 branch May 25, 2023 15:58
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