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

Import Cycle Detection throws unhandled error when graph is not fully a cycle #1535

Open
leeyi45 opened this issue Feb 13, 2024 · 3 comments
Labels
Bug Something isn't working minor less important than important but more than nice-to-have

Comments

@leeyi45
Copy link
Contributor

leeyi45 commented Feb 13, 2024

Consider the following set of programs

// /a.js
import { b } from './b.js';
b;

// /b.js
import { a } from './a.js';
a;

// /program.js
import { a } from './a.js';
a;

Running /program.js causes the directed graph's cycle finder to throws an unhandled error "Node '/playground/program.js' has no incoming edges. This should never happen."

The tests in js-slang account for when all files together form some cycle. In this case however, the cycle only exists between /a.js and /b.js, so the entrypoint program /program.js is not part of that cycle.

Link to example program

@martin-henz
Copy link
Member

Does the program to be run need to be part of the cycle? In my view, importing a module that cyclically depends on some other module is ill-defined.

@leeyi45
Copy link
Contributor Author

leeyi45 commented Feb 13, 2024

Does the program to be run need to be part of the cycle? In my view, importing a module that cyclically depends on some other module is ill-defined.

I don't have a particular opinion on this, just that this specific condition needs to be handled gracefully

@leeyi45 leeyi45 changed the title Import Cycle Detection throws errors when graph is not fully a cycle Import Cycle Detection throws unhandled error when graph is not fully a cycle Feb 14, 2024
@martin-henz
Copy link
Member

I see. We should catch this error and display a more reasonable error message. Something like:
"program /playground/program.js has cyclic dependencies in its import declarations"

@martin-henz martin-henz added Bug Something isn't working minor less important than important but more than nice-to-have labels Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working minor less important than important but more than nice-to-have
Projects
None yet
Development

No branches or pull requests

2 participants