Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use "RFC 2528: type-changing struct update syntax" when it is stabilized #463

Closed
sezna opened this issue Dec 16, 2021 · 4 comments
Closed

Comments

@sezna
Copy link
Contributor

sezna commented Dec 16, 2021

I recently refactored a lot of semantic_analysis to use a struct called TypeCheckArguments. This provides a consistent interface across our many type checking functions.

I wanted to be able to do something like this:

  TypedExpression::type_check(TypeCheckArguments {
      checkee: lhs.clone(),
      help_text: Default::default(),
      mode: Mode::NonAbi, opts,
      ..arguments
  }),

However, this implicit cast of arguments from TypeCheckArguments<T> to TypeCheckArguments<R> is experimental, and nightly only: rust-lang/rust#86555

Without that cast, we have to write this:

    TypedExpression::type_check(TypeCheckArguments {
        checkee: lhs.clone(),
        help_text: Default::default(),
        mode: Mode::NonAbi, opts,
        self_type,
        namespace,
        crate_namespace,
        return_type_annotation,
        build_config,
        dead_code_graph,
        dependency_graph,
    })

This could save us a ton of LOC -- we are currently manually plumbing parameters in lots of places. Once the above issue gets to rust stable, we should refactor these calls to use this syntax.

@sezna
Copy link
Contributor Author

sezna commented Dec 16, 2021

Along with this issue, we could switch all type checking functions to use t he TypeCheckArguments abstraction

@otrho
Copy link
Contributor

otrho commented Dec 17, 2021

Since most (half? 5/10?) of those args are re-used, would it work to have a type checker struct and just make all the type checks methods? I guess it would be spread across multiple source files, but still fit under the semantic actions type checking umbrella.

@sezna
Copy link
Contributor Author

sezna commented Dec 17, 2021

Some of them are truly never modified, like the dead code graph and stuff. That's a good idea!

@mohammadfawaz
Copy link
Contributor

#2004 switched all instances of TypeCheckArguments to use a context instead. I believe this can be closed now.

@mohammadfawaz mohammadfawaz closed this as not planned Won't fix, can't repro, duplicate, stale Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

3 participants