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

feat: add basic support for function call #12

Merged
merged 9 commits into from
Feb 26, 2025

Conversation

xizheyin
Copy link
Contributor

@xizheyin xizheyin commented Feb 19, 2025

  • add basic support for function call
  • print the array type
  • add new directives and unify logic for parsing them
    • new directive:
      • //@ check-stdout: xxx
      • //@ exit-code: xxx
      • //@ check-stderr: xxx
    • invalid detection.

@tgross35
Copy link
Contributor

For the test changes, I think we should try to mirror https://rustc-dev-guide.rust-lang.org/tests/directives.html. That is, the test should have something like:

//@ run-pass
//@ exit-code: 1

Rather than a separate file

@xizheyin
Copy link
Contributor Author

Yeah, let me go over this testing process.

@xizheyin
Copy link
Contributor Author

For the test changes, I think we should try to mirror https://rustc-dev-guide.rust-lang.org/tests/directives.html. That is, the test should have something like:

//@ run-pass
//@ exit-code: 1

Rather than a separate file

I have solved this.

@tgross35
Copy link
Contributor

Thanks, I think we could make things a little more consistent still:

  • We don't need a new directory called runit
  • Instead, a directive //@ run-pass can be added to any test (e.g. tests/codegen/func_call.rs) which means that it should be compiled, run, and exit with success (asserts exit code = 0).
  • run-pass tests can optionally have an //@ exit-code: 1 directive. Same behavior as above, just assert a different exit code
  • //@ exit-code without //@ run-pass is an error

I don't think you need to worry about the check-stdout directive since none of the tests print anything yet, but it's fine to leave as long as it isn't required.

The codegen changes here lgtm 👍

@xizheyin
Copy link
Contributor Author

xizheyin commented Feb 23, 2025

@tgross35
Ok, I have added the directive run-pass, and remove runit/.

When type ./y test, it's just like:

nju@njuyxz:~/rustc_codegen_c$ ./y test
[TEST] found 7 testcases 
[TEST] TEST CompileLib auxiliary/mini_core
[TEST] TEST Compile examples/basic_math
[TEST] TEST file checking codegen/filename
[TEST] TEST file checking codegen/func_call
[TEST] TEST file checking codegen/params_count
[TEST] TEST file checking codegen/ret_value
[TEST] TEST Bless bless/basic_math

While typing ./y test -v or ./y -v test, it's just like:

[TEST] TEST file checking codegen/func_call
       source: tests/codegen/func_call.rs
       output: build/tests/codegen/func_call
       compile: CFLAGS=-Irust_runtime rustc --edition 2021 -Z codegen-backend=crates/target/debug/librustc_codegen_c.so -C panic=abort -C lto=false -Lall=build -lc -lrust_runtime --crate-type bin -O tests/codegen/func_call.rs -o build/tests/codegen/func_call
       success
       filecheck: "/usr/bin/FileCheck-18" "tests/codegen/func_call.rs"
       success
       directives: found 3 directives
       running: build/tests/codegen/func_call
       checking exit code: 1
       exit code: passed
       result: all checks passed
[TEST] TEST file checking codegen/params_count
       source: tests/codegen/params_count.rs
       output: build/tests/codegen/params_count
       compile: CFLAGS=-Irust_runtime rustc --edition 2021 -Z codegen-backend=crates/target/debug/librustc_codegen_c.so -C panic=abort -C lto=false -Lall=build -lc -lrust_runtime --crate-type bin -O tests/codegen/params_count.rs -o build/tests/codegen/params_count
       success
       filecheck: "/usr/bin/FileCheck-18" "tests/codegen/params_count.rs"
       success
       directives: found 1 directives
       result: all checks passed

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple small adjustments but mostly lgtm 👍

@xizheyin xizheyin requested a review from tgross35 February 26, 2025 06:26
Signed-off-by: xizheyin <[email protected]>
@xizheyin xizheyin requested a review from tgross35 February 26, 2025 08:59
Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thank you!

@tgross35 tgross35 merged commit 6044b30 into rust-lang:main Feb 26, 2025
5 checks passed
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.

2 participants