-
-
Notifications
You must be signed in to change notification settings - Fork 165
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
chore: Update rust-toolchain to nightly-2023-10-04 #1095
Conversation
|
Ah, of course I should have expected |
Thank you for the PR! Files to checkThese are files which is affected by the current PR, but not reflected. If there's no file below this message, please ignore this message. You can run
|
Let me know if you have any questions or preferred fixes. The main one is The |
use swc_ecma_parser::{lexer::Lexer, Parser, StringInput, Syntax, TsConfig}; | ||
use swc_macros_common::{call_site, print}; | ||
use syn::{punctuated::Punctuated, Token}; | ||
|
||
#[proc_macro] | ||
pub fn builtin(_: proc_macro::TokenStream) -> proc_macro::TokenStream { | ||
swc_common::GLOBALS.set(&swc_common::Globals::new(), || { | ||
let cm = Arc::new(SourceMap::new(FilePathMapping::empty())); | ||
let cm = Lrc::new(SourceMap::new(FilePathMapping::empty())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was complaining about Arc
on a non-Send/Sync type, interestingly. I think that's probably a clippy bug? Strictly, you don't even need an rc at all, but the docs for SourceMap suggest using this alias, which suppresses the lint.
@@ -27,6 +27,7 @@ impl Analyzer<'_, '_> { | |||
/// orders. | |||
/// | |||
/// After splitting, we can check if each element is assignable. | |||
#[allow(clippy::needless_pass_by_ref_mut)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Complaining about data
not being mutably used here (though it's not even actually used, there's an allow(unused)
)
I didn't want to change APIs here, but that's probably more correct.
"infer_arg_types: {:?}", | ||
type_params.iter().map(|p| format!("{}, ", p.name)).collect::<String>() | ||
); | ||
warn!("infer_arg_types: {:?}", type_params.iter().map(|p| &p.name).join(", ")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically a different format (no trailing ,
), but it was complaining about the inefficient format, and this is much clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! All looks good
Description:
Just updating the toolchain to the latest to get my toes wet. Everything I could try still seems to act the same as far as I can tell, just needed to bump
proc-macro2
for a removed feature, and I setworkspace.resolver
explicitly due to what seems like a new warning - since there aren't any top-level dependencies it seems like this didn't change any dependencies?BREAKING CHANGE:
Shouldn't be anything other than possibly affecting MSRV if there is any policy there.
Related issue (if exists):
Last update was #1034