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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/source_location:57-58
+  // in the context of the caller. An explicit value should never be provided.
+  static consteval source_location current(
+      decltype(__builtin_source_location()) __ptr = __builtin_source_location()) noexcept {
+    source_location __sl;
----------------
Quuxplusone wrote:
> My kneejerk reaction here was "this indentation is messed up," but then my eyes refocused. ;)
> I suggest reflowing as
> ```
> static consteval
> source_location current(const void *__ptr = __builtin_source_location()) noexcept {
> ```
> Using `const void *` instead of `decltype(__builtin_source_location())` helps readability and doesn't seem to hurt anything since you're already doing the cast on line 61. In fact, at that point I'd remove the comment on line 60 because now the cast isn't unusual-looking at all.
> 
> (2) Should we use `_LIBCPP_HIDE_FROM_ABI` here, even though it's consteval? I don't know.
> (2) Should we use _LIBCPP_HIDE_FROM_ABI here, even though it's consteval? I don't know.

I don't think so. A `consteval` function can never be part of any ABI, so there is no need to mark it as `_LIBCPP_HIDE_FOM_ABI`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120634