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

fix(bundler): Fix statement ordering issue #1264

Merged
merged 208 commits into from
Dec 27, 2020
Merged

Conversation

kdy1
Copy link
Member

@kdy1 kdy1 commented Dec 9, 2020

Note: I postponed denoland/deno#8597.


swc_bundler:

swc_ecma_transforms:

  • fixer: Handle assignments in the callee of new correctly.
  • fixer: Handle seqence expression in the callee of new correctly.

I investigated further.

8620 fails, and it's expected.

With lots of logging code and in debug build, all other tests pass.

But when I removed logging code and tested in release mode,

  • deno_8211_1

also fails.

Try: 8620

  for &dep in &deps {
            let cycles = self.graph.all_simple_paths(dep, idx);
            dbg!(&cycles);  
            if !cycles.is_empty() {
                self.emit_cycles(cycles);
                continue;
            }
            // We jump to another module.
            eprintln!("Jumping to dep: `{}`", dep);
            self.insert_inner(dep, false, dejavu, delayed, &[]);

            eprintln!("Done: one dep of `{}`: `{}`", idx, dep);
        }
        eprintln!("Done: deps of `{}`: `{:?}`", idx, deps);

hangs with

[bundler/src/bundler/modules/sort.rs:601] next_idx = 195
Checking `195`
[bundler/src/bundler/modules/sort.rs:519] &cycles = []
Jumping to dep: `194`
Checking `194`
Skipping `194`
Done: one dep of `195`: `194`

So... all_simple_paths hangs in certain condition.

Fix for petgraph

I'm trying to capture all graph operations in parsable form, and I'll use it to debug petgraph.

Tests

  • deno_7288_1:
  • deno_8188_01:
  • deno_8188_02:
  • deno_8188_03:
  • deno_8188_full:
  • deno_8189:
  • deno_8211_1:
  • deno_8246:
  • deno_8302:
  • deno_8314_1:
  • deno_8314_2:
  • deno_8399_1:
  • deno_8399_2:
  • deno_8481_1:
  • deno_8486_1:
  • deno_8530:
  • deno_8573:
  • deno_8620:
  • deno_8627:
  • exec_deno_exec__deno_8211__case2__entry_ts:
  • exec_deno_exec__deno_8545__entry_ts:
  • merging_order_01:
  • oak_6_3_1_application:
  • oak_6_3_1_example_server:
  • oak_6_3_1_example_sse_server:
  • oak_6_3_1_mod:
  • reexport_01:
  • std_0_74_0_http_server:

@kdy1 kdy1 added this to the v1.2.41 milestone Dec 9, 2020
@kdy1 kdy1 self-assigned this Dec 9, 2020
@ije ije mentioned this pull request Dec 12, 2020
15 tasks
@kdy1 kdy1 force-pushed the bundler branch 5 times, most recently from 60b747f to 2c99e53 Compare December 19, 2020 12:35
@kdy1
Copy link
Member Author

kdy1 commented Dec 20, 2020

I found lots of bugs of petgraph...
I just workarounded most of them to reduce time, but I found a bug which is hard to workaround.

Currently failing tests except 8620 fails due to bug of petgraph.

For memo, 8620 can be easily fixed by implementing more rules for collecting requirements.
So maybe I have to fix petgraph..

@ije

This comment has been minimized.

@kdy1 kdy1 modified the milestones: v1.2.41, v1.2.42 Dec 22, 2020
@kdy1
Copy link
Member Author

kdy1 commented Dec 23, 2020

I found that almost all tests pass if I wrap DiGraphMap from petgraph with custom struct.
I'm not sure what's going on...

@Soremwar Soremwar mentioned this pull request Dec 25, 2020
22 tasks
@kdy1 kdy1 changed the title Fix bundler: extra issues fix(bundler): Various bugs Dec 26, 2020
@kdy1 kdy1 changed the title fix(bundler): Various bugs fix(bundler): Statement ordering issue Dec 26, 2020
@kdy1 kdy1 changed the title fix(bundler): Statement ordering issue fix(bundler): Fix statement ordering issue Dec 26, 2020
@kdy1 kdy1 marked this pull request as ready for review December 27, 2020 10:02
@kdy1 kdy1 merged commit b66ee58 into swc-project:master Dec 27, 2020
@kdy1 kdy1 deleted the bundler branch December 27, 2020 10:02
@ije

This comment has been minimized.

@swc-project swc-project locked as resolved and limited conversation to collaborators Nov 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants