[PATCH] PrettyStackTrace should use ManagedStatic for its ThreadLocal

[Filip Pizlo]

On Sep 12, 2013, at 8:48 PM, Dan Gohman <sunfish at google.com> wrote:

> On Thu, Sep 12, 2013 at 6:33 PM, Filip Pizlo <fpizlo at apple.com> wrote:
>> 
>> On Sep 12, 2013, at 5:49 PM, Dan Gohman <sunfish at google.com> wrote:
>> 
>> On Thu, Sep 12, 2013 at 5:00 PM, Filip Pizlo <fpizlo at apple.com> wrote:
>> 
>> llvm::outs() has a static local that closes stdout in its destructor.
>> Obviously, this *really* has to run atexit, and probably can't run at
>> llvm_shutdown() time.  Alternatively, it shouldn't close stdout.
>> 
>> 
>> Is the issue just that you want it to flush its output?
>> 
>> 
>> The issue is that it needs to detect pending I/O errors.
>> 
>> 
>> What would you do with pending I/O errors in an atexit() handler?
> 
> The code is out there :-). It calls report_fatal_error.

Ah, got it - I thought you meant that it had some recovery.  Is it clear that this behavior makes sense for llvm::outs, though?  To be clear, I'm not asking about changing raw_ostream's behavior in general, but changing the behavior of llvm:outs in particular.  We could get the best of both worlds:

- raw_ostream clients that aren't llvm:outs retains all of the current behavior and performance.

- llvm::outs has slightly different behavior, that doesn't require atexit().

> 
>> 
>> 
>> In that case why not do the equivalent of _setvbuf?
>> 
>> 
>> _setvbuf is for libc buffering. raw_ostream's implementation was
>> designed to avoid libc buffering because it wasn't as fast.
>> 
>> 
>> I understand that _setvbuf is for libc.  But why can't raw_ostream do the
>> equivalent for the purpose of llvm::outs()?  Is the concern that a program
>> might use outs() (i.e. outputting to stdout) so heavily that buffering
>> becomes critical?
> 
> I don't understand what you're asking here.

I'm asking because I don't know, and would like to learn.  I find your explanations useful, so that if I try to do more ManagedStatic-like refactorings, I'll know what I'm doing.  There's also the possibility that I'm on to something but just haven't described it clearly enough, yet.

It's preferable (and sometimes obligatory) for non-compiler-tool clients of LLVM to not end up with atexit() handlers installed by LLVM.  The llvm::outs one appears to be less evil and maybe it's even necessary, but I'm not sure I understand why, yet.

> raw_ostream is a simple
> utility for simple compiler-like tools. It's useful for these tools to
> exit with a non-zero exit status if they encounter any output error.
> raw_ostream's current design achieves this automatically, and by
> default (though it can be overridden).

Right, but is llvm::outs() used in a manner where it makes sense for output errors to result in a hard failure?  I.e. is it common for tools to be used in a mode where they output data to fd=1, and is it critical for that use-case to report_fatal_error if the I/O fails (and that error didn't already shoot you down with SIGPIPE or similar)?

-F

> 
> Dan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130912/ecfe16aa/attachment.html>