Thanks for the contribution! This looks pretty good, however I would like to see tests where current() is called in some full-expression with other expressions in it, in a default member initializer, etc.
http://eel.is/c++draft/support.srcloc.class#lib:source_location specifies the intended behavior of source_location in various special cases (like http://eel.is/c++draft/support.srcloc.class#support.srcloc.cons-2), and we should have tests for those.
Also, is there a reason why the runtime and the constexpr tests can't be identical to each other? It looks like some of your coverage is only on constexpr tests, and other coverage is only on the runtime tests.
I think we need to figure out this issue before we move forward.
|29 ↗||(On Diff #265493)|
We can't test it that way because the standard says it's unspecified what it returns. I think those need to be libc++ specific tests.
|21 ↗||(On Diff #265493)|
Should we add ASSERT_NOEXCEPT(loc.column())? This also applies to the other accessors AFAICT.
|25 ↗||(On Diff #265493)|
Like mentioned above, this needs to be a libc++ specific assertion.
|37 ↗||(On Diff #265493)|
I still don't know why it doesn't work with consteval, but I've seen that libstdc++ experimental implementation also uses constexpr here: https://github.com/gitGNU/gcc/blob/master/libstdc%2B%2B-v3/include/experimental/source_location#L50
Ok, so clang 8 doesn't support consteval. It also doesn't have required builtins (__builtin_FILE etc.). consteval works ok on clang 9+.
How can I mark tests as unsupported when the builtins are not supported? (a good example would be column.pass.cpp test that I disabled for gcc that doesn't have __builtin_COLUMN)
Yes, sorry. I should have stated that initially. We need source location to be as low cost as possible to copy.
It should only consume a single register.
Please feel free to reach out via email or chat and we can discuss ideas on achieving this in more depth.
I've updated the "unsupported" comments with precise compiler versions. I cannot test apple-clang though.
Anyway, I'll put this revision into hibernation until there's a builtin for a single-pointer source location (whether it be __bultin_source_location or T* __builtin_static_storage_duration(T)).