-
Notifications
You must be signed in to change notification settings - Fork 612
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
Bratseth/type inference cleanup #33179
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit worried about the unit tests that you have removed, it looks like some of the things they test will stop working now, and it's hard to be certain that it won't be a problem in the larger setting.
I need to move the testing to a level where entire statements are tested since the type resolution is at the statement level. I did that with for_each here but maybe I missed something. What tests did you have in mind? |
right, so ForEach now requires type resolving up-front, but it still tries to do it runtime also? Maybe we can simplify some of this later - the FieldValueConverter class is only used by ForEach. It looks like the tests you added should cover actual usage, so I think we can go ahead with this. |
Ok, thanks! Yes the simplification isn't complete with this. |
No description provided.