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

Nonref object inheritance hints and converts to base in varargs a… #74

Merged
merged 1 commit into from
Nov 29, 2021

Conversation

beef331
Copy link
Contributor

@beef331 beef331 commented Nov 23, 2021

…nd on assignment

This should make it so non ref objects can be used as one expected. varargs[Base] can accept [Base, Derived, Base, Derived2] and so forth. It also makes it so any object conversion up or down(Though down conversion is explicit so no clue why it checks) emits a hint to inform about loss of possible information. var a: Base = Derived(field2: 300) emits 'Derived(field2: 300)' implicitly converted to 'Base' [/home/jason/nimskull/bin/abuse.nim(26, 22)] [ImplicitObjConv]

@saem
Copy link
Collaborator

saem commented Nov 23, 2021

Waiting for #59 in order check against the spec semantics.

@saem
Copy link
Collaborator

saem commented Nov 28, 2021

@beef331 mind rebasing this, I'm with you on the hint. They're all value types.

@haxscramper
Copy link
Collaborator

haxscramper commented Nov 28, 2021

Ideally this should also include spec-like test for #59 (comment) that checks that f(a, b, c) works and does implicit conversion (for different usage scenarios), and f([a, b, c]) does not work, because we construct an array literal, and those should not do the implicit type conversion on construction.

Latter part can be revised later (check array literal differently and/or perfrom implicit conversions), but for now this should be made into compiler error (if it isn't already).

Same applies to the f(@[a, b, c]), although technically this does not need to be tested, since @[] is already a function application on the array literal, but seq construction is magic, so better check this one as well.

…nd on assignment

array handling and more elaborate tests

Better error message for implict object converstion
@@ -0,0 +1,53 @@
discard """
Copy link
Collaborator

Choose a reason for hiding this comment

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

@beef331 is the intention with these tests to fold into the spec?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It probably should be moved to the spec I guess since it's for regression assurance.

Copy link
Collaborator

@saem saem Nov 29, 2021

Choose a reason for hiding this comment

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

OK, if you know how to do it now then go for it, otherwise ask @haxscramper when he's about and do it later?

(we can merge this in the meantime)

@saem
Copy link
Collaborator

saem commented Nov 29, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Nov 29, 2021

Build succeeded:

@bors bors bot merged commit 05b2050 into nim-works:devel Nov 29, 2021
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.

3 participants