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

Feature/16 using definition #116

Merged
merged 7 commits into from
Dec 24, 2017
Merged

Feature/16 using definition #116

merged 7 commits into from
Dec 24, 2017

Conversation

emmanuel099
Copy link
Member

@emmanuel099 emmanuel099 commented Dec 23, 2017

Namespace aliasing doesn't work atm, e.g.:

using MyColor = Color;
... MyColor::Red ...

This requires some changes in the namespace resolving procedure and will be fixed in another PR (next release) to avoid breakages.

Using cycles and not top-down ordered definitions are not handled at the moment.
E.g. the following will fail because T1 depends on T2:

using T1 = T2;
using T2 = ...;

This will be fixed by performing a top-sort of all "using definitions" in another PR (next release).

Refs sealangdotorg/sea#16

See casm-lang/libcasm-tc#21 for tests

… to LHS

Using cycles and not top-down ordered definitions are not handled at the moment.
E.g. the following will fail because T1 depends on T2:
~~~
using T1 = T2;
using T2 = ...;
~~~

This will be fixed by performing a top-sort of all "using definitions".
Copy link
Member

@ppaulweber ppaulweber left a comment

Choose a reason for hiding this comment

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

overall nice PR and I'll approve it ASAP when the requested changes from casm-lang/libcasm-tc#21 are done!

Furthermore, in your PR description you are referencing a Refs #16. Does this reference an issue from this libcasm-fe repo or do you mean the feature issue sealangdotorg/sea#16 ?

@@ -502,6 +507,14 @@ Enumerators
;


UsingDefinition
: USING Identifier EQUAL Type
Copy link
Member

Choose a reason for hiding this comment

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

finally 👍 🥇

@emmanuel099
Copy link
Member Author

Yes you are right, fixed!

Copy link
Member

@ppaulweber ppaulweber left a comment

Choose a reason for hiding this comment

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

really nice change

@@ -173,7 +182,8 @@ void TypeCheckVisitor::visit( BasicType& node )
{
m_log.error(
{ node.sourceLocation() },
"cannot use " + symbol->description() + " '" + name + "' as type" );
"cannot use " + symbol->description() + " '" + name + "' as type",
Code::TypeAnnotationInvalidBasicTypeName );
Copy link
Member

Choose a reason for hiding this comment

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

well done 👍

@ppaulweber ppaulweber merged commit 587887d into master Dec 24, 2017
@ppaulweber ppaulweber deleted the feature/16_using_definition branch December 24, 2017 15:49
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.

2 participants