mclow.lists accepted this revision.
mclow.lists added a comment.
In https://reviews.llvm.org/D46845#1183462, @erik.pilkington wrote:
> In https://reviews.llvm.org/D46845#1183435, @mclow.lists wrote:
>
> > One more thing -
> > When you add a new header file, you need to update
> > `include/modu
erik.pilkington added a comment.
In https://reviews.llvm.org/D46845#1183435, @mclow.lists wrote:
> One more thing -
> When you add a new header file, you need to update
> `include/module.modulemap`, `test/libcxx/double_include.sh.cpp`, and
> `include/CMakeLists.txt`.
> Take a look at https:/
mclow.lists added a comment.
One more thing -
When you add a new header file, you need to update `include/module.modulemap`,
`test/libcxx/double_include.sh.cpp`, and `include/CMakeLists.txt`.
Take a look at https://reviews.llvm.org/D49338 for an example.
https://reviews.llvm.org/D46845
erik.pilkington added a comment.
Ping!
https://reviews.llvm.org/D46845
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
erik.pilkington added a comment.
Ping!
https://reviews.llvm.org/D46845
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.
LGTM -- great! Just make sure the commit message does not pretend the change
includes `merge`.
This may be obvious, but my LGTM alone should probably not be enough to submit.
https://revie
ldionne added inline comments.
Comment at: libcxx/include/map:43
typedef std::reverse_iteratorconst_reverse_iterator;
+typedef unspecified node_type;
+typedef INSERT_RETURN_TYPE insert_return_type;
You are missing `/
erik.pilkington added inline comments.
Comment at: libcxx/include/__hash_table:2165
+#if _LIBCPP_STD_VER > 14
+template
+template
ldionne wrote:
> When a function is declared with a visibility macro
> (`_LIBCPP_INLINE_VISIBILITY` in this case), I think it is c
ldionne added a comment.
I forgot to mention it in my initial review, but it also seems like you forgot
to update all the synopsis in the comments at the beginning of headers.
https://reviews.llvm.org/D46845
___
cfe-commits mailing list
cfe-commits
ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.
Generally looks pretty good, but I have a couple questions/suggestions.
Comment at: libcxx/include/__hash_table:2165
+#if _LIBCPP_STD_VER > 14
+template
+templat
EricWF added a comment.
I'm working on this! I'll be taking what is hopefully a final pass at the tests
today.
https://reviews.llvm.org/D46845
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cf
erik.pilkington added a reviewer: ldionne.
erik.pilkington added a comment.
Ping!
https://reviews.llvm.org/D46845
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
erik.pilkington updated this revision to Diff 153964.
erik.pilkington added a comment.
Herald added a subscriber: dexonsmith.
Split this up. I'm moving the `merge` stuff to a follow-up, that makes this
diff a lot easier to read.
https://reviews.llvm.org/D46845
Files:
libcxx/include/__hash_ta
erik.pilkington added a comment.
Ping! If it'd make this easier to review, I'd be happy to split this up a bit.
https://reviews.llvm.org/D46845
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cf
erik.pilkington added a comment.
Ping!
https://reviews.llvm.org/D46845
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
erik.pilkington added inline comments.
Comment at: libcxx/include/__hash_table:2261
+_NodeHandle
+__hash_table<_Tp, _Hash, _Equal, _Alloc>::__node_handle_extract_unique(
+key_type const& __key)
EricWF wrote:
> If I'm not mistaken, `__node_handle_extract_uniqu
EricWF added inline comments.
Comment at: libcxx/include/__hash_table:2261
+_NodeHandle
+__hash_table<_Tp, _Hash, _Equal, _Alloc>::__node_handle_extract_unique(
+key_type const& __key)
If I'm not mistaken, `__node_handle_extract_unique` and
`__node_handle_ex
rsmith added a comment.
In https://reviews.llvm.org/D46845#1105638, @erik.pilkington wrote:
> make __map_node_handle use a const_cast on the key instead of type-punning
> between pair and pair. Add calls to std::launder in key()
> and before inserting a node handle.
Can you do this as a separ
erik.pilkington added a comment.
In https://reviews.llvm.org/D46845#1098634, @rsmith wrote:
> One way we could deal with this is by adding an attribute to the compiler to
> indicate "the const is a lie", that we can apply to `std::pair::first`, with
> the semantics being that a top-level `const
rsmith added a comment.
In https://reviews.llvm.org/D46845#1099732, @mclow.lists wrote:
> >> One way we could deal with this is by adding an attribute to the compiler
> >> to indicate "the const is a lie", that we can apply to std::pair::first,
> >> with the semantics being that a top-level con
mclow.lists added a comment.
>> One way we could deal with this is by adding an attribute to the compiler to
>> indicate "the const is a lie", that we can apply to std::pair::first, with
>> the semantics being that a top-level const is ignored when determining the
>> "real" type of the member (
erik.pilkington added a comment.
> One way we could deal with this is by adding an attribute to the compiler to
> indicate "the const is a lie", that we can apply to `std::pair::first`, with
> the semantics being that a top-level `const` is ignored when determining the
> "real" type of the memb
rsmith added a comment.
How do you deal with the fact that node handles for map-like types need to be
able to mutate a const member of a `pair` in-place? Are you assuming/hoping the
compiler won't do bad things to the program as a result, or is there some
reason why it would be unlikely to do s
23 matches
Mail list logo
