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

[David Blaikie]

On Tue, Nov 4, 2014 at 10:57 AM, Justin Bogner <mail at justinbogner.com>
wrote:

> Alexander Kornienko <alexfh at google.com> writes:
> > Author: alexfh
> > Date: Tue Nov  4 09:25:22 2014
> > New Revision: 221272
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=221272&view=rev
> > Log:
> > [clang-tidy] Don't print a message if there's no error.
> >
> > Modified:
> >     clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp
> >
> > Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp?rev=221272&r1=221271&r2=221272&view=diff
> >
> ==============================================================================
> > --- clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp (original)
> > +++ clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp Tue Nov  4
> 09:25:22 2014
> > @@ -235,8 +235,9 @@ FileOptionsProvider::TryReadConfigFile(S
> >      llvm::ErrorOr<ClangTidyOptions> ParsedOptions =
> >          ConfigHandler.second((*Text)->getBuffer());
> >      if (!ParsedOptions) {
> > -      llvm::errs() << "Error parsing " << ConfigFile << ": "
> > -                   << ParsedOptions.getError().message() << "\n";
> > +      if (ParsedOptions.getError())
> > +        llvm::errs() << "Error parsing " << ConfigFile << ": "
> > +                     << ParsedOptions.getError().message() << "\n";
>
> 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;
  }


> 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/1a2e67c1/attachment.html>