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.

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

[streambuf.virtuals] Simplify the logic of exposition; remove several unneeded lists #1111

Merged
merged 1 commit into from Nov 26, 2016

Conversation

tkoeppe
Copy link
Contributor

@tkoeppe tkoeppe commented Nov 23, 2016

Some of the logic in this section is needlessly convoluted. A slight rewrite makes the exposition more compact and fluent.

Before:

image

After:

image

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Nov 23, 2016

@jwakely, @jensmaurer: Reviews welcome :-)

\tcode{gptr() - eback()}
characters, then the
characters, and then the
\tcode{gptr() - eback()}
Copy link
Member

Choose a reason for hiding this comment

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

"and then" -> "in which case"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, good idea.

@jensmaurer
Copy link
Member

jensmaurer commented Nov 23, 2016

This seems to be correct (i.e. just a reshuffling of words), but I'd appreciate a second opinion.

That said, this section is a prime example for the following unrelated defects (that should be fixed in a separate pull request for different issues):

  • \term instead of \defn
  • \pnum in the middle of library descriptions, where it's unclear whether the paragraph introduces something new or is part of the previous item.

@jensmaurer
Copy link
Member

Btw, this talk about "backup sequence" seems slightly rubbish. We define the backup sequence to be gptr() - eback() characters starting at eback(). And then, in the "usual backup conditions" p13, we talk about "agreeeing" of the backup sequence with exactly these characters (first bullet). I'm confused.

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Nov 23, 2016

I can change the \terms to \defns, but do we really want those terms to be globally defined concepts? Maybe we can index them as "streambuf!pending sequence"?

@jensmaurer
Copy link
Member

Re: \term to \defn: As I said, not in this pull request. For instance, we'll need to check the scope of those terms. If they're just local terms (used in these five paragraphs), maybe we do not want them to be italics to start with. Or we might need a \localterm thing that keeps the italics, doesn't raise a red flag with a "grep -w term", and (optionally?) allows an index entry.

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Nov 23, 2016

Makes sense, thanks. Do you want me to do anything about the "backup sequence" business? I didn't check the non-local context of the section. My goal was to get rid of the enumeratea environment.

@jensmaurer
Copy link
Member

Don't mess with the "backup sequence" or anything else in that section other than shuffling words around. Essentially, "don't ask, don't tell", I'd say. You seem to have achieved the goal of getting rid of the "enumeratea" environment, so let's leave it at that for now. Anything beyond that probably requires an editorial and/or LWG review of all of iostreams.

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Nov 23, 2016

Agreed, thanks!

@tkoeppe tkoeppe merged commit 0b640ed into cplusplus:master Nov 26, 2016
@tkoeppe tkoeppe deleted the io_ar branch November 26, 2016 22:40
@jwakely
Copy link
Member

jwakely commented Nov 30, 2016

Late to the party, but I like this.

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Nov 30, 2016

Thanks! This is part of the battle against enumerations :-)

@jwakely
Copy link
Member

jwakely commented Nov 30, 2016

@jensmaurer I think the usual backup condition stuff is confusing because it talks about the post-Effects backup sequence in terms of the previous backup sequence. If I understand it correctly, "the backup sequence" is defined as the values in the original range [eback(), gptr()), so imagine that was memcpy'd out to some side buffer for later use, call it buf[len] where len is gptr() - eback(). Then the Effects are saying that the new contents of [eback(), gptr()) are [buf, buf+n) where n is min(gptr() - eback(), len)

i.e. if the previous backup sequence was empty, or was only a few characters, the final backup sequence can be less than gptr() - eback() (so isn't required to be filled with characters plucked from thin air).

But we should ask an iostreams expert, maybe it needs an LWG issue, or maybe we just need to clarify that "backup sequence" means "ye olde backup sequence"

Independent of that, I've created pull #1157 for some other tweaks to the underflow wording. Please take a look.

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.

None yet

3 participants