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

P1020R1 Smart pointer creation with default initialization #2468

Merged
merged 1 commit into from
Nov 26, 2018
Merged

P1020R1 Smart pointer creation with default initialization #2468

merged 1 commit into from
Nov 26, 2018
+141 −9

Conversation

burblebee
Copy link
Contributor

[memory.syn] Move return types to before function names for consistency.

Fixes #2424.

@jensmaurer jensmaurer added this to the post-2018-11 milestone Nov 14, 2018
@jensmaurer
Copy link
Member

@burblebee, could you please fix the overfull \hbox here? Thanks.

@jensmaurer jensmaurer added the changes requested Changes to the wording or approach have been requested and not yet applied. label Nov 15, 2018
@burblebee
Copy link
Contributor Author

@jensmaurer I think it's complaining about this text in utilities.tex around line 9811 because allocate_shared_default_init doesn't fit:

\item
  When a (sub)object of non-array type \tcode{U} is initialized by
  \tcode{make_shared_default_init} or \tcode{allocate_shared_default_init},
  it is initialized via the expression \tcode{::new(pv) U},
  where \tcode{pv} has type \tcode{void*} and
  points to storage suitable to hold an object of type \tcode{U}.

How to fix???

@tkoeppe
Copy link
Contributor

tkoeppe commented Nov 15, 2018

Feel free to add hyphenation hints (\-) into the code.

@burblebee
Copy link
Contributor Author

Feel free to add hyphenation hints (\-) into the code.

I'm trying to fix similar Overfull issues in motions-2018-11-lwg-4 with no luck, using both \- and \brk{}. In LWG3116 for example (commit a6c9c9e), I've tried the following but still latex complains:

initializing the base class with
\tcode{basic_istream<\brk{}charT, \brk{}traits>(\brk{}addressof(\brk{}sb))}\iref{istream}
and initializing \tcode{sb} with
\tcode{basic_string\-buf<\brk{}charT, \brk{}traits, \brk{}Allocator>(\brk{}which | \brk{}ios_base::in)}\iref{stringbuf.cons}.

Oddly enough, when I misspelled \brk{} as brk{}, that fixed things!

How can I tell exactly what word in the line(s) it's complaining about? The Overfull warning is:

Overfull \hbox (3.5105pt too wide) in paragraph at lines 7951--7960
\T1/lmr/m/n/10 (-20) class with \T1/lmtt/m/n/10 basic_-istream<charT, traits>(a
ddressof(sb)) \T1/lmr/m/n/10 (-20) ([][]27.7.4.1[][]) and ini-tial-iz-ing \T1/l
mtt/m/n/10 sb \T1/lmr/m/n/10 (-20) with \T1/lmtt/m/n/10 basic_-

@burblebee
Copy link
Contributor Author

@tkoeppe I was able to fix motions-2018-11-lwg-18 by forcing a linebreak. It's the cleanest solution wrt readability, and there is no resulting underflow.

@jensmaurer jensmaurer removed the changes requested Changes to the wording or approach have been requested and not yet applied. label Nov 15, 2018
jensmaurer
jensmaurer requested changes Nov 20, 2018
View reviewed changes
source/utilities.tex
template<class T, ...>
shared_ptr<T> make_shared_default_init(@\placeholdernc{args}@);
template<class T, class A, ...>
shared_ptr<T> allocate_shared_default_init(const A& a, @\placeholdernc{args}@);
Copy link
Member

Choose a reason for hiding this comment

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

This seems broken (also in the ingress paper): The whole point of ..._default_init is to have no arguments, so why are we talking about "args" here? Also, the pack-like "..." in the template argument list should go.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, that's to cover "size_t N" for the array case. The not-really parallelism to make_shared(args) is highly misleading.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jensmaurer Can you open a separate issue for this?

@jensmaurer jensmaurer added the changes requested Changes to the wording or approach have been requested and not yet applied. label Nov 20, 2018
@burblebee burblebee removed the changes requested Changes to the wording or approach have been requested and not yet applied. label Nov 23, 2018
@zygoloid
P1020R1 Smart pointer creation with default initialization
5f91adf 
[memory.syn] Move return types to before function names for consistency.
@zygoloid zygoloid force-pushed the motions-2018-11-lwg-18 branch from a332a1d to 5f91adf Compare November 26, 2018 03:52
@zygoloid zygoloid merged commit 9bfa2e5 into master Nov 26, 2018
@jensmaurer jensmaurer deleted the motions-2018-11-lwg-18 branch October 19, 2019 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jensmaurer jensmaurer

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
post-2018-11
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@burblebee @jensmaurer @tkoeppe @zygoloid