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

Add optional length parameter to GenericStringStream constructor #789

Closed
wants to merge 1 commit into from
Closed

Conversation

masseman
Copy link

@masseman masseman commented Nov 4, 2016

In order to be able to reference sub-strings (segments that are not zero-terminated) in GenericStringStream, a length parameter is necessary.

If the length parameter is not used, the compiler will optimize away the comparisons added in Peek() and Tell(), so this modification should not have any impact on performance.

@coveralls
Copy link

coveralls commented Nov 4, 2016

Coverage Status

Coverage remained the same at 99.938% when pulling bed701a on masseman:master into fbcd7ac on miloyip:master.

@stervo4ka
Copy link

stervo4ka commented Nov 4, 2016

If the length parameter is not used, the compiler will optimize away the comparisons added

That's not true in general. Compiler might optimize the comparisons in simple cases, but consider this option:

void f(StringStream &stream) {
    // Do something.
}

In this case compiler cannot know whether the length is used.
Here is a proof https://godbolt.org/g/qMlFyH

@miloyip
Copy link
Collaborator

miloyip commented Nov 4, 2016

There is already a MemoryStream which can be used, though lack of documentation.

@masseman
Copy link
Author

masseman commented Nov 4, 2016

@miloyip Thanks, I will use that instead!
@stervo4ka Good point.

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.

4 participants