Try bumping our nightly version through the power of slop-coding#1614
Try bumping our nightly version through the power of slop-coding#1614randomPoison wants to merge 26 commits intomasterfrom
Conversation
| ConstBlock(anon_const), | ||
| Call(#[prec=POSTFIX] #[prec_special=Callee] func, #[mac_table_seq] args), | ||
| MethodCall(path_seg, #[prec_first=POSTFIX] #[mac_table_seq] args, span), | ||
| MethodCall(path_seg, #[prec=POSTFIX] receiver, #[mac_table_seq] args, span), |
There was a problem hiding this comment.
This is our internal attribute, I wonder why it decided this change was needed.
There was a problem hiding this comment.
This is necessary because the method call receiver was pulled out of the args list into its own field. The old prec_first attribute was specifically there to handle this case in the method call syntax, where we needed to give the first element of the args list different precedence. Now that the receiver is its own field, the prec attribute is the one to use.
Also, prec_first was only used in this one place, so we can probably remove it entirely after rolling the toolchain (unless you think we should keep it around in case that case comes up again in a future toolchain).
| attr.style = style; | ||
| self.attrs.push(attr); | ||
| self.attrs.push(Attribute { | ||
| id: AttrId::from_u32(0), |
There was a problem hiding this comment.
We have a constant for this, DUMMY_NODE_ID I think.
There was a problem hiding this comment.
DUMMY_NODE_ID is a NodeId, this as an AttrId. Other code within this file is doing AttrId::from_u32(0), hence why it did the same here.
| match self.kind { | ||
| // FIXME: can this conflict with regular Err literals??? | ||
| LitKind::Err(ref sym) => Some(*sym), | ||
| LitKind::Err => Some(self.token_lit.symbol), |
There was a problem hiding this comment.
Should we keep the comment?
There was a problem hiding this comment.
I think this comment is now irrelevant, though what we're doing here is a little weird so it's a bit hard to tell. This functionality is used in the matcher syntax, so we can do matches like $x:Lit. This ends up getting parsed in the AST as a LitKind::Err, since it's not a valid literal, and we then extract the $x part of the symbol from the error. This is a bit weird because we're overloading LitKind::Err for a case that's not actually an error, but it seems to work for our purposes.
We didn't have any tests around matching literals, so I've added a couple snapshot tests to cover that, along with adding support for substituting the matched literal into the rewritten code, which didn't previously work. I think we can remove the FIXME comment given that we can now be somewhat confident that it works as intended.
| config.opts.incremental = None; | ||
|
|
||
| run_in_thread_pool_with_globals(Edition::Edition2021, 1, move || { | ||
| rustc_span::create_session_globals_then(Edition::Edition2021, move || { |
There was a problem hiding this comment.
This seems extremely questionably, create_session_globals_then was around in the old nightly, and the other function did a lot more than just call it. We'll need to check what happened upstream.
There was a problem hiding this comment.
run_in_thread_pool_with_globals was made private (well, pub(crate)) so we can't call it anymore. create_session_globals is used internally in run_in_thread_pool_with_globals, so it makes sense that that's the fix here. It doesn't seem like we need to thread pool to run the refactorer here, so maybe create_session_globals does what we need? At least it doesn't seem to be interfering with our ability to run the refactorer.
There was a problem hiding this comment.
Oh, we were specifically running it with only 1 thread before, so I think we can do without the thread pool.
| // collide with `DUMMY_SP` (which is `0 .. 0`). | ||
| source_map.new_source_file(FileName::Custom("<dummy>".to_string()), " ".to_string()); | ||
|
|
||
| let codegen_backend = get_codegen_backend( |
There was a problem hiding this comment.
Really weird that it decided to delete a bunch of code here, we need to check upstream.
|
Why was that particular nightly version chosen and not, say, a much newer one? |
kkysen
left a comment
kkysen
left a comment
There was a problem hiding this comment.
Why was that particular nightly version chosen and not, say, a much newer one?
We were mostly just testing out codex on this. We decided to first start with a smaller jump so that it could handle it better. We may try a longer jump next, but there's a larger risk things go wrong, and in case we introduce some regression, it'll be harder to debug/rollback, and a much larger set of changes will be much more difficult to review as well.
And also add the new Assume intrinsic while we're at it.
randomPoison
force-pushed
the
legare/codex-bump-nightly-experiment
branch
from
6efc208 to
017cb29
Compare
| PatKind::Ident(BindingMode::ByValue(Mutability::Not), ref i, None) => { | ||
| i.pattern_symbol() | ||
| } | ||
| PatKind::Ident(BindingAnnotation(ByRef::No, _), ref i, None) => i.pattern_symbol(), |
There was a problem hiding this comment.
Oh weird, this one actually fixed a bug in the rewriter where we weren't handling mut patterns correctly. I added tests to exercise the relevant code paths and confirmed that the new behavior is correct.
4 participants
Copilot choked and died on this, but Codex seems to have succeeded. I let it spin for like an hour and it got code compiling and tests running, so let's let CI run and see if anything is broken that I didn't catch locally.