Skip to content

Refactoring of conversion functions#686

Merged
grisumbras merged 13 commits intoboostorg:developfrom
grisumbras:conversion-mp_cond
Sep 29, 2022
Merged

Refactoring of conversion functions#686
grisumbras merged 13 commits intoboostorg:developfrom
grisumbras:conversion-mp_cond

Conversation

@grisumbras
Copy link
Copy Markdown
Member

@grisumbras grisumbras commented Feb 28, 2022

The list of changes:

  • take advantage of mp11
  • split container_traits into several traits and helpers
  • split map_traits into several traits
  • use value_type more consistently
  • refactor detail::inserter
  • use detail::inserter with map-like types
  • refactor conversion of tuple-like to array
  • remove conversion from value_ref and don't use value_to/from when constructing value from value_ref
  • check for round-trip
  • user-facing conversion traits
  • opt-in null-like conversion
  • same requirements for value_to and value_from
  • stricter is_map_like
  • stricter is_sequence_like

@grisumbras grisumbras marked this pull request as draft February 28, 2022 09:50
@grisumbras grisumbras force-pushed the conversion-mp_cond branch 2 times, most recently from 83d26dc to 5471363 Compare February 28, 2022 10:11
@cppalliance-bot
Copy link
Copy Markdown

@grisumbras grisumbras force-pushed the conversion-mp_cond branch 5 times, most recently from 7641c3f to a9c6df6 Compare March 9, 2022 15:25
@cppalliance-bot
Copy link
Copy Markdown

@grisumbras grisumbras force-pushed the conversion-mp_cond branch 3 times, most recently from 2c1545d to 37a7594 Compare May 15, 2022 08:47
@cppalliance-bot
Copy link
Copy Markdown

@grisumbras grisumbras force-pushed the conversion-mp_cond branch 3 times, most recently from 5e13585 to 0cbfc1f Compare May 15, 2022 12:04
@cppalliance-bot
Copy link
Copy Markdown

@cppalliance-bot
Copy link
Copy Markdown

@grisumbras grisumbras force-pushed the conversion-mp_cond branch 5 times, most recently from 565943c to 018f286 Compare August 12, 2022 18:20
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 12, 2022

Codecov Report

Merging #686 (ce144b5) into develop (9fc23d6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #686   +/-   ##
========================================
  Coverage    99.13%   99.13%           
========================================
  Files           69       69           
  Lines         6584     6587    +3     
========================================
+ Hits          6527     6530    +3     
  Misses          57       57           
Impacted Files Coverage Δ
include/boost/json/impl/value_ref.hpp 100.00% <ø> (ø)
include/boost/json/detail/value_from.hpp 100.00% <100.00%> (ø)
include/boost/json/detail/value_to.hpp 100.00% <100.00%> (ø)
include/boost/json/detail/value_traits.hpp 100.00% <100.00%> (ø)
include/boost/json/value_from.hpp 100.00% <100.00%> (ø)
include/boost/json/value_to.hpp 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9fc23d6...ce144b5. Read the comment docs.

@cppalliance-bot
Copy link
Copy Markdown

@grisumbras grisumbras force-pushed the conversion-mp_cond branch 2 times, most recently from 9c7f931 to 1e57a31 Compare August 13, 2022 08:40
@cppalliance-bot
Copy link
Copy Markdown

@cppalliance-bot
Copy link
Copy Markdown

An automated preview of the documentation is available at https://686.jsondocs.prtest.cppalliance.org/libs/json/doc/html/index.html

@cppalliance-bot
Copy link
Copy Markdown

@cppalliance-bot
Copy link
Copy Markdown

An automated preview of the documentation is available at https://686.jsondocs.prtest.cppalliance.org/libs/json/doc/html/index.html

1 similar comment
@cppalliance-bot
Copy link
Copy Markdown

An automated preview of the documentation is available at https://686.jsondocs.prtest.cppalliance.org/libs/json/doc/html/index.html

@cppalliance-bot
Copy link
Copy Markdown

An automated preview of the documentation is available at https://686.jsondocs.prtest.cppalliance.org/libs/json/doc/html/index.html

@cppalliance-bot
Copy link
Copy Markdown

An automated preview of the documentation is available at https://686.jsondocs.prtest.cppalliance.org/libs/json/doc/html/index.html

@cppalliance-bot
Copy link
Copy Markdown

An automated preview of the documentation is available at https://686.jsondocs.prtest.cppalliance.org/libs/json/doc/html/index.html

@cppalliance-bot
Copy link
Copy Markdown

@grisumbras grisumbras merged commit ce144b5 into boostorg:develop Sep 29, 2022
@grisumbras grisumbras deleted the conversion-mp_cond branch September 29, 2022 12:46

#include <utility>

BOOST_JSON_NS_BEGIN
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we don't have standalone we don't need this namespace macro

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All public header files need to be listed in <boost/json.hpp>, this one is missing (are any other new files missing?)

#include <utility>

BOOST_JSON_NS_BEGIN
namespace detail {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation details should be in json/detail or json/impl not a public header since this is scanned by doxygen

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stuff should go in <boost/json/detail/conversion.hpp>

;
#endif

/** Determine if `T` can be treated like a 1-to-1 mapping during
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metafunction briefs should start with "Metafunction"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These aren't metafunctions, they are type traits.

Copy link
Copy Markdown
Member

@vinniefalco vinniefalco Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it isn't a function taking a type as an argument and returning a value?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metafunctions return types, these return boolean values, so they are type traits.

<member><link linkend="json.ref.boost__json__is_sequence_like">is_sequence_like</link></member>
<member><link linkend="json.ref.boost__json__is_map_like">is_map_like</link></member>
<member><link linkend="json.ref.boost__json__is_null_like">is_null_like</link></member>
<member><link linkend="json.ref.boost__json__is_tuple_like">is_tuple_like</link></member>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are not in alphabetical order, was this intended?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it was a mistake

@vinniefalco
Copy link
Copy Markdown
Member

This needs exposition in the docs

@vinniefalco
Copy link
Copy Markdown
Member

@pdimov did you review this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants