[clang-tools-extra] r221272 - [clang-tidy] Don't print a message if there's no error.

[David Blaikie]

On Tue, Nov 4, 2014 at 12:23 PM, Justin Bogner <mail at justinbogner.com>
wrote:

> David Blaikie <dblaikie at gmail.com> writes:
> >     This is totally wrong. ErrorOr's implicit bool conversion is true iff
> >     there is an error,
> >
> > Doesn't look like it:
> >
> > ErrorOr.h:
> >   /// \brief Return false if there is an error.
> >   LLVM_EXPLICIT operator bool() const {
> >     return !HasError;
> >   }
>
> Oops, I misinterpreted "An implicit conversion to bool provides a way to
> check if there was an error." - I'll probably clarify that RSN. In any
> case, isn't the added check here redundant then?
>

Yep, looks redundant to me.


>
> Maybe it would avoid confusion to use this (fairly common) pattern:
>
>   llvm::ErrorOr<ClangTidyOptions> ParsedOptions =
>       ConfigHandler.second((*Text)->getBuffer());
>   if (std::error_code EC = ParsedOptions.getError()) {
>

Agreed, this seems fairly canonical.


>     // ...
>   }
>
> >
> >     so !ParsedOptions implies !ParsedOptions.getError().
> >
> >     I think you want:
> >
> >       llvm::ErrorOr<ClangTidyOptions> ParsedOptions =
> >           ConfigHandler.second((*Text)->getBuffer());
> >       if (ParsedOptions) {
> >         llvm::errs() << "Error parsing " << ConfigFile << ": "
> >                      << ParsedOptions.getError().message() << "\n";
> >         ...
> >
> >     This obviously changes the behaviour, but the current behaviour
> doesn't
> >     make sense.
> >
> >     >        continue;
> >     >      }
> >     >
> >     >
> >     >
> >     > _______________________________________________
> >     > cfe-commits mailing list
> >     > cfe-commits at cs.uiuc.edu
> >     > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >     _______________________________________________
> >     cfe-commits mailing list
> >     cfe-commits at cs.uiuc.edu
> >     http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141104/49646cb2/attachment.html>