[Parallel_libs-commits] [PATCH] D22687: [StreamExecutor] Add utility library

[Jason Henline via Parallel_libs-commits]

jhen added inline comments.

================
Comment at: streamexecutor/include/streamexecutor/Utils/Error.h:121
@@ +120,3 @@
+/// Or if the containing function returns an Expected<U>, the error can be
+/// passed up the stack:
+///
----------------
jlebar wrote:
> Maybe we should say
> 
> "You can also write the same code if doTask3() returns an Expected<U> instead of an Error."  One less example, and it calls out the equivalence of the code rather than asking the reader to figure out that it's the same.
> 
> On another note, could I do
> 
>   return ExpectedInt;
> 
> instead of 
> 
>   return ExpectedInt.takeError();
> 
> I think so based on the rules above.  Not sure if that's worth mentioning, or if we have a preference between these two ways of doing it.
I combined the task3 and task4 examples to show a few different ways of passing errors up the stack.

================
Comment at: streamexecutor/include/streamexecutor/Utils/Error.h:180
@@ +179,3 @@
+// error.
+static inline std::string getSuccessMessage() { return "success"; }
+
----------------
jlebar wrote:
> I wonder if we need this one.  It may encourage people to do string comparisons to check whether a function returned success, and that's kind of lame.
> 
> What do you think, based on the code you haven't released yet?
I'm fine to get rid of it.


https://reviews.llvm.org/D22687