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

[Eric Fiselier via Phabricator via libcxx-commits]

EricWF 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;
----------------
philnik wrote:
> 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`.
I don't think using `const void*` improves readability. Why do you say that?

@Quuxplusone Going forward the project should take a "format and forget about it" approach to whitespace changes. What's correct is what clang-format spits out. I don't believe reviewing whitespace changes is a good use of engineering effort.


================
Comment at: libcxx/include/source_location:64
+  }
+  _LIBCPP_HIDE_FROM_ABI constexpr source_location() noexcept = default;
+
----------------
When does this constructor get called?

I assume it's in the spec?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120634