search

stan-dev / stanc3

The Stan transpiler (from Stan to C++ and beyond).
BSD 3-Clause "New" or "Revised" License
140 stars 44 forks source link

Remove unused MIR specializations #1204

Closed WardBrian closed 2 years ago

WardBrian commented 2 years ago

We've had two MIR specializations (Labelled and NoMeta) which have been there since the refactor in #346 but are never instantiated. Removing them requires no additional code changes to the compiler, and it is unclear what they were intended for.

I believe removing these also clears up any circular dependency issues which would arise if we wanted to move things like SizedType and UnsizedType from Middle to Common in the future.

Submission Checklist

Release notes

Simplified internal representations in the compiler.

Copyright and Licensing

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)

rybern commented 2 years ago

FYI, IIRC Labeled was meant as a possible replacement for the MIR data structure in analysis/optimization, but the switch was never made

On Thu, May 26, 2022, 11:08 AM Niko Huurre @.***> wrote:

@.**** approved this pull request.

— Reply to this email directly, view it on GitHub https://github.com/stan-dev/stanc3/pull/1204#pullrequestreview-986370677, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGAMRLEKE3LQPAGRGQZIDLVL6HUPANCNFSM5XBLUZPA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

WardBrian commented 2 years ago

That would explain why the Non_recursive specialization is labeled as "a temporary replacement". Unless someone is dedicated to finishing that change, I don't think it will end up happening (much like how the AST never got the same treatment as the MIR).

Perhaps it is just different sensibilities, but my instinct is to go directly in the opposite direction and define untyped_ast and typed_ast patterns which are distinct from each other. The current structure means we have a fair number of illegal states which the type system is unaware of, such as Promotion AST nodes being in the untyped AST, which logically can never happen.