Re: Mixing nsresult and Result code

[Kris Maglione]

On Tue, May 09, 2017 at 12:50:26PM +, Nicolas B. Pierron wrote:

On 05/07/2017 07:34 PM, Kris Maglione wrote:

 static inline Result
 WrapNSResult(PRStatus aRv)
 {
 if (aRv != PR_SUCCESS) {
 return Err(NS_ERROR_FAILURE);
 }
 return Ok();
 }

 static inline Result
 WrapNSResult(nsresult aRv)
 {
 if (NS_FAILED(aRv)) {
 return Err(aRv);
 }
 return Ok();
 }

 #define NS_TRY(expr) MOZ_TRY(WrapNSResult(expr))


This sounds like we could have a template function named ToResult(), 
which is used by the MOZ_TRY macro to coerce a given type into the 
equivalent mozilla::Result<> type.  This ToResult function could be 
specialized to forward the Result<…> arguments by default.


Yeah, I wasn't especially happy with the NS_TRY name either, and 
just handling the case for MOZ_TRY had occurred to me. I mainly 
went with a separate macro to try to avoid confusion, but I'd 
definitely be happy to reuse MOZ_TRY.


The main concern that I have with that approach at that point is 
avoiding unnecessary copies or moves. I'm not sure how well the 
compiler would legally be able to optimize that for complex 
Result types.


Another concern would be the representation of the Result (or Result ?), which should definitely 
be represented as an nsresult, as this enum is capable of representing 
success with NS_OK, and also various other forms of success values.  
There is already a way to specialize the PackingStrategy of the Result 
class to consume less space, and it can be used to specialize the way 
we encode such Result class, to avoid the overhead of 
duplicating the success flag.


This sounds like a good idea. We'd have to assert that NS_OK was 
never used as the failure value, but it seems doable.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Mixing nsresult and Result code

[Nicolas B. Pierron]

On 05/07/2017 07:34 PM, Kris Maglione wrote:

  static inline Result
  WrapNSResult(PRStatus aRv)
  {
  if (aRv != PR_SUCCESS) {
  return Err(NS_ERROR_FAILURE);
  }
  return Ok();
  }

  static inline Result
  WrapNSResult(nsresult aRv)
  {
  if (NS_FAILED(aRv)) {
  return Err(aRv);
  }
  return Ok();
  }

  #define NS_TRY(expr) MOZ_TRY(WrapNSResult(expr))


This sounds like we could have a template function named ToResult(), which 
is used by the MOZ_TRY macro to coerce a given type into the equivalent 
mozilla::Result<> type.  This ToResult function could be specialized to 
forward the Result<…> arguments by default.


 #define MOZ_TRY(expr) \
   do { \
-auto mozTryTempResult_ = (expr); \
+auto mozTryTempResult_ = ::mozilla::ToResult(expr); \
 if (mozTryTempResult_.isErr()) { \
   return ::mozilla::Err(mozTryTempResult_.unwrapErr()); \
 } \
   } while (0)

Another concern would be the representation of the Result (or 
Result ?), which should definitely be represented as 
an nsresult, as this enum is capable of representing success with NS_OK, and 
also various other forms of success values.  There is already a way to 
specialize the PackingStrategy of the Result class to consume less space, 
and it can be used to specialize the way we encode such Result 
class, to avoid the overhead of duplicating the success flag.


--
Nicolas B. Pierron
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Mixing nsresult and Result code

[Michael Layzell]

I also don't like the NS_TRY name, it seems too close to MOZ_TRY. I would
prefer to avoid names which are identical except s/NS/MOZ/.

Perhaps NSRESULT_TRY would be a better name? It makes it clear that it is
performing the try against the nsresult type.

On Mon, May 8, 2017 at 10:17 AM, Ehsan Akhgari 
wrote:

> I think these seem like valuable additions to nscore.h.  It would be
> helpful to extend these facilities that would allow more code to use the
> Result-based programming model.
>
> (I'm not too much of a fan of the NS_TRY name, but can't think of a better
> name myself... :/ )
>
>
>
> On 05/07/2017 03:34 PM, Kris Maglione wrote:
>
>> I've been trying to write most of my new code to use Result.h as much as
>> possible. When I have to mix Result-based code with nsresult-based code,
>> though, things tend to get ugly, and I wind up writing helpers. At this
>> point I've wound up writing the same helpers in 3 or 4 different places, so
>> it may be time to try to standardize on something.
>>
>> The helpers I've been using look something like:
>>
>>  template <>
>>  class MOZ_MUST_USE_TYPE GenericErrorResult
>>  {
>>nsresult mErrorValue;
>>
>>template friend class Result;
>>
>>  public:
>>explicit GenericErrorResult(nsresult aErrorValue) :
>> mErrorValue(aErrorValue) {}
>>
>>operator nsresult() { return mErrorValue; }
>>  };
>>
>>  static inline Result
>>  WrapNSResult(PRStatus aRv)
>>  {
>>  if (aRv != PR_SUCCESS) {
>>  return Err(NS_ERROR_FAILURE);
>>  }
>>  return Ok();
>>  }
>>
>>  static inline Result
>>  WrapNSResult(nsresult aRv)
>>  {
>>  if (NS_FAILED(aRv)) {
>>  return Err(aRv);
>>  }
>>  return Ok();
>>  }
>>
>>  #define NS_TRY(expr) MOZ_TRY(WrapNSResult(expr))
>>
>> And their use looks something like:
>>
>>  Result
>>  GetFile(nsIFile* aBase, const char* aChild)
>>  {
>>nsCOMPtr file;
>>NS_TRY(aBase->Clone(getter_AddRefs(file)));
>>NS_TRY(aBase->AppendNative(aChild));
>>
>>return Move(file);
>>  }
>>
>>  nsresult
>>  ReadFile(const char* aPath)
>>  {
>>nsCOMPtr file;
>>MOZ_TRY_VAR(file, GetFile(mBase, aPath));
>>
>>PRFileDesc* fd;
>>NS_TRY(file->OpenNSPRFileDesc(PR_RDONLY, 0, ));
>>
>>...
>>return NS_OK;
>>  }
>>
>> Where NS_TRY converts a nsresult or PRStatus to an appropriate Result or
>> GenericErrorResult, and the GenericErrorResult specialization
>> automatically converts an nsresult when used in nsresult-based code.
>>
>> Does this seem like the kind of thing we should implement in some
>> standard header, or would a different approach be better?
>>
>> -Kris
>> ___
>> dev-platform mailing list
>> dev-platform@lists.mozilla.org
>> https://lists.mozilla.org/listinfo/dev-platform
>>
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Mixing nsresult and Result code

[Ehsan Akhgari]

I think these seem like valuable additions to nscore.h.  It would be 
helpful to extend these facilities that would allow more code to use the 
Result-based programming model.


(I'm not too much of a fan of the NS_TRY name, but can't think of a 
better name myself... :/ )



On 05/07/2017 03:34 PM, Kris Maglione wrote:
I've been trying to write most of my new code to use Result.h as much 
as possible. When I have to mix Result-based code with nsresult-based 
code, though, things tend to get ugly, and I wind up writing helpers. 
At this point I've wound up writing the same helpers in 3 or 4 
different places, so it may be time to try to standardize on something.


The helpers I've been using look something like:

 template <>
 class MOZ_MUST_USE_TYPE GenericErrorResult
 {
   nsresult mErrorValue;

   template friend class Result;

 public:
   explicit GenericErrorResult(nsresult aErrorValue) : 
mErrorValue(aErrorValue) {}


   operator nsresult() { return mErrorValue; }
 };

 static inline Result
 WrapNSResult(PRStatus aRv)
 {
 if (aRv != PR_SUCCESS) {
 return Err(NS_ERROR_FAILURE);
 }
 return Ok();
 }

 static inline Result
 WrapNSResult(nsresult aRv)
 {
 if (NS_FAILED(aRv)) {
 return Err(aRv);
 }
 return Ok();
 }

 #define NS_TRY(expr) MOZ_TRY(WrapNSResult(expr))

And their use looks something like:

 Result
 GetFile(nsIFile* aBase, const char* aChild)
 {
   nsCOMPtr file;
   NS_TRY(aBase->Clone(getter_AddRefs(file)));
   NS_TRY(aBase->AppendNative(aChild));

   return Move(file);
 }

 nsresult
 ReadFile(const char* aPath)
 {
   nsCOMPtr file;
   MOZ_TRY_VAR(file, GetFile(mBase, aPath));

   PRFileDesc* fd;
   NS_TRY(file->OpenNSPRFileDesc(PR_RDONLY, 0, ));

   ...
   return NS_OK;
 }

Where NS_TRY converts a nsresult or PRStatus to an appropriate Result 
or GenericErrorResult, and the GenericErrorResult 
specialization automatically converts an nsresult when used in 
nsresult-based code.


Does this seem like the kind of thing we should implement in some 
standard header, or would a different approach be better?


-Kris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Mixing nsresult and Result code

[Kris Maglione]

I've been trying to write most of my new code to use Result.h as 
much as possible. When I have to mix Result-based code with 
nsresult-based code, though, things tend to get ugly, and I wind 
up writing helpers. At this point I've wound up writing the same 
helpers in 3 or 4 different places, so it may be time to try to 
standardize on something.


The helpers I've been using look something like:

 template <>
 class MOZ_MUST_USE_TYPE GenericErrorResult
 {
   nsresult mErrorValue;

   template friend class Result;

 public:
   explicit GenericErrorResult(nsresult aErrorValue) : mErrorValue(aErrorValue) 
{}

   operator nsresult() { return mErrorValue; }
 };

 static inline Result
 WrapNSResult(PRStatus aRv)
 {
 if (aRv != PR_SUCCESS) {
 return Err(NS_ERROR_FAILURE);
 }
 return Ok();
 }

 static inline Result
 WrapNSResult(nsresult aRv)
 {
 if (NS_FAILED(aRv)) {
 return Err(aRv);
 }
 return Ok();
 }

 #define NS_TRY(expr) MOZ_TRY(WrapNSResult(expr))

And their use looks something like:

 Result
 GetFile(nsIFile* aBase, const char* aChild)
 {
   nsCOMPtr file;
   NS_TRY(aBase->Clone(getter_AddRefs(file)));
   NS_TRY(aBase->AppendNative(aChild));

   return Move(file);
 }

 nsresult
 ReadFile(const char* aPath)
 {
   nsCOMPtr file;
   MOZ_TRY_VAR(file, GetFile(mBase, aPath));

   PRFileDesc* fd;
   NS_TRY(file->OpenNSPRFileDesc(PR_RDONLY, 0, ));

   ...
   return NS_OK;
 }

Where NS_TRY converts a nsresult or PRStatus to an appropriate Result or 
GenericErrorResult, and the GenericErrorResult 
specialization automatically converts an nsresult when used in 
nsresult-based code.


Does this seem like the kind of thing we should implement in some standard 
header, or would a different approach be better?


-Kris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform