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

DCE cleanup 2024 #11485

Merged
merged 4 commits into from
Jan 12, 2024
Merged

DCE cleanup 2024 #11485

merged 4 commits into from
Jan 12, 2024

Conversation

Simn
Copy link
Member

@Simn Simn commented Jan 11, 2024

  1. Add CfUsed and CfMaybeUsed to get rid of @:used and @:maybeUsed for fields.
  2. Add CUsed to get rid of @:used for classes. The meta still exists for enums and abstracts because those don't have flags.
  3. Remove m_if_feature, instead have DCE check @:ifFeature metadata itself.
  4. Remove the removal of (now) CfMaybeUsed at the end of DCE. This is set by the typer, so removing it in post-processing is wrong.

The unit.js diff against development shows the removal of unit_issues__$Issue2785_Child, which makes sense to me because it's unused and the only @:keep exists on its parent class. The dump diff of the JVM tests shows 3000 removed @:used metadata and 45 removed lines. 13 of those are from the Issue2785 thing, and the other 32 are... I don't know where.

Edit: I think my diff tool was lying to me, if I avoid printing @:used there are 0 differences.

also copy class @:ifFeature to field @:ifFeature
Can't get rid of Meta.Used entirely yet because enums and abstracts don't have flags
instead play them in DCE because otherwise we won't deal with __init__ correctly
@Simn
Copy link
Member Author

Simn commented Jan 12, 2024

The JVM tests on windows 64 fail consistently, so there's a chance that it's related to the changes here somehow. I can't make sense of the CI log though:

test jvm
Changing directory to D:/a/haxe/haxe/tests/unit
Command: haxelib [git,utest,https://github.com/haxe-utest/utest,424a7[182](https://github.com/HaxeFoundation/haxe/actions/runs/7494491280/job/20414033497?pr=11485#step:13:183)a93057730fada54b9d27d90b3cb7065c,--always]
You already have utest version git installed.
Installing utest from https://github.com/haxe-utest/utest branch: 424a7182a93057730fada54b9d27d90b3cb7065c
  Current version is now git
Done
Command exited with 0 in 1s: haxelib [git,utest,https://github.com/haxe-utest/utest,424a7182a93057730fada54b9d27d90b3cb7065c,--always]
Command: haxelib [git,hxjava,https://github.com/HaxeFoundation/hxjava]
You already have hxjava version git installed.
Updating hxjava version git...
Command exited with 1 in 1s: haxelib [git,hxjava,https://github.com/HaxeFoundation/hxjava]
Command will be re-run...
Command: haxelib [git,hxjava,https://github.com/HaxeFoundation/hxjava]
You already have hxjava version git installed.
Updating hxjava version git...
Command exited with 1 in 1s: haxelib [git,hxjava,https://github.com/HaxeFoundation/hxjava]
Command will be re-run...
Command: haxelib [git,hxjava,https://github.com/HaxeFoundation/hxjava]
You already have hxjava version git installed.
Updating hxjava version git...
Command exited with 1 in 1s: haxelib [git,hxjava,https://github.com/HaxeFoundation/hxjava]
The system cannot find the path specified.
javac 1.8.0_392
Cloning into 'java-lib-tests'...
The system cannot find the file specified.
Error: std@process_exit
Error: std@process_exit
An exception occurred in a neko Thread :
An exception occurred in a neko Thread :
std@lock_release
std@lock_release
Error: std@process_exit
test jvm failed
Error: Process completed with exit code 1.

it won't help
@Simn
Copy link
Member Author

Simn commented Jan 12, 2024

Uhm okay, random commit fixed this... no idea what that was.

@Simn Simn merged commit 59c8625 into development Jan 12, 2024
122 checks passed
@Simn Simn deleted the dce_cleanup_2024 branch January 12, 2024 07:26
@andyli
Copy link
Member

andyli commented Jan 12, 2024

api doc gen was broken since this.

https://github.com/HaxeFoundation/api.haxe.org/actions/runs/7499155735/job/20415432376

              +gen.n *failed* | --> RUN haxe gen.hxml
              +gen.n *failed* | Error: Compiler failure: haxe.ValueException is missing field "value"
              +gen.n *failed* | Please submit an issue at https://github.com/HaxeFoundation/haxe/issues/new
              +gen.n *failed* | Attach the following information:
              +gen.n *failed* | Haxe: 5.0.0-alpha.1+ba272a6; OS type: unix;
              +gen.n *failed* | File "src/filters/exceptions.ml", line 387, characters 75-82
              +gen.n *failed* | Called from file "src/filters/exceptions.ml", line 387, characters 22-82
              +gen.n *failed* | Called from file "src/filters/exceptions.ml", line 455, characters 5-70
              +gen.n *failed* | Called from file "src/filters/exceptions.ml", line 488, characters 14-78
              +gen.n *failed* | Called from file "src/filters/exceptions.ml", line 559, characters 5-46
              +gen.n *failed* | Called from file "list.ml", line 92, characters 20-23
              +gen.n *failed* | Called from file "src/core/texpr.ml", line 131, characters 26-41
              +gen.n *failed* | Called from file "src/core/texpr.ml", line 140, characters 49-61
              +gen.n *failed* | Called from file "src/std.ml", line 26, characters 6-9
              +gen.n *failed* | Called from file "list.ml", line 121, characters 24-34
              +gen.n *failed* | Called from file "src/core/tUnification.ml", line 462, characters 10-15
              +gen.n *failed* | Called from file "src/filters/filtersCommon.ml", line 74, characters 23-91
              +gen.n *failed* | Called from file "list.ml", line 110, characters 12-15
              +gen.n *failed* | Called from file "src/filters/filtersCommon.ml", line 79, characters 2-45
              +gen.n *failed* | Called from file "src/typing/macroContext.ml", line 663, characters 2-54
              +gen.n *failed* | Called from file "src/macro/eval/evalPrototype.ml", line 319, characters 3-11
              +gen.n *failed* | Called from file "list.ml", line 234, characters 17-20
              +gen.n *failed* | Called from file "src/macro/eval/evalPrototype.ml", line 308, characters 17-801
              +gen.n *failed* | Called from file "src/macro/eval/evalMain.ml", line 393, characters 72-99
              +gen.n *failed* | Called from file "src/macro/eval/evalExceptions.ml", line 101, characters 5-8
              +gen.n *failed* | Called from file "src/macro/eval/evalExceptions.ml", line 116, characters 10-38

@Simn
Copy link
Member Author

Simn commented Jan 12, 2024

Thanks for catching that! I just tried to clone api.haxe.org but I'm having some trouble:

Receiving objects:  57% (14123/24776), 146.02 MiB | 1.80 MiB/s
Receiving objects:  58% (14371/24776), 146.02 MiB | 1.80 MiB/s
Receiving objects:  58% (14499/24776), 149.04 MiB | 2.31 MiB/s
error: RPC failed; curl 18 transfer closed with outstanding read data remaining
error: 719 bytes of body are still expected
fetch-pack: unexpected disconnect while reading sideband packet
fatal: early EOF
fatal: fetch-pack: invalid index-pack output

I've attempted a blind fix here: f6805db

@Simn
Copy link
Member Author

Simn commented Jan 12, 2024

Right, it wants --deph=1. Doing so lets me run haxe dox.hxml, haxe gen.hxml and then neko gen.n without issue. Is that the command CI is failing on? I can't quite tell in that log...

@kLabz
Copy link
Contributor

kLabz commented Jan 12, 2024

That was #11477 (5.0.0-alpha.1+ba272a6 from above logs -- edit: and last successful build was with dec37b31a0af73de9ec59db040ce73e6cc37b345 so there's a few more commits in between) which happened to break "old" versions of hxparse which was fixed in Simn/hxparse@876070e

I updated the submodule, let's see what CI thinks ~ (still broken, though that does seem to be similar to what I have locally which does not break 🤔)

@Simn
Copy link
Member Author

Simn commented Jan 13, 2024

@andyli Both Rudy and I cannot reproduce this locally, and we can't tell what state the server has exactly when encountering this problem. Could you give us a hint how to replicate the behavior?

@andyli
Copy link
Member

andyli commented Jan 15, 2024

I merely discovered it since I got GitHub Actions emails about the broken build over api.haxe.org. I don't have much idea on how to reproduce it.

I tried it locally and here are what I discovered:

  • Turn out that error already happens since ba272a6, but dec37b3 still works. I'm not sure about the commit in-between them since there are no builds on build.haxe.org.
  • Somehow the error only occurs when HAXE_STD_PATH is NOT set.

@kLabz
Copy link
Contributor

kLabz commented Jan 15, 2024

@andyli Should be fixed now; api.haxe.org CI is green again :)

@skial skial mentioned this pull request Jan 16, 2024
1 task
0b1kn00b pushed a commit to 0b1kn00b/haxe that referenced this pull request Jan 25, 2024
* remove m_if_feature, make DCE check the meta directly instead

also copy class @:ifFeature to field @:ifFeature

* add CfUsed, CfMaybeUsed, CUsed

Can't get rid of Meta.Used entirely yet because enums and abstracts don't have flags

* don't play silly meta merge games in the typeloader

instead play them in DCE because otherwise we won't deal with __init__ correctly

* update haxelib

it won't help
0b1kn00b pushed a commit to 0b1kn00b/haxe that referenced this pull request Jan 25, 2024
* remove m_if_feature, make DCE check the meta directly instead

also copy class @:ifFeature to field @:ifFeature

* add CfUsed, CfMaybeUsed, CUsed

Can't get rid of Meta.Used entirely yet because enums and abstracts don't have flags

* don't play silly meta merge games in the typeloader

instead play them in DCE because otherwise we won't deal with __init__ correctly

* update haxelib

it won't help
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.

None yet

3 participants