Rubberduck: Unify expression evaluation back-ends for Unreachable case and the preprocessor

Created on 26 Apr 2018  路  5Comments  路  Source: rubberduck-vba/Rubberduck

We currently have two separate back-ends trying to evaluate literals and expressions in VBA code, one used by the preprocessor and one used by the unreachable case inspection. Both do their job reasonably well, but both also have their shortcomings. (See #3961, #3962, #3963, #3966, #3967.)

My suggestion in this issue is to decide on one of the approaches and then to move that into its own namespace in Rubberduck.Parsing. Then this one approach should be fixed to agree with what the VBA specification dictates and adapted to be suitable to serve both clients, the preprocessor and the unreachable case inspection.

This would allow to maintain only one back-end and not having to fix both. Moreover, this one back-end could be used by other features to determine the type and, if possible, the value of expressions as well. One application coming to mind is the determination of the type of constants without As Type statement.

discussion technical-debt

Most helpful comment

Although I appreciate the work that has gone into the back-end for the Unreachable Case inspection, I would vote to base the unified approach primarily on the one used by the preprocessor. My main reasons for this preference are the following:

  1. The preprocesor actually uses different types implementing a common interface to cover the different VBA types instead of relying on string representations.
  2. The preprocessor actually already covers more literal formats and understands that logical operators are bit-wise operators.

Regarding the the comment about its own namespace, that is actually what I have proposed.

All 5 comments

I completely agree. However, IMO, it's large enough to warrant its own namespace, perhaps Rubberduck.Parsing.Expression or something like that. The mistake was that it was tied to predecessor and now there's another tied to the USCI when in fact it ought to be more general and thus be useful for other scenarios like one you mention.

Although I appreciate the work that has gone into the back-end for the Unreachable Case inspection, I would vote to base the unified approach primarily on the one used by the preprocessor. My main reasons for this preference are the following:

  1. The preprocesor actually uses different types implementing a common interface to cover the different VBA types instead of relying on string representations.
  2. The preprocessor actually already covers more literal formats and understands that logical operators are bit-wise operators.

Regarding the the comment about its own namespace, that is actually what I have proposed.

Regarding the test cases for the UCI backend, is there any merits in converting them at least for those that isn't currently covered by the preprocessor backend? It's a good use of data-driven test cases since we must handle so many different valid formats.

I think there is more than just the tests we can reuse from the UCI back-end. E.g. we could use the logic from the ParseTreeValue to provide IValue objects from string representations of literals and combinations of a string literal and a declared type, respectively.

Just to record for posterity, the new unified expression engine should ideally make use of the functions imported from the oleaut32 DLL. The functions are described here. The engine should invoke those functions to get the same behavior as we observe with VBA for parsing, conversion and casting of the expressions and its types.

Was this page helpful?
0 / 5 - 0 ratings