[PATCH] D68164: [Commandline] Clear error on raw_ostream when there is parsing error.

Mon Sep 30 22:44:14 PDT 2019

MaskRay added a comment.

In D68164#1689162 <https://reviews.llvm.org/D68164#1689162>, @ychen wrote:

> In D68164#1689155 <https://reviews.llvm.org/D68164#1689155>, @MaskRay wrote:
>
> > > The exit is called twice and somehow the test exit cleanly.
> >
> > The current behavior is undefined. POSIX says "If exit() is called more than once, the behavior is undefined."  Changing exit to abort in D67847 <https://reviews.llvm.org/D67847> will fix the issue. Thanks.
> >
> > The test `; RUN: sh -c 'opt --reject-this-option 2>&-; echo $?; opt -o /dev/null /dev/null 2>&-; echo $?;'` is testing an undefined behavior, but I can see `2>&-` is a simple alternative to a full-fledged test that makes write operations fail with some other errno. My feeling is that you can just update close-stderr.ll to (`not --crash opt --reject-this-option 2>&-`) in D67847 <https://reviews.llvm.org/D67847> and be prepared to update it if build bots on some platforms have different behaviors.
>
>
> sorry, I'm really confused now. Based on the comment in close-stderr.ll, the test is to make sure `abort` does not happen? Why is it checking the undefined behavior?

close-stderr.ll was added to test a particular behavior of an undefined behavior. There was some churn to mark it as fail on Windows or under valgrind. This is a signal that it really should not test the particular behavior. As this patch suggests, to maintain that particular behavior, we would have to make the code complex. Adding a virtual function for this tricky case is something I really do not want to happen... We probably should just use `not --crash opt --reject-this-option 2>&-`.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68164/new/

https://reviews.llvm.org/D68164