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

[Ilya Biryukov via Phabricator via llvm-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM from my side, SFINAE check looks correct.
Not familiar with the code, though, so we should probably wait for an approval from @zturner too.



================
Comment at: include/llvm/Support/Error.h:1210-1216
+template <class Err, typename = typename std::enable_if<
+                         std::is_base_of<Error, Err>::value &&
+                             !std::is_base_of<ErrorSuccess, Err>::value,
+                         Err>::type>
 inline Error createFileError(std::string F, Err E) {
   return FileError::build(F, std::move(E));
 }
----------------
zturner wrote:
> What if you just write:
> 
> `Error createFileError(std::string F, ErrorSuccess) = delete;`
> 
> Does that also solve the problem?
I think this won't work if we want it to work for descendants of ErorrSucces. The reason is that the template would still be preferred for the derived classes, e.g.

```
class MyErrorSuccess : public ErrorSuccess {
  ///... 
};

void test() {
  createFile("foo", MyErrorSuccess()); // <-- chooses a template, not `=delete`d function.
}
```

The reason is that the non-template overload needs a derived-to-base conversion for the second arg, so it's considered "worse" than the template one for the purpose of the overloading.

Haven't checked with the compiler, though.


Repository:
  rL LLVM

https://reviews.llvm.org/D51490