Hi Janne,
>> here is another attempt to make gfortran support user-requested backtraces.
>>
>> [...]
>>
>> Ok for trunk?
>
> Some comments.
thanks for your comments ...
> - I'd prefer to reverse the name, e.g. BACKTRACE_{SHOW,PRINT,SPLURGE},
> to make it more "discoverable" when browsing the manual.
> BACKTRACE_PRINT would have the advantage of matching backtrace_print()
> from libbacktrace, but then again that function takes a couple of
> arguments so perhaps it would cause more confusion than enlightenment.
well, yeah. There was discussion about the naming before. I don't have
a very strong opinion on that, and I just used what is there in
libgfortran, namely SHOW_BACKTRACE.
IMHO any reasonably intelligent person would be capable of using the
"search"-feature of his browser of pdf viewer to find any *BACKTRACE*
name in the list of intrinsics, but I guess there is no harm in
starting the name with BACKTRACE.
So, in principle I'm fine with all your BACKTRACE_* variants (except
for _splurge, maybe ;)
Or, why not just (plain and simple) "BACKTRACE"?
> - As previously show_backtrace() was always followed by program
> termination, we now need to ensure that it properly cleans up after
> itself in case the application continues execution. In particular,
> make sure it doesn't leak file descriptors, and that the addr2line
> child process terminates properly.
Good point. Do you have any particular suggestions about what would be
needed in this direction? (You're probably much more familiar with the
libgfortran code than I am.)
> - In the beginning of show_backtrace(), we print a line "Backtrace for
> this error:". Now that we allow the user to initiate a backtrace print
> at any point, this line may not be what the user wants. I suggest
> moving that line to runtime/compile_options.c (show_signal).
Yes, I also noticed this. I think your suggestion makes sense.
> Otherwise the general idea is Ok, IMHO.
Thanks again for the review!
Cheers,
Janus
