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

Fix treatment of WithWarnings in builtin method annotation processor #6070

Closed
1 of 2 tasks
Akirathan opened this issue Mar 24, 2023 · 2 comments
Closed
1 of 2 tasks
Assignees
Labels
--bug Type: bug -compiler p-low Low priority

Comments

@Akirathan
Copy link
Member

Akirathan commented Mar 24, 2023

org.enso.interpreter.dsl.MethodProcessor generates code for every builtin method in such a way that it removes all the warnings from all the arguments, except for self, and then reattaches these gathered warnings to the result. It does that by checking if (arg instanceof WithWarnings) which is incorrect, it should be replaced by WarningsLibrary.hasWarnings(arg). Moreover, the lack of warnings removal from the self argument is suspicious, since you have to take care of the warnings in the builtin method manually.

This might cause problems especially for vectors, for which, if one of the elements has a warning attached, it is propagated to the whole vector and sometimes duplicated across other elements.

Tasks

Preview Give feedback

Related:

@hubertp
Copy link
Collaborator

hubertp commented Apr 19, 2023

Regarding warnings on self. I think that part will have to stay in order to be able to do dispatch on Warning correctly. This was for example needed for #6176.

@jdunkerley jdunkerley removed this from the Beta Release milestone Jul 18, 2023
@jdunkerley jdunkerley assigned Akirathan and unassigned hubertp Sep 26, 2023
@jdunkerley jdunkerley assigned hubertp and unassigned Akirathan Oct 17, 2023
@jdunkerley jdunkerley moved this from 📤 Backlog to ❓New in Issues Board Oct 17, 2023
@jdunkerley jdunkerley moved this from ❓New to 📤 Backlog in Issues Board Oct 17, 2023
@JaroslavTulach JaroslavTulach assigned Akirathan and unassigned hubertp Jul 16, 2024
@Akirathan
Copy link
Member Author

Closing this as not an issue:

Stripping of warnings from the self argument is controlled by AcceptsWarning annotation.

Checking for warnings via instanceof is just fine. Passing messages via WarningsLibrary has unnecessary overhead.

@github-project-automation github-project-automation bot moved this from 📤 Backlog to 🟢 Accepted in Issues Board Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--bug Type: bug -compiler p-low Low priority
Projects
Archived in project
Development

No branches or pull requests

3 participants