Skip to content

Conversation

@GearsDatapacks
Copy link
Member

@GearsDatapacks GearsDatapacks commented Oct 21, 2024

Closes #1488
This PR implements support for binary operations in constants.

A few notes:

  • It's still missing tests. This probably needs a lot of tests.

  • This is implemented differently on Erlang and Javascript, due to the different languages being somewhat different.
    For example, this code:

    const minutes_per_hour = 60
    const hours_per_day = 24
    const minutes_per_day = minutes_per_hour * hours_per_day
    
    pub fn main() {
      minutes_per_day
    }

    Compiles to this Javascript:

    const minutes_per_hour = 60
    const hours_per_day = 24
    const minutes_per_day = minutes_per_hour * hours_per_day
    
    export function main() {
      return minutes_per_day;
    }

    And this erlang:

    main() ->
        1440.

    Although this constant folding process could be done in the analysis stage instead and be applied to javascript as well.

  • I've been having some trouble trying to get the formatter to play nicely with this; I still don't really understand how it works. If someone who is more familiar with it could point me in the right direction, that would be amazing.

  • I'm not super happy with the parser code for constant binary operations: I had to copy a lot of the expression-parsing code. It might be good to merge this code somehow.

@giacomocavalieri
Copy link
Member

One more thing that's missing is the version tracking. We want to track that if you're using this your code is only compatible with 1.6 or later (given this is merged before the 1.6 release), you can have take inspiration from the way it works with arithmetic in guards

self.track_feature_usage(FeatureKind::ArithmeticInGuards, location);

@lpil
Copy link
Member

lpil commented Oct 26, 2024

I wonder if there's a way to make sure we always remember to add that. I forgot!

@GearsDatapacks
Copy link
Member Author

Closing this for now as I ran into some issues with maths operations on floats. I'll reopen later if I get something working

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow basic math/... expressions in the const syntax

3 participants