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

[Justin Bogner]

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?

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()) {
    // ...
  }

>
>     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