allow BENCHMARK_VERSION to be undefined

[PhilipDeegan]

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 modifications

I 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

[LebedevRI]

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
}

?

[LebedevRI]

Hm, and if BENCHMARK_VERSION is not defined, GetBenchmarkVersion() now returns "BENCHMARK_VERSION":
https://godbolt.org/z/d9xKEbc7o
Is that the expected outcome?

[PhilipDeegan]

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

[PhilipDeegan]

How about we just explicitly do ...

I can do this, but I still think the escapes can be avoided via stringification as @dmah42 suggested

[LebedevRI]

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.

[PhilipDeegan]

updated to just

std::string GetBenchmarkVersion() {
#ifdef BENCHMARK_VERSION
  return {BENCHMARK_VERSION};
#else
  return {""};
#endif
}

[dmah42]

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.

[LebedevRI]

@PhilipDeegan thank you!