[PATCH] D27751: [LLVM] Use after move bug fixes

Mehdi AMINI via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Dec 17 16:13:11 PST 2016


mehdi_amini added inline comments.


================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:53
   if (E.isA<InstrProfError>()) {
     handleAllErrors(std::move(E), [&](const InstrProfError &IPE) {
       instrprof_error instrError = IPE.get();
----------------
mehdi_amini wrote:
> vsk wrote:
> > mehdi_amini wrote:
> > > I'm wondering why there is a check for error before calling handleAllErrors here? (@lhames while you're at it...)
> > If it is an instrprof error, we'd like the chance to inspect it and display an additional hint, instead of simply printing out a string representation of the error.
> The check still does not seem idiomatic to me, it could be instead:
> 
> ```
>     E = handleErrors(std::move(E), [&](const InstrProfError &IPE) {
>      instrprof_error instrError = IPE.get();
>       StringRef Hint = "";
>       if (instrError == instrprof_error::unrecognized_format) {
>         // Hint for common error of forgetting -sample for sample profiles.
>         Hint = "Perhaps you forgot to use the -sample option?";
>       }
>       exitWithError(IPE.message(), Whence, Hint);
>     });
>     exitWithError(toString(std::move(E)), Whence);
> ````
I *think* you can also write:

```
    handleAllErrors(std::move(E),
      [&](const InstrProfError &IPE) {
       instrprof_error instrError = IPE.get();
        StringRef Hint = "";
        if (instrError == instrprof_error::unrecognized_format) {
          // Hint for common error of forgetting -sample for sample profiles.
          Hint = "Perhaps you forgot to use the -sample option?";
        }
        exitWithError(IPE.message(), Whence, Hint);
      },
      [&](const Error &E) {
        exitWithError(toString(E), Whence);
    });
````


https://reviews.llvm.org/D27751





More information about the llvm-commits mailing list