Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add any_allocator #112

Merged
merged 1 commit into from Jan 19, 2022
Merged

add any_allocator #112

merged 1 commit into from Jan 19, 2022

Conversation

alandefreitas
Copy link
Member

A type-erased allocator with shared ownership that satisfies the requirements for Allocator.

@cppalliance-bot
Copy link

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

@cppalliance-bot
Copy link

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

@cppalliance-bot
Copy link

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

@codecov
Copy link

codecov bot commented Jan 16, 2022 •
edited

Codecov Report

Merging #112 (c00ae2f) into develop (cbd0ad6) will increase coverage by 0.05%.
The diff coverage is 100.00%.

❗ Current head c00ae2f differs from pull request most recent head 432f0e1. Consider uploading reports for the commit 432f0e1 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #112      +/-   ##
===========================================
+ Coverage    96.17%   96.22%   +0.05%     
===========================================
  Files          103      107       +4     
  Lines         5406     5487      +81     
===========================================
+ Hits          5199     5280      +81     
  Misses         207      207
Impacted Files Coverage Δ
include/boost/url/any_allocator.hpp 100.00% <100.00%> (ø)
include/boost/url/detail/any_allocator.hpp 100.00% <100.00%> (ø)
include/boost/url/detail/type_id.hpp 100.00% <100.00%> (ø)
include/boost/url/impl/any_allocator.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 cbd0ad6...432f0e1. Read the comment docs.

@alandefreitas alandefreitas changed the title any_allocator class add any_allocator Jan 16, 2022
@cppalliance-bot
Copy link

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

@cppalliance-bot
Copy link

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

@cppalliance-bot
Copy link

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

@cppalliance-bot
Copy link

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

@alandefreitas alandefreitas force-pushed the any_allocator branch 2 times, most recently from a42432a to 771ec8b Compare January 17, 2022 02:03
@cppalliance-bot
Copy link

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

@cppalliance-bot
Copy link

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

@cppalliance-bot
Copy link

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

vinniefalco
vinniefalco reviewed Jan 18, 2022
View reviewed changes
include/boost/url/any_allocator.hpp Outdated
@par Specification
@li <a
href="https://en.cppreference.com/w/cpp/named_req/Allocator"
>Allocator (cppreference.com)</a>
Copy link
Member

Choose a reason for hiding this comment

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

Sometimes you have to break the rules. In this case I would format it thusly:

    @par Specification
    @li <a href="https://en.cppreference.com/w/cpp/named_req/Allocator"
        >Allocator (cppreference.com)</a>

Copy link
Member Author

Choose a reason for hiding this comment

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

Even if it goes on "forever"? That's more than 100 chars.

vinniefalco
vinniefalco reviewed Jan 18, 2022
View reviewed changes
include/boost/url/any_allocator.hpp
#endif

} // namespace urls
} // namespace boost
Copy link
Member

Choose a reason for hiding this comment

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

I don't bother putting the word "namespace" because it doesn't add any information, but if you feel better with it there, I won't complain. See:
https://github.com/boostorg/beast/blob/5bdd1efb8be740bb5425adc054c990818134f3e2/include/boost/beast/http/basic_parser.hpp#L691

Copy link
Member Author

Choose a reason for hiding this comment

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

So can I leave the word "namespace"? That would be one less thing to get wrong because it's how it's generated automatically.

Copy link
Member

Choose a reason for hiding this comment

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

you can leave it in if you want, but I think you will over time learn that these "automatic" formatting tools are going to cause you more work than they relieve.

vinniefalco
vinniefalco reviewed Jan 18, 2022
View reviewed changes
include/boost/url/detail/any_allocator.hpp Outdated
template <std::size_t N>
typename std::enable_if <
N< alignof(max_align_t), void*>::type
try_allocate_aligned(std::size_t n,
Copy link
Member

Choose a reason for hiding this comment

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

Something weird is going on here with the margin

Copy link
Member Author

Choose a reason for hiding this comment

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

Should it be larger? That was my fault. It was trying to do that triangle and trapeziums, etc...

@cppalliance-bot
Copy link

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

@alandefreitas alandefreitas force-pushed the any_allocator branch 2 times, most recently from 8f09c73 to ba98fc9 Compare January 18, 2022 20:58
@cppalliance-bot
Copy link

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

@cppalliance-bot
Copy link

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

@cppalliance-bot
Copy link

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

@cppalliance-bot
Copy link

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

@cppalliance-bot
Copy link

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

@vinniefalco vinniefalco merged commit a6bcad9 into boostorg:develop Jan 19, 2022
@alandefreitas alandefreitas deleted the any_allocator branch January 19, 2022 18:38
@alandefreitas alandefreitas mentioned this pull request Jul 12, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vinniefalco vinniefalco vinniefalco left review comments

@pdimov pdimov pdimov left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@alandefreitas @cppalliance-bot @vinniefalco @pdimov