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

Allow unused variables with todo! #79850

46 changes: 44 additions & 2 deletions compiler/rustc_passes/src/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,14 @@
use self::LiveNodeKind::*;
use self::VarKind::*;

use rustc_ast::InlineAsmOptions;
use rustc_ast::{InlineAsmOptions, LitKind, StrStyle};
use rustc_data_structures::fx::FxIndexMap;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::def::*;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::{Expr, HirId, HirIdMap, HirIdSet};
use rustc_hir::{Expr, ExprKind, HirId, HirIdMap, HirIdSet, QPath};
use rustc_index::vec::IndexVec;
use rustc_middle::hir::map::Map;
use rustc_middle::ty::query::Providers;
Expand Down Expand Up @@ -329,6 +329,48 @@ impl<'tcx> Visitor<'tcx> for IrMaps<'tcx> {
}
}

// Allow todo! macro
/*
Skips checking for unused variables when the trailing expression
of the body is a panic with a message that contains "not yet implemented".
Copy link
Member

Choose a reason for hiding this comment

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

I would still include something saying that this is a somewhat hacky way to check if it's todo!.


# Example

fn foo(x: i32) {
// arbitrary code
todo!()
}
*/
if let ExprKind::Block(block, _) = &body.value.kind {
if let Some(expr) = block.expr {
if let ExprKind::Call(call, [arg]) = expr.kind {
if let ExprKind::Path(QPath::Resolved(_, path)) = call.kind {
if let Res::Def(DefKind::Fn, path_def_id) = &path.res {
let panic_fn = self.tcx.lang_items().panic_fn();
// Note: there is no function for panic_fmt, so we have to extract it from the debug output :(
// builder doesn't like this being called without panic `self.tcx.def_path_str(*path_def_id);`
let path_str = format!("{:?}", path_def_id);
Comment on lines +354 to +355
Copy link
Member

Choose a reason for hiding this comment

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

What does this mean? Why did self.tcx.def_path_str not work?

Copy link
Author

Choose a reason for hiding this comment

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

It errors with:

thread 'rustc' panicked at 'no warnings or errors encountered even though `delayed_good_path_bugs` issued', compiler/rustc_errors/src/lib.rs:974:13

if Some(*path_def_id) == panic_fn
|| ((path_str.contains("std[") || path_str.contains("core["))
Copy link
Member

Choose a reason for hiding this comment

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

By the way, why does this use [?

Copy link
Author

@charles-r-earp charles-r-earp Jan 17, 2021

Choose a reason for hiding this comment

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

The path_str is like core[76aa]::panicking::panic. I was concerned about matching on some arbitrary crate with a matching path, though it's a bit unlikely. I actually think it should be ~ core[" .. "]::panicking::panic" .. just to ensure that it only triggers for std / core.

&& path_str.contains("panicking::panic"))
{
if let ExprKind::Lit(spanned) = &arg.kind {
if let LitKind::Str(symbol, StrStyle::Cooked) = spanned.node {
if symbol.as_str().starts_with("not yet implemented") {
return;
}
}
} else if format!("{:?}", &arg.kind).contains("not yet implemented")
{
return;
}
}
}
}
}
}
}

if let Some(captures) = maps.tcx.typeck(local_def_id).closure_captures.get(&def_id) {
for &var_hir_id in captures.keys() {
let var_name = maps.tcx.hir().name(var_hir_id);
Expand Down
30 changes: 30 additions & 0 deletions src/test/ui/unused/allow-unused-variables-with-todo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// check-pass

#[deny(unused_variables)]
fn plain(x: i32, y: i32) -> i32 {
todo!()
}

#[deny(unused_variables)]
fn message(x: i32, y: i32) -> i32 {
todo!("message")
}

#[deny(unused_variables)]
fn statement(x: i32, y: i32) -> i32 {
let z = x + y;
todo!()
}

#[deny(unused_variables)]
fn statement_message(x: i32, y: i32) -> i32 {
let z = x + y;
todo!("message")
}

fn main() {
plain(0, 1);
message(0, 1);
statement(0, 1);
statement_message(0, 1);
}