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

Avoid various uses of Option<Span> in favor of using DUMMY_SP in the few cases that used None #122480

Merged
merged 1 commit into from
Mar 18, 2024

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Mar 14, 2024

based on #122471

DUMMY_SP is already the sentinel value we have that says "no span". We don't need to wrap these Spans in a separate Option.

@rustbot
Copy link
Collaborator

rustbot commented Mar 14, 2024

r? @Nadrieril

rustbot has assigned @Nadrieril.
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 S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Mar 14, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 14, 2024

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred in const_evaluatable.rs

cc @BoxyUwU

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

Some changes occurred in match checking

cc @Nadrieril

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rust-log-analyzer

This comment has been minimized.

@RalfJung
Copy link
Member

DUMMY_SP is already the sentinel value we have that says "no span". We don't need to wrap these Spans in a separate Option.

Yeah that's fair... I think I didn't like using DUMMY_SP in so many places, but maybe it's actually good that this is a clear signal "if you have a way to get a span here, please do".

@RalfJung
Copy link
Member

r? @RalfJung
r=me once CI is happy and #122471 landed

@rustbot rustbot assigned RalfJung and unassigned Nadrieril Mar 14, 2024
@bors
Copy link
Contributor

bors commented Mar 14, 2024

☔ The latest upstream changes (presumably #122483) made this pull request unmergeable. Please resolve the merge conflicts.

@RalfJung
Copy link
Member

#122471 landed so this PR should be good to go after a rebase. :)

@oli-obk oli-obk force-pushed the const-eval-span-no-opt branch from 4aabb46 to adda9da Compare March 18, 2024 09:34
@rustbot
Copy link
Collaborator

rustbot commented Mar 18, 2024

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@oli-obk
Copy link
Contributor Author

oli-obk commented Mar 18, 2024

@bors r=RalfJung

@bors
Copy link
Contributor

bors commented Mar 18, 2024

📌 Commit adda9da has been approved by RalfJung

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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 18, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 18, 2024
…RalfJung

Avoid various uses of `Option<Span>` in favor of using `DUMMY_SP` in the few cases that used `None`

based on rust-lang#122471

`DUMMY_SP` is already the sentinel value we have that says "no span". We don't need to wrap these `Span`s in a separate `Option`.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 18, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#122480 (Avoid various uses of `Option<Span>` in favor of using `DUMMY_SP` in the few cases that used `None`)
 - rust-lang#122567 (Remove fixme about LLVM basic block naming)
 - rust-lang#122588 (less useless filter calls in imported_source_file)
 - rust-lang#122647 (add_retag: ensure box-to-raw-ptr casts are preserved for Miri)
 - rust-lang#122649 (Update the minimum external LLVM to 17)
 - rust-lang#122680 (Do not eat nested expressions' results in `MayContainYieldPoint` format args visitor)
 - rust-lang#122683 (add missing test: expected paren or brace in macro)
 - rust-lang#122689 (Add missing `try_visit` calls in visitors.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4608079 into rust-lang:master Mar 18, 2024
11 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Mar 18, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 18, 2024
Rollup merge of rust-lang#122480 - oli-obk:const-eval-span-no-opt, r=RalfJung

Avoid various uses of `Option<Span>` in favor of using `DUMMY_SP` in the few cases that used `None`

based on rust-lang#122471

`DUMMY_SP` is already the sentinel value we have that says "no span". We don't need to wrap these `Span`s in a separate `Option`.
adpaco-aws added a commit to model-checking/kani that referenced this pull request Mar 27, 2024
Upgrades the Rust toolchain to `nightly-2024-03-21`. The relevant
changes in Rust are:
* rust-lang/rust#122480
* rust-lang/rust#122748
* rust-lang/cargo#12783

I wasn't confident that our regression testing could detect differences
in the file paths being generated with the new logic, so I added code
similar to the following just to check they were equivalent:
```rust
             let base_filename = tcx.output_filenames(()).output_path(OutputType::Object);
+            let binding = tcx.output_filenames(()).path(OutputType::Object);
+            let base_filename2 = binding.as_path();
+            assert_eq!(base_filename, base_filename2);
```
Note that this was done for each instance where we used `output_path`,
and completed regression testing with the previous toolchain version. I
also checked manually the instance where we generate a `.dot` graph for
debug purposes following the instructions
[here](https://model-checking.github.io/kani/cheat-sheets.html?highlight=dot#debug)
(a `libmain.dot` file was generated for the `main.rs` in one of my
projects).

---------

Co-authored-by: Celina G. Val <celinval@amazon.com>
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 7, 2024
…rrors

replace another Option<Span> by DUMMY_SP

This was missed in rust-lang#122480.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 7, 2024
Rollup merge of rust-lang#124842 - RalfJung:option-span, r=compiler-errors

replace another Option<Span> by DUMMY_SP

This was missed in rust-lang#122480.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants