On Wed, Aug 10, 2016 at 12:12 PM, Aldy Hernandez <al...@redhat.com> wrote:
> On 08/10/2016 06:04 AM, Richard Biener wrote:
>>
>> On Tue, Aug 9, 2016 at 3:17 PM, Aldy Hernandez <al...@redhat.com> wrote:
>>>
>>> On 08/05/2016 01:55 PM, Richard Biener wrote:
>>>
>>> Hi Richard.
>>>
>>>> Please don't use std::string. For string building you can use obstacks.
>>>
>>>
>>>
>>> Alright let's talk details then so I can write things up in a way you
>>> approve of.
>>>
>>> Take for instance simple uses like all the tree_*check_failed routines,
>>> which I thought were great candidates for std::string-- they're going to
>>> be
>>> outputted to the screen or disk which is clearly many times more
>>> expensive
>>> than the malloc or overhead of std::string:
>>>
>>> length += strlen ("expected ");
>>> buffer = tmp = (char *) alloca (length);
>>> length = 0;
>>> while ((code = (enum tree_code) va_arg (args, int)))
>>> {
>>> const char *prefix = length ? " or " : "expected ";
>>>
>>> strcpy (tmp + length, prefix);
>>> length += strlen (prefix);
>>> strcpy (tmp + length, get_tree_code_name (code));
>>> length += strlen (get_tree_code_name (code));
>>> }
>>>
>>> Do you suggest using obstacks here, or did you have something else in
>>> mind?
>>
>>
>> Why would you want to get rid of the alloca here?
>>
>>> How about if it's something like the above but there are multiple exit
>>> points from the function. I mean, we still need to call obstack_free()
>>> on
>>> all exit points, which is what I wanted to avoid for something as simple
>>> as
>>> building a throw-away string.
>>
>>
>> But you'll end up in
>>
>> internal_error ("tree check: %s, have %s in %s, at %s:%d",
>> buffer, get_tree_code_name (TREE_CODE (node)),
>> function, trim_filename (file), line);
>>
>> where you have an interface using C strings again.
>>
>> It's not that the standard library is bad per-se, it's just using a
>> tool that doesn't
>> fit its uses. This makes the code a messy mix of two concepts which is
>> what
>> I object to.
>>
>> Yes, the above example may have premature optimization applied. Is that
>> a reason to re-write it using std::string? No. Is it a reason to rewrite
>> it
>> using obstracks? No. Just leave it alone.
>
>
> How about we use auto_vec<> with an expected sane default? This way we have
> a performance conscious solution that will fallback to malloc if necessary,
> while cleaning up after itself without the need for changing every exit
> point from a function.
>
> For example, the attached untested patch implements this approach for
> tree.c.
>
> Would this be acceptable?
Again - why change way from alloca in the tree_*_check_fail routines?
get_file_function_name may be seen as working on "arbitrary" user input
(filenames), but I'm not sure I'd treat it that way. The function computes
strings that are the same over the whole compilation (but it does so
possibly repeatedly), so simply using malloc and having the result cached
would work here, too.
Richard.
> Aldy
>
