Is this 'good' rust style for 'call chaining'

1

middle of a compiler I am writing

        let index = self.do_expression(0)?;
        let index = self.make_rvalue(&index)?;
        let index = self.convert_to(&index, &SymbolType::Int64, true)?;

Ideally (in other languages), I would chain these

let index = self.do_expression(0).make_rvalue().convert_to(SymbolType::Int64,true);

But rust doesnt lend itself to that (the return value from the first call would have to have a mutable reference to self)

At one point I did

        let exp_index = self.do_expression(0)?;
        let index_rv = self.make_rvalue(&exp_index)?;
        let index = self.convert_to(&index_rv, &SymbolType::Int64, true)?;

Becuase as a programmer of other languages I shudder when I see multiple variables with the same name . But rust doesnt care. In fact I like the implicit chain of using the same name over and over

2

I think the sequential statements are fine.

But if you really want that, it's probably still possible with enough boilerplate.

1 Like
3

I agree. Both shadowing and using different variables is fine.
Though, my personal preferred style would be

let index = self.convert_to(
    &self.make_rvalue(&self.do_expression(0)?)?,
    &SymbolType::Int64,
    true,
)?;

And if the nesting of method calls goes too far, I usually rethink my design.

4

It is also possible to write methods that take self and return self but I don't know if the compiler is smart enough to avoid all the copies that would be necessary in a naive implementation.

5

Maybe the root problem is, that index is a very weak name itself and practically somewhat useless.

And:

Is only slightly better, because the naming scheme is inconsistent.

I'm just guessing, I don't know what your methods are returning, but maybe something like this fits better:

let expr = self.do_expression(0)?;
let rvalue = self.make_rvalue(&index)?;
let symbol = self.convert_to(&index, &SymbolType::Int64, true)?;

But even this is weak. I'm just guessing again, but I'm pretty sure, there is some more information available for the reader of the code. Maybe this line work on some right-hand-side stuff? So, in this case, the variables can be named:

let rhs_expr = self.do_expression(0)?;
let rhs_rvalue = self.make_rvalue(&index)?;
let rhs_symbol = self.convert_to(&index, &SymbolType::Int64, true)?;

When using shadowing, maybe it's beneficially to the reader of the code, to add some information, why all this is written. Often type annotations make things more clear, in this case just guessing again:

let rhs: Expression = self.do_expression(0)?;
let rhs: Rvalue = self.make_rvalue(&index)?;
let rhs: Symbol = self.convert_to(&index, &SymbolType::Int64, true)?;

So the reader of the code can reason about the steps and the outcome.

6

thanks,

my compiler is compiling this

array[<index>]

so it really is a good name.

FYI, the three stages are

  • parse the expression inside [], do_expression doesnt know if its going to be used as an lvalue or rvlaue , matters for something like [*pidx] because *pidx has to be treated entirely differently as an lvalue as opposed to rvalue. So it returns a 'maybe l or r' value
  • commit it to being an rvalue
  • promote it to the int64 type, my canonical type for array indexing