Skip to content

Commit fb26301

Browse files
committed
temporary_as_ptr: fixes
1. Fix `is_temporary_rvalue` for: - Unary operations (such as dereference) - `yield` - Literals, array literals, repeat array literals - `DropTemps` 2. Fix typecheck to account for `T` in `Box<T>`
1 parent 3301eb1 commit fb26301

File tree

1 file changed

+43
-35
lines changed

1 file changed

+43
-35
lines changed

compiler/rustc_lint/src/methods.rs

Lines changed: 43 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use rustc_hir::{Expr, ExprKind, LangItem};
2-
use rustc_middle::ty;
2+
use rustc_middle::ty::Ty;
33
use rustc_session::{declare_lint, declare_lint_pass};
44
use rustc_span::symbol::{sym, Ident};
55

@@ -36,7 +36,7 @@ declare_lint! {
3636
/// TODO
3737
pub TEMPORARY_AS_PTR,
3838
Warn,
39-
"TODO"
39+
"detects getting the inner pointer of a temporary container"
4040
}
4141

4242
declare_lint_pass!(TemporaryCStringAsPtr => [TEMPORARY_CSTRING_AS_PTR, TEMPORARY_AS_PTR]);
@@ -57,33 +57,15 @@ impl<'tcx> LateLintPass<'tcx> for TemporaryCStringAsPtr {
5757

5858
// It is called on a temporary rvalue.
5959
let is_temp = is_temporary_rvalue(receiver);
60-
tracing::debug!(?receiver, ?is_temp);
60+
tracing::debug!(receiver = ?receiver.kind, ?is_temp);
6161
if !is_temp {
6262
return;
6363
}
6464

6565
// The temporary value's type is array, box, Vec, String, or CString
6666
let ty = cx.typeck_results().expr_ty(receiver);
6767
tracing::debug!(?ty);
68-
69-
// None => not a container
70-
// Some(true) => CString
71-
// Some(false) => String, Vec, box, array
72-
let lint_is_cstring = match ty.kind() {
73-
ty::Array(_, _) => Some(false),
74-
ty::Adt(def, _) if def.is_box() => Some(false),
75-
ty::Adt(def, _) if cx.tcx.lang_items().get(LangItem::String) == Some(def.did()) => {
76-
Some(false)
77-
}
78-
ty::Adt(def, _) => match cx.tcx.get_diagnostic_name(def.did()) {
79-
Some(sym::Vec) => Some(false),
80-
Some(sym::cstring_type) => Some(true),
81-
_ => None,
82-
},
83-
_ => None,
84-
};
85-
tracing::debug!(?lint_is_cstring);
86-
let Some(is_cstring) = lint_is_cstring else {
68+
let Some(is_cstring) = as_container(cx, ty) else {
8769
return;
8870
};
8971

@@ -106,10 +88,10 @@ impl<'tcx> LateLintPass<'tcx> for TemporaryCStringAsPtr {
10688
fn is_temporary_rvalue(expr: &Expr<'_>) -> bool {
10789
match expr.kind {
10890
// We are not interested in these
109-
ExprKind::Cast(_, _) | ExprKind::Closure(_) | ExprKind::Tup(_) => false,
91+
ExprKind::Cast(_, _) | ExprKind::Closure(_) | ExprKind::Tup(_) | ExprKind::Lit(_) => false,
11092

11193
// Const is not temporary.
112-
ExprKind::ConstBlock(_) => false,
94+
ExprKind::ConstBlock(_) | ExprKind::Repeat(_, _) => false,
11395

11496
// This is literally lvalue.
11597
ExprKind::Path(_) => false,
@@ -120,39 +102,65 @@ fn is_temporary_rvalue(expr: &Expr<'_>) -> bool {
120102
| ExprKind::Index(_, _, _)
121103
| ExprKind::Binary(_, _, _) => true,
122104

123-
// TODO: Check if x: &String, *(x).as_ptr() gets triggered
124-
ExprKind::Unary(_, _) => true,
105+
// This is likely a dereference.
106+
ExprKind::Unary(_, _) => false,
125107

126108
// Inner blocks are rvalues.
127109
ExprKind::If(_, _, _)
128110
| ExprKind::Loop(_, _, _, _)
129111
| ExprKind::Match(_, _, _)
130112
| ExprKind::Block(_, _) => true,
131113

114+
ExprKind::DropTemps(inner) => is_temporary_rvalue(inner),
132115
ExprKind::Field(parent, _) => is_temporary_rvalue(parent),
133116

134-
// FIXME: some of these get promoted to const/'static ?
135-
ExprKind::Struct(_, _, _)
136-
| ExprKind::Array(_)
137-
| ExprKind::Repeat(_, _)
138-
| ExprKind::Lit(_) => true,
117+
ExprKind::Struct(_, _, _) => true,
118+
// False negatives are possible, but arrays get promoted to 'static way too often.
119+
ExprKind::Array(_) => false,
139120

140121
// These typecheck to `!`
141122
ExprKind::Break(_, _) | ExprKind::Continue(_) | ExprKind::Ret(_) | ExprKind::Become(_) => {
142123
false
143124
}
144125

145126
// These typecheck to `()`
146-
ExprKind::Assign(_, _, _) | ExprKind::AssignOp(_, _, _) => false,
127+
ExprKind::Assign(_, _, _) | ExprKind::AssignOp(_, _, _) | ExprKind::Yield(_, _) => false,
147128

148129
// Not applicable
149130
ExprKind::Type(_, _) | ExprKind::Err(_) | ExprKind::Let(_) => false,
150131

151132
// These are compiler-magic macros
152133
ExprKind::AddrOf(_, _, _) | ExprKind::OffsetOf(_, _) | ExprKind::InlineAsm(_) => false,
134+
}
135+
}
153136

154-
// TODO: WTF are these
155-
ExprKind::DropTemps(_) => todo!(),
156-
ExprKind::Yield(_, _) => todo!(),
137+
// None => not a container
138+
// Some(true) => CString
139+
// Some(false) => String, Vec, box, array
140+
fn as_container(cx: &LateContext<'_>, ty: Ty<'_>) -> Option<bool> {
141+
if ty.is_array() {
142+
Some(false)
143+
} else if ty.is_box() {
144+
let inner = ty.boxed_ty();
145+
// We only care about Box<[..]>, Box<str>, Box<CStr>,
146+
// or Box<T> iff T is another type we care about
147+
if inner.is_slice()
148+
|| inner.is_str()
149+
|| inner.ty_adt_def().is_some_and(|def| cx.tcx.is_lang_item(def.did(), LangItem::CStr))
150+
|| as_container(cx, inner).is_some()
151+
{
152+
Some(false)
153+
} else {
154+
None
155+
}
156+
} else if let Some(def) = ty.ty_adt_def() {
157+
match cx.tcx.get_diagnostic_name(def.did()) {
158+
Some(sym::cstring_type) => Some(true),
159+
Some(sym::Vec) => Some(false),
160+
_ if cx.tcx.is_lang_item(def.did(), LangItem::String) => Some(false),
161+
_ => None,
162+
}
163+
} else {
164+
None
157165
}
158166
}

0 commit comments

Comments
 (0)