Skip to content

Commit 3301eb1

Browse files
GrigorenkoPVGrigorenkoPV
committed
Add temporary_as_ptr lint, reuse it for temporary_cstring_as_ptr 
1 parent c9bd03c commit 3301eb1

File tree

4 files changed

+131
-40
lines changed

4 files changed

+131
-40
lines changed

‎compiler/rustc_lint/messages.ftl

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -209,12 +209,6 @@ lint_crate_name_in_cfg_attr_deprecated =
209209
lint_crate_type_in_cfg_attr_deprecated =
210210
`crate_type` within an `#![cfg_attr]` attribute is deprecated
211211
212-
lint_cstring_ptr = getting the inner pointer of a temporary `CString`
213-
.as_ptr_label = this pointer will be invalid
214-
.unwrap_label = this `CString` is deallocated at the end of the statement, bind it to a variable to extend its lifetime
215-
.note = pointers do not have a lifetime; when calling `as_ptr` the `CString` will be deallocated at the end of the statement because nothing is referencing it as far as the type system is concerned
216-
.help = for more information, see https://doc.rust-lang.org/reference/destructors.html
217-
218212
lint_custom_inner_attribute_unstable = custom inner attributes are unstable
219213
220214
lint_default_hash_types = prefer `{$preferred}` over `{$used}`, it has better performance
@@ -758,6 +752,12 @@ lint_suspicious_double_ref_clone =
758752
lint_suspicious_double_ref_deref =
759753
using `.deref()` on a double reference, which returns `{$ty}` instead of dereferencing the inner type
760754
755+
lint_temporary_as_ptr = getting the inner pointer of a temporary `{$ty}`
756+
.as_ptr_label = this pointer will be invalid
757+
.temporary_label = this `{$ty}` is deallocated at the end of the statement, bind it to a variable to extend its lifetime
758+
.note = pointers do not have a lifetime; when calling `{$method}` the `{$ty}` will be deallocated at the end of the statement because nothing is referencing it as far as the type system is concerned
759+
.help = for more information, see https://doc.rust-lang.org/reference/destructors.html
760+
761761
lint_trailing_semi_macro = trailing semicolon in macro used in expression position
762762
.note1 = macro invocations at the end of a block are treated as expressions
763763
.note2 = to ignore the value produced by the macro, add a semicolon after the invocation of `{$name}`

‎compiler/rustc_lint/src/lints.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1126,14 +1126,16 @@ pub struct IgnoredUnlessCrateSpecified<'a> {
11261126

11271127
// methods.rs
11281128
#[derive(LintDiagnostic)]
1129-
#[diag(lint_cstring_ptr)]
1129+
#[diag(lint_temporary_as_ptr)]
11301130
#[note]
11311131
#[help]
1132-
pub struct CStringPtr {
1132+
pub struct TemporaryAsPtr {
1133+
pub method: Symbol,
1134+
pub ty: String,
11331135
#[label(lint_as_ptr_label)]
1134-
pub as_ptr: Span,
1135-
#[label(lint_unwrap_label)]
1136-
pub unwrap: Span,
1136+
pub as_ptr_span: Span,
1137+
#[label(lint_temporary_label)]
1138+
pub temporary_span: Span,
11371139
}
11381140

11391141
// multiple_supertrait_upcastable.rs

‎compiler/rustc_lint/src/methods.rs

Lines changed: 117 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
1-
use rustc_hir::{Expr, ExprKind};
1+
use rustc_hir::{Expr, ExprKind, LangItem};
22
use rustc_middle::ty;
33
use rustc_session::{declare_lint, declare_lint_pass};
4-
use rustc_span::symbol::sym;
5-
use rustc_span::Span;
4+
use rustc_span::symbol::{sym, Ident};
65

7-
use crate::lints::CStringPtr;
6+
use crate::lints::TemporaryAsPtr;
87
use crate::{LateContext, LateLintPass, LintContext};
98

109
declare_lint! {
@@ -33,38 +32,127 @@ declare_lint! {
3332
"detects getting the inner pointer of a temporary `CString`"
3433
}
3534

36-
declare_lint_pass!(TemporaryCStringAsPtr => [TEMPORARY_CSTRING_AS_PTR]);
35+
declare_lint! {
36+
/// TODO
37+
pub TEMPORARY_AS_PTR,
38+
Warn,
39+
"TODO"
40+
}
41+
42+
declare_lint_pass!(TemporaryCStringAsPtr => [TEMPORARY_CSTRING_AS_PTR, TEMPORARY_AS_PTR]);
3743

3844
impl<'tcx> LateLintPass<'tcx> for TemporaryCStringAsPtr {
3945
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
40-
if let ExprKind::MethodCall(as_ptr_path, as_ptr_receiver, ..) = expr.kind
41-
&& as_ptr_path.ident.name == sym::as_ptr
42-
&& let ExprKind::MethodCall(unwrap_path, unwrap_receiver, ..) = as_ptr_receiver.kind
43-
&& (unwrap_path.ident.name == sym::unwrap || unwrap_path.ident.name == sym::expect)
44-
{
45-
lint_cstring_as_ptr(cx, as_ptr_path.ident.span, unwrap_receiver, as_ptr_receiver);
46+
// We have a method call.
47+
let ExprKind::MethodCall(method, receiver, _args, _span) = expr.kind else {
48+
return;
49+
};
50+
let Ident { name: method_name, span: method_span } = method.ident;
51+
tracing::debug!(?method);
52+
53+
// The method is `.as_ptr()` or `.as_mut_ptr`.
54+
if method_name != sym::as_ptr && method_name != sym::as_mut_ptr {
55+
return;
56+
}
57+
58+
// It is called on a temporary rvalue.
59+
let is_temp = is_temporary_rvalue(receiver);
60+
tracing::debug!(?receiver, ?is_temp);
61+
if !is_temp {
62+
return;
4663
}
64+
65+
// The temporary value's type is array, box, Vec, String, or CString
66+
let ty = cx.typeck_results().expr_ty(receiver);
67+
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 {
87+
return;
88+
};
89+
90+
let span = method.ident.span;
91+
let decorator = TemporaryAsPtr {
92+
method: method_name,
93+
ty: ty.to_string(),
94+
as_ptr_span: method_span,
95+
temporary_span: receiver.span,
96+
};
97+
98+
if is_cstring {
99+
cx.emit_span_lint(TEMPORARY_CSTRING_AS_PTR, span, decorator);
100+
} else {
101+
cx.emit_span_lint(TEMPORARY_AS_PTR, span, decorator);
102+
};
47103
}
48104
}
49105

50-
fn lint_cstring_as_ptr(
51-
cx: &LateContext<'_>,
52-
as_ptr_span: Span,
53-
source: &rustc_hir::Expr<'_>,
54-
unwrap: &rustc_hir::Expr<'_>,
55-
) {
56-
let source_type = cx.typeck_results().expr_ty(source);
57-
if let ty::Adt(def, args) = source_type.kind() {
58-
if cx.tcx.is_diagnostic_item(sym::Result, def.did()) {
59-
if let ty::Adt(adt, _) = args.type_at(0).kind() {
60-
if cx.tcx.is_diagnostic_item(sym::cstring_type, adt.did()) {
61-
cx.emit_span_lint(
62-
TEMPORARY_CSTRING_AS_PTR,
63-
as_ptr_span,
64-
CStringPtr { as_ptr: as_ptr_span, unwrap: unwrap.span },
65-
);
66-
}
67-
}
106+
fn is_temporary_rvalue(expr: &Expr<'_>) -> bool {
107+
match expr.kind {
108+
// We are not interested in these
109+
ExprKind::Cast(_, _) | ExprKind::Closure(_) | ExprKind::Tup(_) => false,
110+
111+
// Const is not temporary.
112+
ExprKind::ConstBlock(_) => false,
113+
114+
// This is literally lvalue.
115+
ExprKind::Path(_) => false,
116+
117+
// Calls return rvalues.
118+
ExprKind::Call(_, _)
119+
| ExprKind::MethodCall(_, _, _, _)
120+
| ExprKind::Index(_, _, _)
121+
| ExprKind::Binary(_, _, _) => true,
122+
123+
// TODO: Check if x: &String, *(x).as_ptr() gets triggered
124+
ExprKind::Unary(_, _) => true,
125+
126+
// Inner blocks are rvalues.
127+
ExprKind::If(_, _, _)
128+
| ExprKind::Loop(_, _, _, _)
129+
| ExprKind::Match(_, _, _)
130+
| ExprKind::Block(_, _) => true,
131+
132+
ExprKind::Field(parent, _) => is_temporary_rvalue(parent),
133+
134+
// FIXME: some of these get promoted to const/'static ?
135+
ExprKind::Struct(_, _, _)
136+
| ExprKind::Array(_)
137+
| ExprKind::Repeat(_, _)
138+
| ExprKind::Lit(_) => true,
139+
140+
// These typecheck to `!`
141+
ExprKind::Break(_, _) | ExprKind::Continue(_) | ExprKind::Ret(_) | ExprKind::Become(_) => {
142+
false
68143
}
144+
145+
// These typecheck to `()`
146+
ExprKind::Assign(_, _, _) | ExprKind::AssignOp(_, _, _) => false,
147+
148+
// Not applicable
149+
ExprKind::Type(_, _) | ExprKind::Err(_) | ExprKind::Let(_) => false,
150+
151+
// These are compiler-magic macros
152+
ExprKind::AddrOf(_, _, _) | ExprKind::OffsetOf(_, _) | ExprKind::InlineAsm(_) => false,
153+
154+
// TODO: WTF are these
155+
ExprKind::DropTemps(_) => todo!(),
156+
ExprKind::Yield(_, _) => todo!(),
69157
}
70158
}

‎compiler/rustc_span/src/symbol.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,7 @@ symbols! {
411411
arm,
412412
arm_target_feature,
413413
array,
414+
as_mut_ptr,
414415
as_ptr,
415416
as_ref,
416417
as_str,

0 commit comments

Comments
 (0)