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

Unclear error if a cycle dependency between namespaces #444

Open
davidyuk opened this issue Dec 16, 2022 · 2 comments
Open

Unclear error if a cycle dependency between namespaces #444

davidyuk opened this issue Dec 16, 2022 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@davidyuk
Copy link
Member

Reproduction

├── Includes.aes
└── lib
    ├── Library.aes
    └── Sublibrary.aes

Includes.aes

include "./lib/Library.aes"

contract Includes =
    entrypoint test(x: int): int = Library.sum(x, 4)

Library.aes

include "./lib/Sublibrary.aes"

namespace Library =
  type number = int
  function sum(x: int, y: int): int = Sublibrary.sum(x, y)

Sublibrary.aes

include "./lib/Library.aes"

namespace Sublibrary =
  function sum(x: int, y: int): int = x + y
  function return42(): Library.number = 42

CLI prints an unclear error

$ aesophia_cli ./Includes.aes
Type error in 'lib/Sublibrary.aes' at line 5, col 24:
Unbound type Library.number.

Type error in 'lib/Sublibrary.aes' at line 5, col 41:
Cannot unify `int` and `Library.number`
when checking the type of the expression `42 : int` against the expected type `Library.number`
$ aesophia_cli --version     
Sophia compiler version 7.0.1

it works if I include number from another namespace (without cycle deps). Sophia should disallow cycle dependencies explicitly or the above reproduction should work.

@ghallak ghallak transferred this issue from aeternity/aesophia_cli Mar 22, 2023
@ghallak
Copy link
Contributor

ghallak commented Mar 22, 2023

First, this has nothing to do with includes. You can put the namespaces in the same file and it will still not work.

The error is produced because a namespace can only recognize what was written before it, so if you want to have a base namespace and a dependent namespace, you should have them in order.

namespace Base =
    ...

namespace Dependent =
    ...

Is there a specific situation where you need 2 namespaces to depend on each other, and you're not better off refactoring the code?

I would say that it's better to keep it this way in order to prevent messy designs, and the error messages are clear enough once you know that a namespace can only recognize what's written before it.

@radrow Do you have any opinion on this?

@radrow
Copy link
Member

radrow commented Mar 22, 2023

I think it would be much more user friendly if we explicitly point out that dependency cycles are not supported. For example functions within one namespace do support mutual recursion, so I can imagine this being confusing.

@ghallak ghallak self-assigned this Mar 22, 2023
@ghallak ghallak added the enhancement New feature or request label Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants