Re: [PATCH] Derive interface buffers from max name length

[Janne Blomqvist <blomqvist dot janne at gmail dot com>]

On Tue, Dec 1, 2015 at 6:51 PM, Bernhard Reutner-Fischer
<rep.dot.nop@gmail.com> wrote:
> On 1 December 2015 at 15:52, Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
>> On Tue, Dec 1, 2015 at 2:54 PM, Bernhard Reutner-Fischer
>> <rep.dot.nop@gmail.com> wrote:
>>> These three function used a hardcoded buffer of 100 but would be better
>>> off to base off GFC_MAX_SYMBOL_LEN which denotes the maximum length of a
>>> name in any of our supported standards (63 as of f2003 ff.).
>>
>> Please use xasprintf() instead (and free the result, or course). One
>> of my backburner projects is to get rid of these static symbol
>> buffers, and use dynamic buffers (or the symbol table) instead. We
>> IIRC already have some ugly hacks by using hashing to get around
>> GFC_MAX_SYMBOL_LEN when handling mangled symbols. Your patch doesn't
>> make the situation worse per se, but if you're going to fix it, lets
>> do it properly.
>
> I see.
>
> /scratch/src/gcc-6.0.mine/gcc/fortran$ git grep
> "^[[:space:]]*char[[:space:]][[:space:]]*[^[;[:space:]]*\[" | wc -l
> 142
> /scratch/src/gcc-6.0.mine/gcc/fortran$ git grep "xasprintf" | wc -l
> 32

Yes, that's why it's on the TODO-list rather than on the DONE-list. :)

> What about memory fragmentation when switching to heap-based allocation?
> Or is there consensus that these are in the noise compared to other
> parts of the compiler?

Heap fragmentation is an issue, yes. I'm not sure it's that
performance-critical, but I don't think there is any consensus. I just
want to avoid ugly hacks like symbol hashing to fit within some fixed
buffer. Perhaps an good compromise would be something like std::string
with small string optimization, but as you have seen there is some
resistance to C++. But this is more relevant for mangled symbols, so
GFC_MAX_MANGLED_SYMBOL_LEN is more relevant here, and there's only a
few of them left. So, well, if you're sure that mangled symbols are
never copied into the buffers your patch modifies, please consider
your original patch Ok as well. Whichever you prefer.

Performance-wise I think a bigger benefit would be to use the symbol
table more and then e.g. be able to do pointer comparisons rather than
strcmp(). But that is certainly much more work.

> BTW:
> $ git grep APO
> io.c:  static const char *delim[] = { "APOSTROPHE", "QUOTE", "NONE", NULL };
> io.c:  static const char *delim[] = { "APOSTROPHE", "QUOTE", "NONE", NULL };

? What are you saying?



-- 
Janne Blomqvist