[libcxx-commits] [PATCH] D125300: type_traits: make __invokable_r work in terms of argument passing.

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik accepted this revision as: philnik.
philnik added a comment.

In D125300#3507841 <https://reviews.llvm.org/D125300#3507841>, @jacobsa wrote:

> In D125300#3505634 <https://reviews.llvm.org/D125300#3505634>, @philnik wrote:
>
>> In D125300#3505552 <https://reviews.llvm.org/D125300#3505552>, @jacobsa wrote:
>>
>>> Dumb question: is there a way to see a list of unresolved comments? All I can
>>> see how to do is look at _all_ the comments in the diff view, which is hard
>>> to parse when there are a lot of resolved ones. I'm not 100% sure I addressed
>>> all of your comments.
>>
>> I don't know what you are looking at exactly, but on the top right you should see something like `12 / 23 Comments`. You can click on that to go to the next unresolved comment. Right next to that you can also select `Hide "done" Inlines`.
>
> I see that this is supposed to work; there is even a screenshot here:
>
>   https://secure.phabricator.com/rPb3b30dde6ac99786a77f903823ebf259db6e3adc
>
> But my diff view doesn't look like that; it doesn't have any of the widgets
> shown in that screenshot. Here's what I see: https://imgur.com/FdJwQz1

You have to scroll down a bit further. Then a banner should pop up at the top of the screen where the other options are. BTW if you want some of your questions get answered a bit quicker you might want to join the LLVM discord.

LGTM % nits. If you don't have commit access (which I assume) please provide "Your Name" <your.email at address.com> for attribution.



================
Comment at: libcxx/include/type_traits:3508-3509
+
+  template <class _Tp>
+  static _Tp _Make();
+
----------------
Could you add a comment why you can't just use `std::declval`? I think that's not obvious.


================
Comment at: libcxx/test/std/utilities/meta/meta.rel/is_nothrow_invocable.pass.cpp:200-201
+
+    static_assert(std::is_nothrow_invocable_r<CantMove, Fn>::value);
+    static_assert(!std::is_nothrow_invocable_r<CantMove, Fn, int>::value);
+  }
----------------
Could you also add `static_assert`s for the `_v` version?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125300