[libcxx-commits] [PATCH] D120634: [Libcxx] Add <source_location> header.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

Thanks for working on this! Some additional comments.



================
Comment at: libcxx/include/source_location:11
+#ifndef _LIBCPP_SOURCE_LOCATION
+#define _LIBCPP_SOURCE_LOCATION
+
----------------
Please update the `source_location` status in `docs/Cxx2aStatusPaperStatus.csv`. Look like this implements both both P1208 and LWG-3396.


================
Comment at: libcxx/include/source_location:28
+
+#include <version>
+
----------------
This header needs to be included in all library header, it provides all important macros like `_LIBCPP_BEGIN_NAMESPACE_STD`.


================
Comment at: libcxx/include/source_location:55
+  // static_cast is redundant on Clang, but needed on GCC, which returns 'const void*' from the builtin.
+  static consteval source_location current(const __impl* __ptr =
+                                           static_cast<const __impl*>(__builtin_source_location()) noexcept {
----------------
The `__ptr` argument differs from the wording in the Standard, can you add comment why this is done?


================
Comment at: libcxx/test/std/language.support/support.srcloc/general.pass.cpp:16-19
+// Only run tests when support is available; it's not yet universally available
+// in c++20 compilers. TODO: would it be better to exclude this test with some
+// more "UNSUPPORTED" annotations?
+#if defined(__cpp_lib_source_location)
----------------
philnik wrote:
> This should be something along the lines of `// UNSUPPORTED: clang-13`, depending on when `__builtin_source_location was implemented`.
Since the clang part is under review it should be also unsupported in clang-13 and apple-clang-12.
All our supported platforms are tested in the precommit CI so it's possible to see which other platforms fail.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120634/new/

https://reviews.llvm.org/D120634