Skip to content

switch to constexpr char[] buildinfo - #2372

Merged
JulianGro merged 1 commit into
masterfrom
fix_static_storage
Aug 12, 2026
Merged

switch to constexpr char[] buildinfo#2372
JulianGro merged 1 commit into
masterfrom
fix_static_storage

Conversation

@RTUnreal

Copy link
Copy Markdown
Collaborator

This should stop the code checker spam in PRs.

@RTUnreal
RTUnreal requested a review from JulianGro July 19, 2026 11:50
@RTUnreal
RTUnreal force-pushed the fix_static_storage branch 2 times, most recently from 03ab8de to 2fe4619 Compare July 19, 2026 13:47
// If this is a first run we short-circuit the address passed in
if (_firstRun.get()) {
if (!BuildInfo::PRELOADED_STARTUP_LOCATION.isEmpty()) {
if (!std::string_view(BuildInfo::PRELOADED_STARTUP_LOCATION).empty()) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we need to use std::string_view here, but can get away with std::empty in libraries/networking/src/AddressManager.cpp:41?
What is the difference?

@RTUnreal RTUnreal Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

One is in constexpr, the other one isn't, which means I need to construct it here. But theoretically, both could be the same.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So the difference is that here we just want to access BuildInfo::PRELOADED_STARTUP_LOCATION, and in libraries/networking/src/AddressManager.cpp:41 we want to copy it, since the compiler does that at compile time? Does std::string_view not work at compile time, so we need to copy instead?

@RTUnreal RTUnreal Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

std::string_view is does not make a copy. It is basically a fat pointer to a slice of memory, which is interpreted as a string. In the constructor, we are passing the char array as the thing we point to.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

ok, after some reading, I have found out that the usage of std::empty in this case is not workable, because here it is always returning false???

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Did we just run into exactly what Clang means when it says "may throw an exception that cannot be caught"?

This should stop the code checker spam in PRs.
@RTUnreal
RTUnreal force-pushed the fix_static_storage branch from 2fe4619 to ee13908 Compare August 6, 2026 13:50
@RTUnreal
RTUnreal requested a review from JulianGro August 8, 2026 08:47
@vatral

vatral commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Wouldn't QStringLiteral work here and be simpler?

@RTUnreal
RTUnreal force-pushed the fix_static_storage branch 2 times, most recently from 7a31709 to ee13908 Compare August 10, 2026 20:58
@RTUnreal

Copy link
Copy Markdown
Collaborator Author

Wouldn't QStringLiteral work here and be simpler?

Sadly, the same error appears, which we wanted to prevent in the first place

@JulianGro
JulianGro merged commit 324f1fb into master Aug 12, 2026
12 of 14 checks passed
@JulianGro
JulianGro deleted the fix_static_storage branch August 12, 2026 14:41
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