Skip to content

Ignore self-cycles on bsb_helper to suppress false positives #5393

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

Merged
merged 6 commits into from
Jun 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified darwin/ninja.exe
Binary file not shown.
Binary file modified darwinarm64/ninja.exe
Binary file not shown.
13 changes: 10 additions & 3 deletions jscomp/bsb_helper/bsb_helper_depfile_gen.ml
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ let oc_cmi buf namespace source =

When ns is turned on, `B` is interprted as `Ns-B` which is a cyclic dependency,
it can be errored out earlier

#5368: It turns out there are many false positives on detecting self-cycles (see: `jscomp/build_tests/zerocycle`)
To properly solve this, we would need to `jscomp/ml/depend.ml` because
cmi and cmj is broken in the first place (same problem as in ocaml/ocaml#4618).
So we will just ignore the self-cycles. Even if there is indeed a self-cycle, it should fail to compile anyway.
*)
let oc_deps (ast_file : string) (is_dev : bool) (db : Bsb_db_decode.t)
(namespace : string option) (buf : Ext_buffer.t) (kind : [ `impl | `intf ])
Expand Down Expand Up @@ -133,9 +138,11 @@ let oc_deps (ast_file : string) (is_dev : bool) (db : Bsb_db_decode.t)
while !offset < size do
let next_tab = String.index_from s !offset magic_sep_char in
let dependent_module = String.sub s !offset (next_tab - !offset) in
if dependent_module = cur_module_name then (
prerr_endline ("FAILED: " ^ cur_module_name ^ " has a self cycle");
exit 2);
if dependent_module = cur_module_name then
(*prerr_endline ("FAILED: " ^ cur_module_name ^ " has a self cycle");
exit 2*)
(* #5368 ignore self dependencies *) ()
else
(match Bsb_db_decode.find db dependent_module is_dev with
| None -> ()
| Some { dir_name; case } ->
Expand Down
10 changes: 8 additions & 2 deletions jscomp/build_tests/cycle/input.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
//@ts-check
var cp = require("child_process");
var assert = require("assert");
const cp = require("child_process");
const assert = require("assert");
const fs = require('fs')
const path = require('path')

var output = cp.spawnSync(`rescript`, { encoding: "utf8", shell: true });

assert(/dependency cycle/.test(output.stdout));

var compilerLogFile = path.join(__dirname, 'lib', 'bs', '.compiler.log');
var compilerLog = fs.readFileSync(compilerLogFile, 'utf8');
assert(/dependency cycle/.test(compilerLog));
26 changes: 26 additions & 0 deletions jscomp/build_tests/cycle1/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
*.exe
*.obj
*.out
*.compile
*.native
*.byte
*.cmo
*.annot
*.cmi
*.cmx
*.cmt
*.cmti
*.cma
*.a
*.cmxa
*.obj
*~
*.annot
*.cmj
*.bak
lib/bs
*.mlast
*.mliast
.vscode
.merlin
.bsb.lock
16 changes: 16 additions & 0 deletions jscomp/build_tests/cycle1/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@


# Build
```
npm run build
```

# Watch

```
npm run watch
```


# Editor
If you use `vscode`, Press `Windows + Shift + B` it will build automatically
14 changes: 14 additions & 0 deletions jscomp/build_tests/cycle1/bsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"name": "cycle1",
"namespace": true,
"version": "0.1.0",
"sources": {
"dir" : "src",
"subdirs" : true
},
"package-specs": {
"module": "es6",
"in-source": true
},
"suffix": ".bs.js"
}
13 changes: 13 additions & 0 deletions jscomp/build_tests/cycle1/input.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//@ts-check
const cp = require("child_process");
const assert = require("assert");
const fs = require('fs')
const path = require('path')

var output = cp.spawnSync(`rescript`, { encoding: "utf8", shell: true });

assert(/is dangling/.test(output.stdout));

var compilerLogFile = path.join(__dirname, 'lib', 'bs', '.compiler.log');
var compilerLog = fs.readFileSync(compilerLogFile, 'utf8');
assert(/is dangling/.test(compilerLog));
17 changes: 17 additions & 0 deletions jscomp/build_tests/cycle1/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"name": "cycle1",
"version": "0.1.0",
"scripts": {
"clean": "rescript clean",
"build": "rescript build",
"watch": "rescript build -w"
},
"keywords": [
"ReScript"
],
"author": "",
"license": "MIT",
"devDependencies": {
"rescript": "^10.0.0"
}
}
3 changes: 3 additions & 0 deletions jscomp/build_tests/cycle1/src/A.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
include A

let x = 42
1 change: 1 addition & 0 deletions jscomp/build_tests/cycle1/src/A.resi
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
let x : int
8 changes: 8 additions & 0 deletions jscomp/build_tests/pinned/input.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ function checkSpawnOut(out) {
assert.fail(out.stderr + "\n" + out.stdout);
}
}

// Clean beforehand to force its dependency to be rebuilt
var out = cp.spawnSync(`npx rescript clean`, {
encoding: "utf-8",
shell: true,
});
checkSpawnOut(out);

var out = cp.spawnSync(`npx rescript build`, {
encoding: "utf-8",
shell: true,
Expand Down
3 changes: 2 additions & 1 deletion jscomp/build_tests/zerocycle/input.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
var p = require("child_process");
var assert = require("assert");
p.spawnSync(`rescript`, { encoding: "utf8", cwd: __dirname });
var out = p.spawnSync(`rescript`, { encoding: "utf8", cwd: __dirname });
assert(out.status == 0)
14 changes: 14 additions & 0 deletions jscomp/build_tests/zerocycle/src/bar.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// one-file false positive: https://github.com/rescript-lang/rescript-compiler/issues/5368

module Nested = {
module Bar = {
type t = private int
}
}

open Nested

module Bar = {
open Bar
let t : t = Obj.magic(42)
}
1 change: 1 addition & 0 deletions jscomp/build_tests/zerocycle/src/demo2.res
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module Foo2 = {}
2 changes: 2 additions & 0 deletions jscomp/build_tests/zerocycle/src/foo2.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
open Demo2
include Foo2
Binary file modified linux/ninja.exe
Binary file not shown.
2 changes: 1 addition & 1 deletion ninja
Binary file modified win32/ninja.exe
Binary file not shown.