Skip to content

Conversation

@xbauch
Copy link
Contributor

@xbauch xbauch commented Feb 25, 2020

when linking the cprover library. The default mode for this constructor of
stringstream is to over-write the string used in construction, which presumably
was not the intention here.

Note: until now there wasn't anything too interesting in the prologue except for
the string-abstraction define, which was ignored. But we might want more defines
in the future.

  • Each commit message has a non-empty body, explaining why the change was made.
  • Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • White-space or formatting changes outside the feature-related changed lines are in commits of their own.

when linking the cprover library. The default mode for this constructor of
stringstream is to over-write the string used in construction, which presumably
was not the intention here.

Note: until now there wasn't anything too interesting in the prologue except for
the string-abstraction define, which was ignored. But we might want more defines
in the future.
std::ostringstream library_text(prologue);
// the default mode is ios_base::out which means subsequent write to the
// stream will overwrite the original content
std::ostringstream library_text(prologue, std::ios_base::ate);
Copy link
Contributor

@hannes-steffenhagen-diffblue hannes-steffenhagen-diffblue Feb 25, 2020

Choose a reason for hiding this comment

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

#include <iostream>
#include <sstream>
int main(void) {
  std::ostringstream send_help{"SOS"};
  send_help << "WAT";
  std::cout << send_help.str() << '\n';
}

Try it online!

Copy link
Contributor

Choose a reason for hiding this comment

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

WAT is the point of this constructor...
A potentially more obvious fix might be

std::ostringstream library_text;
library_text << prologue

Choose a reason for hiding this comment

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

@thk123 In a slightly less humorous note, I can see uses for this (for example, if you need to guarantee some minimum width for the output so you add in a bunch of filler characters to override first), the “wtf” here is that this is the default, which I think few sane people would expect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

std::ostringstream library_text(prologue);
library_text.seekp(0, library_text.end);

would also work.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd personally lean either towards the solution @thk123 suggested, or to the original solution @xbauch had in the PR. I think the alternative using seekp just looks even more odd :-) Though I appreciate the oddness comes from the C++ spec here.

@hannes-steffenhagen-diffblue
Copy link
Contributor

Turning it off

@hannes-steffenhagen-diffblue
Copy link
Contributor

And turning it on again

@thk123
Copy link
Contributor

thk123 commented Apr 9, 2020

Superseded by #5285

@thk123 thk123 closed this Apr 9, 2020
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.

6 participants