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

feat: resolve java types against namespace imports in try-catch #7902

Merged
merged 14 commits into from
Jun 24, 2024

Conversation

jaschdoc
Copy link
Member

@jaschdoc jaschdoc commented Jun 17, 2024

Related to #7901
Related to #7872
Related to #7897

@jaschdoc jaschdoc marked this pull request as draft June 17, 2024 14:10
@jaschdoc
Copy link
Member Author

@magnus-madsen quick review? Still need to add documentation

@magnus-madsen
Copy link
Member

@jaschdoc The future plan is that all Java types must be imported. So its totally fine if you just do a look up in the environment like in InvokeStaticMethod2.

@jaschdoc
Copy link
Member Author

Hmm instanceof tests are still parser error... not sure how to expect a non ## symbol. @herluf-ba do you know what's going on? I can explain more on gitter or here if necessary?

@magnus-madsen
Copy link
Member

Hmm instanceof tests are still parser error... not sure how to expect a non ## symbol. @herluf-ba do you know what's going on? I can explain more on gitter or here if necessary?

Can we do the exceptions first and then later the instanceof?

@jaschdoc jaschdoc marked this pull request as ready for review June 23, 2024 15:13
@jaschdoc
Copy link
Member Author

I will update tests and std lib in smaller PRs related to #7872

@jaschdoc jaschdoc changed the title feat: resolve java types against namespace imports feat: resolve java types against namespace imports in try-catch Jun 23, 2024
* Returns the class reflection object for the given `className`.
*/
private def lookupJvmClass2(className: String, env0: ListMap[String, Resolution], loc: SourceLocation)(implicit flix: Flix): Result[Class[_], ResolutionError with Recoverable] = {
lookupJvmClass(className, loc) match {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, in the future this extra lookup should go away... But. that of course requires refactoring more code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point being that all Java classes and interfaces must be imported.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but wanted to do this in a soft way so I can progress on the Java import issue before breaking everything :)

@magnus-madsen magnus-madsen enabled auto-merge (squash) June 24, 2024 08:14
@magnus-madsen magnus-madsen merged commit 27b1135 into flix:master Jun 24, 2024
12 checks passed
@jaschdoc jaschdoc deleted the resolver-trycatch-imports branch June 24, 2024 08:56
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.

None yet

2 participants