stan-dev / math

The Stan Math Library is a C++ template library for automatic differentiation of any order using forward, reverse, and mixed modes. It includes a range of built-in functions for probabilistic modeling, linear algebra, and equation solving.
https://mc-stan.org
BSD 3-Clause "New" or "Revised" License
745 stars 187 forks source link

convert std::stringstream message use to one-liners #590

Open bob-carpenter opened 7 years ago

bob-carpenter commented 7 years ago

Summary:

We have a lot of code to convert to strings that uses stringstream as a builder:

std::stringstream ss;
ss << "foo" << x << std::endl;
std::string s = ss.str();

I'm pretty sure this is OK as a one-liner:

std::string s = (std::stringstream() << "foo" << x << std::endl).str();

I tested that it works on clang++ and g++ in a simple case. And I think it should work in general because str() returns a copy:

http://www.cplusplus.com/reference/sstream/stringstream/str/

The next question is whether the pattern could be used as an argument, and I think so given that the string is copied.

It would be even better if we could do this without explicitly including <sstream>. But there's nothing in Stan that's automatically brought in, so there's nowhere to put a typedef in stan::math that will automatically get picked up in every file.

Current Version:

v2.16.0

andrjohns commented 7 years ago

Seems to be a bit tricky. Using the << operator with a stringstream returns an ostream, which doesn't have an str() member that can be called:

#include <sstream>

int main(){
  int x;
  std::string s = (std::stringstream() << "foo" << x << std::endl).str();
}

clang++  -std=c++1y test_2.cpp
test_2.cpp:6:67: error: no member named 'str' in 'std::basic_ostream<char>'
        std::string s = (std::stringstream() << "foo" << x << std::endl).str();
                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
1 error generated.

Looks like the general consensus is that you need a helper class if you want stringstream one-liners.

Could use a variadic function as a generic string builder that will work on any number of arguments:

#include <sstream>

template<typename T>
std::string to_str(const T& value){
  std::ostringstream tmp_str;
  tmp_str << value;
  return tmp_str.str();
}

template<typename T, typename ... Args >
std::string to_str(const T& value, const Args& ... args){
  return to_str(value) + to_str(args...);
}

Appears to work across most use-cases:

#include <stan/math.hpp>
#include <to_str.hpp>

int main(){
  std::cout << to_str("a ", 1," b ",3,6) << std::endl
            << to_str(6) << std::endl
            << to_str("") << std::endl
            << to_str(stan::math::normal_lpdf(0,0,1),"\n",
                        stan::math::machine_precision()) << std::endl;

  throw std::runtime_error(to_str("Failed with err ",404));

  return 0;
}

Returns:

a 1 b 36
6

-0.918939
2.22045e-16
terminate called after throwing an instance of 'std::runtime_error'
  what():  Failed with err 404
Aborted (core dumped)
andrjohns commented 7 years ago

Feel free to let me know if I'm completely off-track here, pretty much learning as I go.

sakrejda commented 7 years ago

We hashed some of this out a while ago and I agree variadic template would work.

Discourse thread: http://discourse.mc-stan.org/t/logging-in-stan-services/300/20

bob-carpenter commented 7 years ago

I was thinking everyone missed the obvious template solution until I got to here:

https://groups.google.com/d/msg/comp.lang.c++/_GWLGQhbxYE/TnCk6kVnxuMJ

That'd be the right way to produce a string. That's an independent issue from logging except insofar as the production is in a call to logging, where you don't want to do the production if you're not logging that level. So it really needs to be a lazy expression! I don't know how to do that with templates as you need to know all the types.

sakrejda commented 7 years ago

You lost me Bob.

On Mon, Sep 25, 2017, 1:58 PM Bob Carpenter notifications@github.com wrote:

I was thinking everyone missed the obvious template solution until I got to here:

https://groups.google.com/d/msg/comp.lang.c++/_GWLGQhbxYE/TnCk6kVnxuMJ

That'd be the right way to produce a string. That's an independent issue from logging except insofar as the production is in a call to logging, where you don't want to do the production if you're not logging that level. So it really needs to be a lazy expression! I don't know how to do that with templates as you need to know all the types.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/stan-dev/math/issues/590#issuecomment-331962488, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfA6cdeOOtHM666dsVU6en1d1TzvvDNks5sl-nHgaJpZM4OnYxA .

bob-carpenter commented 7 years ago

That link is the way to write a class that enables << one-liners with an internal stringstream accumulator. It defines a class with the approprite template operator<< that delegates to the stringstream version.

On Sep 25, 2017, at 3:04 PM, Krzysztof Sakrejda notifications@github.com wrote:

You lost me Bob.

On Mon, Sep 25, 2017, 1:58 PM Bob Carpenter notifications@github.com wrote:

I was thinking everyone missed the obvious template solution until I got to here:

https://groups.google.com/d/msg/comp.lang.c++/_GWLGQhbxYE/TnCk6kVnxuMJ

That'd be the right way to produce a string. That's an independent issue from logging except insofar as the production is in a call to logging, where you don't want to do the production if you're not logging that level. So it really needs to be a lazy expression! I don't know how to do that with templates as you need to know all the types.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/stan-dev/math/issues/590#issuecomment-331962488, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfA6cdeOOtHM666dsVU6en1d1TzvvDNks5sl-nHgaJpZM4OnYxA .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

jorgbrown commented 4 years ago

The one-liner you're looking for simply static_casts the ostream back to the ostringstream that you know it is:

string s = static_cast<ostringstream &&>((ostringstream() << "foo" << x << std::endl)).str(); See https://godbolt.org/z/FJkDya for a demo.

p.s. Also, note that std::endl is inefficient in general because it flushes the underlying stream. Doesn't matter much for strings, but for a program that writes a lot of data to std::cout, for example, it's a 10x-20x slowdown. See https://youtu.be/GMqQOEZYVJQ

bob-carpenter commented 4 years ago

We use this pattern everywhere, so it'd be fantastic to clean it up.

We can convert all the std::endl to \n and then flush where we need to (like in print() statements in Stan or exception messages).

That pattern's neat---it took me a second to convince myself there wasn't a dangling reference with the copy in .str() and move semantics. Is there a way we could write that up in a convenient variadic template like:

string s = format_line("foo", x)

We could use parameter packs for arguments and have a move-based return, I'd think.