-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Suggest replacing typeof(...)
with an actual type
#95784
Conversation
This commit suggests replacing typeof(...) with an actual type of "...", for example in case of `typeof(1)` we suggest replacing it with `i32`. If the expression - Is not const (`{ let a = 1; let _: typeof(a); }`) - Can't be found (`let _: typeof(this_variable_does_not_exist)`) - Or has non-suggestable type (closure, generator, error, etc) we don't suggest anything.
r? @fee1-dead (rust-highfive has picked a reviewer for you, use r? to override) |
@@ -2462,8 +2462,16 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { | |||
self.normalize_ty(ast_ty.span, array_ty) | |||
} | |||
hir::TyKind::Typeof(ref e) => { | |||
tcx.sess.emit_err(TypeofReservedKeywordUsed { span: ast_ty.span }); | |||
tcx.type_of(tcx.hir().local_def_id(e.hir_id)) | |||
let ty = tcx.type_of(tcx.hir().local_def_id(e.hir_id)); |
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.
does this report a cycle error if we do something like:
fn foo() -> typeof(foo) {}
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 does
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.
hrm.. wonder if that's ez to fix... probably not. should be ok, since this is illegal syntax anyways.
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.
Does this ICE? If then it should be fixed. Not necessarily in this PR though.
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 doesn't ICE, it just shows the usual cycle error like "cycle detected when computing function signature of foo
"
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=c657d61f3db8f63bce43cffa68c1f85b
This comment has been minimized.
This comment has been minimized.
this suggestion is relatively harmless, and we already have the infrastructure to suggest it (since we parse the inside of typeof as an AnonConst)... @bors delegate+ |
✌️ @WaffleLapkin can now approve this pull request |
@bors r+ rollup |
📌 Commit 8412d5d has been approved by |
…, r=compiler-errors Suggest replacing `typeof(...)` with an actual type This PR adds suggestion to replace `typeof(...)` with an actual type of `...`, for example in case of `typeof(1)` we suggest replacing it with `i32`. If the expression 1. Is not const (`{ let a = 1; let _: typeof(a); }`) 2. Can't be found (`let _: typeof(this_variable_does_not_exist)`) 3. Or has non-suggestable type (closure, generator, error, etc) we don't suggest anything. The 1 one is sad, but it's not clear how to support non-consts expressions for `typeof`. _This PR is inspired by [this tweet]._ [this tweet]: https://twitter.com/compiler_errors/status/1511945354752638976
Rollup of 7 pull requests Successful merges: - rust-lang#95566 (Avoid duplication of doc comments in `std::char` constants and functions) - rust-lang#95784 (Suggest replacing `typeof(...)` with an actual type) - rust-lang#95807 (Suggest adding a local for vector to fix borrowck errors) - rust-lang#95849 (Check for git submodules in non-git source tree.) - rust-lang#95852 (Fix missing space in lossy provenance cast lint) - rust-lang#95857 (Allow multiple derefs to be splitted in deref_separator) - rust-lang#95868 (rustdoc: Reduce allocations in a `html::markdown` function) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This PR adds suggestion to replace
typeof(...)
with an actual type of...
, for example in case oftypeof(1)
we suggest replacing it withi32
.If the expression
{ let a = 1; let _: typeof(a); }
)let _: typeof(this_variable_does_not_exist)
)we don't suggest anything.
The 1 one is sad, but it's not clear how to support non-consts expressions for
typeof
.This PR is inspired by this tweet.