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

convert from adhoc exception #5237

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

convert from adhoc exception #5237

wants to merge 5 commits into from

Conversation

coke
Copy link
Collaborator

@coke coke commented Mar 25, 2023

The sample here copies a chunk of the lang_setup method from the main grammar.

"use v4" prints out the error message and dies.

"use v5" couldn't create the exception. I've tried to setup the core.c resolver here but am just cargo culting, this also dies.

Add sample showing that throwing the exception is the problem
'use 4' is processed correctly, but use 5 dies on the exception
so we can safely throw the exception
@vrurg
Copy link
Member

vrurg commented Mar 26, 2023

"use v5" couldn't create the exception. I've tried to setup the core.c resolver here but am just cargo culting, this also dies.

Same problem as I explained on IRC, perhaps. CORE setting isn't loaded yet. You'd need to load it first. Guess, just CORE.c is OK for the purpose.

@vrurg
Copy link
Member

vrurg commented Mar 26, 2023

I just've noticed that @niner has similar approach to mine: when an error exception is about to be thrown matters of inefficiency are not that important.

@coke
Copy link
Collaborator Author

coke commented Mar 26, 2023

"use v5" couldn't create the exception. I've tried to setup the core.c resolver here but am just cargo culting, this also dies.

Same problem as I explained on IRC, perhaps. CORE setting isn't loaded yet. You'd need to load it first. Guess, just CORE.c is OK for the purpose.

Ok, tried harder to load the v6.c language before throwing the exception.

@vrurg
Copy link
Member

vrurg commented Mar 26, 2023

Ok, tried harder to load the v6.c language before throwing the exception.

If it works this way, I'd like to see a less hacky implementation.

@coke
Copy link
Collaborator Author

coke commented Mar 26, 2023

If it works this way, I'd like to see a less hacky implementation.

Sorry, wasn't clear: it's still failing.

@coke
Copy link
Collaborator Author

coke commented Mar 26, 2023

ok, current version now does something reasonable with -e 'use v6' and -e 'use v5' but fails when running files.

@lizmat
Copy link
Contributor

lizmat commented Apr 4, 2023

Hmm... I'm wondering: why don't we load v6.c (or whatever is the basic supported version) unconditionally? And have any use v6x statement figure out that 1. there have not been any other statements, and 2. load any subsequent setting if the version indicated is higher?

@vrurg
Copy link
Member

vrurg commented Apr 4, 2023

why don't we load v6.c (or whatever is the basic supported version) unconditionally?

Considering that loaded core settings are cached anyway, it makes even more sense.

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

3 participants