Skip to content
This repository has been archived by the owner on Mar 8, 2020. It is now read-only.

new fixtures for semantic UAST #11

Closed
wants to merge 3 commits into from
Closed

new fixtures for semantic UAST #11

wants to merge 3 commits into from

Conversation

dennwc
Copy link
Member

@dennwc dennwc commented Feb 28, 2018

Signed-off-by: Denys Smirnov denys@sourced.tech

Signed-off-by: Denys Smirnov <denys@sourced.tech>
@dennwc dennwc self-assigned this Feb 28, 2018
Signed-off-by: Denys Smirnov <denys@sourced.tech>
@dennwc dennwc requested a review from juanjux March 2, 2018 14:47
package fixtures

type Foo struct {
Bar
Copy link
Contributor

@juanjux juanjux Mar 2, 2018

Choose a reason for hiding this comment

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

Having Bar defined above would make the test more complete.

package fixtures

type Foo interface {
Bar
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

@@ -0,0 +1,7 @@
package fixtures

type Foo int
Copy link
Contributor

Choose a reason for hiding this comment

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

There is some inconsistency because in some test filenames the types are called u2_class, but here are called u2_type, is there a rationale for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I consider "classes" as something that can have fields, while "types" can only have methods attached. Probably it's wrong.

Signed-off-by: Denys Smirnov <denys@sourced.tech>
@juanjux
Copy link
Contributor

juanjux commented Mar 2, 2018

Not wrong, just an opinable matter, the important thing would be to use the same criteria for all languages with structural subtyping.

@juanjux
Copy link
Contributor

juanjux commented Mar 15, 2018

Closing this one, it's superseded by #17 branched from this one.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants