bison-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: multistart: returning structs


From: Akim Demaille
Subject: Re: multistart: returning structs
Date: Tue, 29 Sep 2020 08:41:22 +0200

Hi Rici,

Thanks a lot for your very complete and thorough analysis.

I'm somewhat afraid of long threads of long messages, so I'm going to split my 
answer in threads.  I can see this will not do justice to your message which is 
a whole, pieces binding themselves together, but this way I believe it will be 
easier for anybody to jump into the discussion and to share their viewpoint.

> Le 27 sept. 2020 à 20:46, Rici Lake <ricilake@gmail.com> a écrit :
> 
> Hi, Akim.
> 
> Sorry for not responding earlier. I'm in the middle of a move, and your 
> proposal needed more concentration than I was capable of giving.

It means a lot to me that you took the time to write your message in spite of 
that.  Thanks!


> With respect to the return mechanism, I feel like the use of a status return 
> and indirect out arguments is much more normal C style than a compound return 
> object.

I agree with that, yet I decided to something different, for several reasons.

1. I tend to think that this way of writing things is historical, because C 
compilers did poorly on this regard.  Today, compilers can easily break this 
and use registers (but it does require interprocedural optimization).  When 
interprocedural optimization is not possible, as far as I could see, even with 
-O0, compiler generate reasonable code, and entirely avoid memcpy.  Not exactly 
the same code as if you had passed a pointer yourself though.

In the present case, yyparse, I believe the possible cost of the calling 
convention to be entirely negligible compared to what the function will 
actually do.

2. The Go language has shown how valuable it is to return both a result, and an 
error value.  Rust has Result, C++ is most probably going to offer something 
similar (call it std::result or Boost.Outcome or std::expected), etc.  I also 
learned, over the years, to avoid as much as possible assignments, and 
uninitialized declarations.  In other words, going for a style some people like 
to call "functional".

Sure, new languages support multiple return values, which make this idiom 
lighter.  But here

3. I personally dislike having to deal with the members of YYSTYPE, because it 
lacks strong typing.  That's also why I'm a happy user of %define 
api.token.constructor.  So I wanted dedicated functions for each symbol, and 
then the alternative was (i) returning some struct that binds the result and 
status/errors, (ii) return the status/error and pass a pointer for the value.  
There's an argument that goes very much in favor of (i): it is future proof.  
Maintaining backward compatibility and yet adding new features is sometimes 
extremely delicate and can result in maintenance nightmares.  So here I'd 
rather return a struct, and be free to add new members in that struct in the 
future.

That does not rule out the possibility of passing a pointer to that struct, 
granted.  But here we're back to item 1.


> Also, the compound return value does not seem to me to simplify typical use 
> cases. It's reasonably rare that you will want to bundle a status value other 
> than success with a returned value, because in most cases the returned value 
> will only be valid if the status indicates success. (This doesn't apply to 
> yynerrs, of course; but see below.) So the caller will generally end up 
> deconstructing the return value anyway, in a pattern such as:
> 
>      SomeStructType parseCompoundResult = yyparse_expression(...);
>      if (parseCompoundResult.status != YYSUCCESS) { 
>          /* signal error and don't try to process value */
>      }
>      SomeValueType result = parseCompoundResult.value; /* In C++ this might 
> be a reference */

(I don't see what the C++ reference changes here.)

> That doesn't seem to me to offer any value over the more common:
> 
>      SomeValueType result;
>      if (yyparse_expression(&result) != YYSUCCESS) {
>         /* signal error and don't try to process value */
>      }

That's true, but there are more values to pass, and I would like to keep it 
future-proof for even more values.



I'll address your other points later.  Cheers!


reply via email to

[Prev in Thread] Current Thread [Next in Thread]