[PATCH] D51490: [Error] Fix template deduction in createFileError

[Zachary Turner via llvm-commits]

That works, I agree it doesn’t make sense to have descendants of
ErrorSuccess.

Lgtm
On Fri, Sep 7, 2018 at 6:55 AM Alexandre Ganea via Phabricator <
reviews at reviews.llvm.org> wrote:

> aganea updated this revision to Diff 164412.
> aganea added a reviewer: lhames.
> aganea added a comment.
>
> Updated diff. I made `ErrorSuccess` a `final` class. That makes things
> simpler and solves the issue for `Expected` as well.
>
> Please let me know if that sounds better.
>
>
> https://reviews.llvm.org/D51490
>
> Files:
>   include/llvm/Support/Error.h
>   unittests/Support/ErrorTest.cpp
>
>
> Index: unittests/Support/ErrorTest.cpp
> ===================================================================
> --- unittests/Support/ErrorTest.cpp
> +++ unittests/Support/ErrorTest.cpp
> @@ -874,6 +874,9 @@
>        },
>        "");
>  #endif
> +  // Not allowed, would fail at compile-time
> +  //consumeError(createFileError("file.bin", ErrorSuccess()));
> +
>    Error E1 = make_error<CustomError>(1);
>    Error FE1 = createFileError("file.bin", std::move(E1));
>    EXPECT_EQ(toString(std::move(FE1)).compare("'file.bin': CustomError
> {1}"), 0);
> Index: include/llvm/Support/Error.h
> ===================================================================
> --- include/llvm/Support/Error.h
> +++ include/llvm/Support/Error.h
> @@ -322,7 +322,7 @@
>  /// Subclass of Error for the sole purpose of identifying the success
> path in
>  /// the type system. This allows to catch invalid conversion to
> Expected<T> at
>  /// compile time.
> -class ErrorSuccess : public Error {};
> +class ErrorSuccess final : public Error {};
>
>  inline ErrorSuccess Error::success() { return ErrorSuccess(); }
>
> @@ -1171,8 +1171,7 @@
>  /// show more detailed information to the user.
>  class FileError final : public ErrorInfo<FileError> {
>
> -  template <class Err>
> -  friend Error createFileError(std::string, Err);
> +  friend Error createFileError(std::string, Error);
>
>  public:
>    void log(raw_ostream &OS) const override {
> @@ -1207,11 +1206,12 @@
>
>  /// Concatenate a source file path and/or name with an Error. The
> resulting
>  /// Error is unchecked.
> -template <class Err>
> -inline Error createFileError(std::string F, Err E) {
> +inline Error createFileError(std::string F, Error E) {
>    return FileError::build(F, std::move(E));
>  }
>
> +Error createFileError(std::string F, ErrorSuccess) = delete;
> +
>  /// Helper for check-and-exit error handling.
>  ///
>  /// For tool use only. NOT FOR USE IN LIBRARY CODE.
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180907/c3fa8fca/attachment.html>