On Mon, Aug 08, 2016 at 11:00:28AM -0600, Jeff Law wrote: > On 08/06/2016 09:08 AM, Richard Biener wrote: > > On August 6, 2016 12:15:26 PM GMT+02:00, Aldy Hernandez <al...@redhat.com> > > wrote: > > > On 08/05/2016 04:07 PM, Richard Biener wrote: > > > > On August 5, 2016 8:15:54 PM GMT+02:00, Oleg Endo > > > <oleg.e...@t-online.de> wrote: > > > > > On Fri, 2016-08-05 at 19:55 +0200, Richard Biener wrote: > > > > > > > > > > > > > > > > > Please don't use std::string. For string building you can use > > > > > > obstacks. > > > > > > > > > > > > > > > > Just out of curiosity ... why? I remember there was some discussion > > > > > about it, what was the conclusion? Is that now a general rule or > > > does > > > > > it depend on the context where strings are used? > > > > > > > > Because you make a messy mix of string handling variants. > > > Std::string is not powerful enough to capture all uses, it is vastly > > > more expensive to embed into structs and it pulls in too much headers. > > > > (Oh, and I hate I/o streams even more) > > > > > > Oh, and there is prior use in ipa-chkp.c, although I suppose it > > > could've > > > crept in: > > > > Definitely. > > > > > > > > std::string s; > > > > > > /* called_as_built_in checks DECL_NAME to identify calls to > > > builtins. We want instrumented calls to builtins to be > > > recognized by called_as_built_in. Therefore use original > > > DECL_NAME for cloning with no prefixes. */ > > > s = IDENTIFIER_POINTER (DECL_NAME (fndecl)); > > > s += ".chkp"; > > > DECL_NAME (new_decl) = get_identifier (s.c_str ()); > > > > > > You can't tell me obstacks are easier on the eyes for this ;-). > > > > Even strcat is shorter and cheaper. > ISTM that unless the code is performance critical we should be writing code > that is easy to understand and hard to get wrong.
it seems hard to disagree with that ;) > Thus when we have something that is non-critical and can be handled by std:: > routines we should be using them. > > If performance is important in a particular piece of code an obstack or > explicit malloc/free seems better. I'm not totally convinced we have to trade off performance against a good interface. It seems to me it wouldn't be that complicated to build a string class on top of auto_vec that has a similar interface to std::string. I'd really rather not have to write a string class of our own, but I can see some significant advantages to it. First sizeof std::string is 32 on x86_64, a char *, a size_t for the length, and a 16 byte union of a size_t for allocated size and a 16 byte buffer for short strings. I suppose some of this is required by the C++ standard, but I doubt we really need to care about strings longer than 2^32 in gcc. If we put the length and allocated size in the buffer, and have a separate class for stack allocated buffers I think we can have a string that is just sizeof void *. Second it would be useful performance wise to have a std::string_view type class, but that is c++14 or 17? only so we'd need to import it into gcc or something. Trev
