[PATCH] D27751: [LLVM] Use after move bug fixes
Piotr Padlewski via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 23 04:23:32 PST 2016
Prazek added a reviewer: lhames.
Prazek added inline comments.
================
Comment at: lib/Object/Error.cpp:94
return Err2;
- return Err;
+ return Err; // Use after move, not sure how to fix it.
}
----------------
mehdi_amini wrote:
> @lhames : can you look at this?
ping
================
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:
> 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);
> });
> ````
Both seems fine, but I guess only the first one will get rid of the warning, because compiler will not be able to reason that handleErrors is noreturn because each lambda is noreturn
https://reviews.llvm.org/D27751
More information about the llvm-commits
mailing list