Skip to content

[istream.sentry] Minor tweaks #8198

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[istream.sentry] Minor tweaks #8198

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b28d744
[istream.sentry] Minor tweaks
cpplearner Sep 1, 2025
61aaf2e
Address review comments
cpplearner Nov 19, 2025
a41ccee
Address review comments
cpplearner Nov 20, 2025
38963ad
Address review comments
cpplearner Nov 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions source/iostreams.tex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4571,7 +4571,7 @@
that are part of the library.
The semantics of the constructor used in user code is as specified.
\end{footnote}
If \tcode{noskipws} is zero and
If \tcode{noskipws} is \tcode{false} and
\tcode{is.flags() \& ios_base::skipws}
is nonzero, the function extracts and discards each character as long as
the next available input character \tcode{c} is a whitespace character.
Expand Down Expand Up @@ -4600,8 +4600,8 @@
To decide if the character \tcode{c} is a whitespace character,
Copy link
Contributor

@timsong-cpp timsong-cpp Nov 13, 2025

Choose a reason for hiding this comment

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

While we are here, we can also drop the quote of the constructor signature in the previous paragraph.

Like...it's a Remarks for this constructor. Do we really need to say yet again that we are talking about this constructor?

the constructor performs as if it executes the following code fragment:
\begin{codeblock}
const ctype<charT>& ctype = use_facet<ctype<charT>>(is.getloc());
if (ctype.is(ctype.space, c) != 0)
const ctype<charT>& ct = use_facet<ctype<charT>>(is.getloc());
if (ct.is(ct.space, c))
// \tcode{c} is a whitespace character.
\end{codeblock}

Expand All @@ -4610,7 +4610,7 @@
\tcode{is.good()}
is
\tcode{true},
\tcode{\exposid{ok_} != false}
\tcode{\exposid{ok_} != false};
Copy link
Member

@eisenwave eisenwave Nov 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
\tcode{\exposid{ok_} != false};
\tcode{\exposid{ok_}} is \tcode{false};

The == false below should also be changed for local consistency; then we have local consistency about using "is true"/"is false" in this \itemdescr. Not fixing this would be silly if we're already touching this line.

Copy link
Contributor

@tkoeppe tkoeppe Nov 7, 2025

Choose a reason for hiding this comment

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

The original wording is awfully terse. Can we spell this out a bit more, like:

"""
If, after any preparation is completed, is.good() is true, then ok_ is true, otherwise ok_ is false.
"""

Or could we deunpack this even further and say something like "the value of ok_ is is.good()."?

@jwakely?

Copy link
Member

@jwakely jwakely Nov 13, 2025

Choose a reason for hiding this comment

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

Well we definitely can't change "ok_ != false" to "ok_ is false"!

I agree with tkoeppe's suggestion, including adding "then" after the third comma.

We could also just say:

After any preparation is completed, ok_ is set to the value of is.good().

Just saying ok_ = is.good() seems much simpler than describing the following logic, which would never pass code review:

if (is.good() == true)
  ok_ = !false;
else
  ok_ = false;

Copy link
Member

@jwakely jwakely Nov 13, 2025

Choose a reason for hiding this comment

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

Anything would be better than what's in the draft today ;)

Copy link
Contributor

@timsong-cpp timsong-cpp Nov 13, 2025

Choose a reason for hiding this comment

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

Also, this sentence should probably be part of the effects instead of remarks?

otherwise,
\tcode{\exposid{ok_} == false}.
During preparation, the constructor may call
Expand Down