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

Stricter checks for typedesc #8657

Closed
wants to merge 3 commits into from
Closed

Stricter checks for typedesc #8657

wants to merge 3 commits into from

Conversation

metagn
Copy link
Collaborator

@metagn metagn commented Aug 16, 2018

Fixes #8610, fixes #8654

@metagn metagn changed the title Add typedesc check for type annotation in var statement Stricter checks for typedesc Aug 16, 2018
@mratsim
Copy link
Collaborator

mratsim commented Aug 16, 2018

Can't we just put an error message and that says:

const Foo=int # Error: use "type Foo = int" instead

Special casing const for typedesc is a bad idea.

In the VM, type and NimNode should cover all use cases already.

"""

const a: typedesc = typedesc[int]
echo a is typedesc[int]
Copy link
Member

Choose a reason for hiding this comment

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

if you're gonna error on line 6, why have line 7? (+ same in other files)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sanity check? I copied existing tests

Copy link
Member

Choose a reason for hiding this comment

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

why is that a sanity check, if it won't even get compiled?
seems like you could put whatever there instead...
maybe existing tests could be updated too

@metagn
Copy link
Collaborator Author

metagn commented Aug 29, 2018

I'm not eligible to fix the test failures, so I'm gonna close the PR and not abandon it and make everyone wait forever, if anyone wants to take over they can start another PR

@metagn metagn closed this Aug 29, 2018
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.

Compiler crash with typedesc as variable const Foo=int compiles; is that legal? what does it mean?
3 participants