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.

Sign up for GitHub

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

Jump to bottom

Add a Few Codegen Tests #132170

Merged
merged 1 commit into from
Nov 2, 2024
Merged

Add a Few Codegen Tests #132170

merged 1 commit into from
Nov 2, 2024

Conversation

veera-sivarajan
Copy link
Contributor

@veera-sivarajan veera-sivarajan commented Oct 26, 2024 •
edited by jieyouxu
Loading

Closes #86109
Closes #64219

Those issues somehow got fixed over time.

So, this PR adds a couple of codegen tests to ensure we don't regress in the future.

@rustbot
Copy link
Collaborator

rustbot commented Oct 26, 2024

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 26, 2024
jieyouxu
jieyouxu requested changes Oct 26, 2024
View reviewed changes
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

The tests themselves look good, and the comments are good, but please also:

  1. Give the tests meaningful names (not just issue-xxxxx). E.g. fn-ptr-never-ret-ty-has-noreturn-attr.rs.
  2. Please move them into meaningful subdirectories instead of issues/.

Other than that, LGTM.

tests/codegen/issues/issue-86109.rs Outdated Show resolved Hide resolved
@jieyouxu jieyouxu assigned jieyouxu and unassigned Mark-Simulacrum Oct 26, 2024
@jieyouxu jieyouxu added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 26, 2024
@jieyouxu
Copy link
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 26, 2024
@veera-sivarajan
Copy link
Contributor Author

Thank you for the feedback.

I've renamed the tests and added the remark. But I cannot find a more appropriate directory for these tests than codegen/issues.

@jieyouxu
Copy link
Member

That's fine. I'll r+ after PR CI is green.

@workingjubilee
Copy link
Member

@bors r=jieyouxu

@bors
Copy link
Contributor

bors commented Oct 27, 2024

📌 Commit 85d6d9c has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 27, 2024
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Oct 27, 2024
@workingjubilee
Rollup merge of rust-lang#132170 - veera-sivarajan:codegen-tests, r=j…
804c8a9 
…ieyouxu

Add a Few Codegen Tests

Closes rust-lang#86109
Closes rust-lang#64219

Those issues somehow got fixed over time.

So, this PR adds a couple of codegen tests to ensure we don't regress in the future.
@workingjubilee workingjubilee mentioned this pull request Oct 27, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 27, 2024
@bors
Auto merge of rust-lang#132211 - workingjubilee:rollup-l9xd4jw, r=wor…
1c365b9 
…kingjubilee

Rollup of 5 pull requests

Successful merges:

 - rust-lang#132123 (allow type-based search on foreign functions)
 - rust-lang#132170 (Add a Few Codegen Tests)
 - rust-lang#132183 (Fix code HTML items making big blocks if too long)
 - rust-lang#132192 (expand: Stop using artificial `ast::Item` for macros loaded from metadata)
 - rust-lang#132205 (docs: Correctly link riscv32e from platform-support.md)

r? `@ghost`
`@rustbot` modify labels: rollup
@workingjubilee
Copy link
Member

did not pass: #132211 (comment)

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 27, 2024
@jieyouxu
Copy link
Member

Yeah, that matched on a tail call on aarc64 apple

@jieyouxu
Copy link
Member

jieyouxu commented Nov 2, 2024

Thanks!
@bors r+ rollup=iffy

@bors
Copy link
Contributor

bors commented Nov 2, 2024

📌 Commit d2aa910 has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 2, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request Nov 2, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 2, 2024
@bors
Auto merge of rust-lang#132497 - matthiaskrgr:rollup-gaskcn0, r=matth…
588a420 
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#131037 (Move versioned Apple LLVM targets from `rustc_target` to `rustc_codegen_ssa`)
 - rust-lang#132170 (Add a Few Codegen Tests)
 - rust-lang#132333 (rust_analyzer_helix.toml: add library/ manifest)
 - rust-lang#132398 (Add a couple of intra-doc links to str)
 - rust-lang#132411 (CI: switch dist-x86_64-musl to free runner)
 - rust-lang#132453 (Also treat `impl` definition parent as transparent regarding modules)
 - rust-lang#132457 (Remove needless #![feature(asm_experimental_arch)] from loongarch64 inline assembly test)
 - rust-lang#132465 (refactor(config): remove FIXME statement in comment of `omit-git-hash`)
 - rust-lang#132466 (Account for late-bound depth when capturing all opaque lifetimes.)
 - rust-lang#132471 (Add a bunch of mailmap entries)
 - rust-lang#132488 (Remove or fix some more `FIXME(async_closure)`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a513e0e into rust-lang:master Nov 2, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 2, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 2, 2024
@rust-timer
Unrolled build for rust-lang#132170
268c1bd 
Rollup merge of rust-lang#132170 - veera-sivarajan:codegen-tests, r=jieyouxu

Add a Few Codegen Tests

Closes rust-lang#86109
Closes rust-lang#64219

Those issues somehow got fixed over time.

So, this PR adds a couple of codegen tests to ensure we don't regress in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jieyouxu jieyouxu jieyouxu requested changes

Assignees

@jieyouxu jieyouxu

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.84.0
6 participants
@veera-sivarajan @rustbot @jieyouxu @workingjubilee @bors @Mark-Simulacrum