Closed PhilipDeegan closed 3 months ago
I feel this is a roundabout way to achieve the the end result. How about we just explicitly do
std::string GetBenchmarkVersion() {
#ifdef BENCHMARK_VERSION
return {BENCHMARK_VERSION};
#else
return {""};
#endif
}
?
Hm, and if BENCHMARK_VERSION
is not defined, GetBenchmarkVersion()
now returns "BENCHMARK_VERSION"
:
https://godbolt.org/z/d9xKEbc7o
Is that the expected outcome?
Hm, and if
BENCHMARK_VERSION
is not defined,GetBenchmarkVersion()
now returns"BENCHMARK_VERSION"
: https://godbolt.org/z/d9xKEbc7o Is that the expected outcome?
I'm kinda surprised it even compiles if it's not defined, but perhaps BENCHMARK_VERSION
is more informative than an empty string
How about we just explicitly do ...
I can do this, but I still think the escapes can be avoided via stringification as @dmah42 suggested
Hm, and if
BENCHMARK_VERSION
is not defined,GetBenchmarkVersion()
now returns"BENCHMARK_VERSION"
: https://godbolt.org/z/d9xKEbc7o Is that the expected outcome?I'm kinda surprised it even compiles if it's not defined, but perhaps
BENCHMARK_VERSION
is more informative than an empty string
That's because it's passed into the stringification macro. Normally it indeed fails: https://godbolt.org/z/j5x4a4sWG Which is why i would've preferred to not change anything in benchmark, and let the external buildsystems deal wit this, because if we do this workaround, they may not even realize they need to deal with this.
updated to just
std::string GetBenchmarkVersion() {
#ifdef BENCHMARK_VERSION
return {BENCHMARK_VERSION};
#else
return {""};
#endif
}
oh, that's a shame. i quite liked the reduced complexity in the bazel config. i was thinking we'd land somewhere in between where be both stringify it if it's set but return a safe value if it's not. anyway, this also looks fine.
@PhilipDeegan thank you!
closes #1768
motivation is to allow simpler override at compilation time shouldn't impact anyone using the official build tools as it's all internal in that case
I haven't tested the bazel build output for the function
benchmark::GetBenchmarkVersion()
but I'm guessing it's without the extra quote from "stringification" given the modificationsI have tested the cmake build output for the funciton, and the string is without quotes with the proposed changes
PRed on my fork for testing here I can't explain the sanitizer failures so I'm figuring they are existing