[PATCH] PrettyStackTrace should use ManagedStatic for its ThreadLocal

[Filip Pizlo]

> On Sep 13, 2013, at 8:36 AM, Dan Gohman <sunfish at google.com> wrote:
> 
>> On Thu, Sep 12, 2013 at 8:56 PM, Filip Pizlo <fpizlo at apple.com> wrote:
>> 
>> 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 the current design, clients can do this:
> 
> int main() {
>  outs() << "hello world\n";
> }
> 
> and unlike the common hello world in standard C++, this program exits
> with a non-zero exit status if it encounters an output error. This
> happens automatically, and by default, which is nice, because it's
> easy to forget error checking if it has to be done manually. Try it
> with a redirect to /dev/full to see it in action, for example. atexit
> is the mechanism that makes this possible. Maybe better designs are
> possible, but this is what the current design is.
> 
>> 
>> 
>> 
>> 
>> 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.
> 
> Clients which don't use llvm::outs won't get an atexit call registered.

Got it!  That's great and addresses my concerns. 

> 
>> 
>> 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)?
> 
> Yes. For example, it's used for compiler-like tools running on systems
> which sometimes run out of disk space, where it's useful to have
> compilations fail instead of silently "succeeding" and letting the
> build haplessly carry on only to fail later in strange ways due to
> having truncated data. And for a compiler-like tool, fd=1 can be an
> output file just like any other.
> 
> Dan