Skip to content

Commit 871ceb4

Browse files
committed
Fix clippy lints in validation_err.rs and codegen_cx/mod.rs
- Inline format args (uninlined_format_args) - Collapse nested if statements using let chains (collapsible_if) - Add backticks around SPIR-V terms in doc comments (doc_markdown) - Remove unused &self from format_multi_location_hint (unused_self) - Use map_or instead of map().unwrap_or() (map_unwrap_or)
1 parent 7867fde commit 871ceb4

2 files changed

Lines changed: 64 additions & 91 deletions

File tree

crates/rustc_codegen_spirv/src/codegen_cx/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ pub struct CodegenArgs {
313313

314314
pub run_spirv_opt: bool,
315315

316-
/// Preserve internal debug decorations (SrcLocDecoration, ZombieDecoration) in the output.
316+
/// Preserve internal debug decorations (`SrcLocDecoration`, `ZombieDecoration`) in the output.
317317
/// These are normally stripped after validation. Enable for debugging or testing.
318318
pub preserve_debug_decorations: bool,
319319

crates/rustc_codegen_spirv/src/validation_err.rs

Lines changed: 63 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,8 @@ impl<'a> ValidationErrorContext<'a> {
125125
let first_var_start_location = self.get_location_decoration(first_var_id);
126126
let first_var_type_info = self.get_variable_type_info(first_var_id);
127127

128-
let first_name_fallback = format!("%{}", first_var_id);
129-
let second_name_fallback = format!("%{}", second_var_id);
128+
let first_name_fallback = format!("%{first_var_id}");
129+
let second_name_fallback = format!("%{second_var_id}");
130130
let first_name_display = first_name.as_deref().unwrap_or(&first_name_fallback);
131131
let second_name_display = second_name.as_deref().unwrap_or(&second_name_fallback);
132132

@@ -135,42 +135,38 @@ impl<'a> ValidationErrorContext<'a> {
135135
let mut err = self.sess.dcx().struct_span_err(
136136
span2,
137137
format!(
138-
"{:?} variable `{}` at location {} conflicts with another variable",
139-
storage_class, second_name_display, location
138+
"{storage_class:?} variable `{second_name_display}` at location {location} conflicts with another variable"
140139
),
141140
);
142141

143142
err.span_note(
144143
span1,
145144
format!(
146-
"variable `{}` already uses location {} component {}",
147-
first_name_display, location, component
145+
"variable `{first_name_display}` already uses location {location} component {component}"
148146
),
149147
);
150148

151149
// Add hint if the first variable spans multiple locations
152-
if let Some(start_loc) = first_var_start_location {
153-
if start_loc != location {
154-
err.help(self.format_multi_location_hint(
155-
first_name_display,
156-
start_loc,
157-
first_var_type_info.as_ref(),
158-
));
159-
}
150+
if let Some(start_loc) = first_var_start_location
151+
&& start_loc != location
152+
{
153+
err.help(Self::format_multi_location_hint(
154+
first_name_display,
155+
start_loc,
156+
first_var_type_info.as_ref(),
157+
));
160158
}
161159

162160
err.note(format!("module `{}`", self.filename.display()));
163161
err.emit();
164162
} else {
165163
// Fall back to non-span error
166164
let mut err = self.sess.dcx().struct_err(format!(
167-
"{:?} variable `{}` at location {} conflicts with `{}`",
168-
storage_class, second_name_display, location, first_name_display
165+
"{storage_class:?} variable `{second_name_display}` at location {location} conflicts with `{first_name_display}`"
169166
));
170167
err.note(format!("module `{}`", self.filename.display()));
171168
err.note(format!(
172-
"variables `{}` and `{}` both use location {} component {}",
173-
first_name_display, second_name_display, location, component
169+
"variables `{first_name_display}` and `{second_name_display}` both use location {location} component {component}"
174170
));
175171
err.emit();
176172
}
@@ -179,12 +175,11 @@ impl<'a> ValidationErrorContext<'a> {
179175
fn emit_invalid_block_layout(&mut self, struct_type_id: u32, reason: &str) {
180176
let span = self.id_to_span(struct_type_id);
181177
let type_name = self.get_name(struct_type_id);
182-
let type_name_fallback = format!("%{}", struct_type_id);
178+
let type_name_fallback = format!("%{struct_type_id}");
183179
let type_name_display = type_name.as_deref().unwrap_or(&type_name_fallback);
184180

185181
let message = format!(
186-
"struct `{}` has invalid block layout: {}",
187-
type_name_display, reason
182+
"struct `{type_name_display}` has invalid block layout: {reason}"
188183
);
189184

190185
let mut err = if let Some(span) = span {
@@ -202,13 +197,11 @@ impl<'a> ValidationErrorContext<'a> {
202197

203198
fn emit_invalid_builtin_type(&self, builtin: BuiltIn, expected: &str) {
204199
let mut err = self.sess.dcx().struct_err(format!(
205-
"BuiltIn {:?} has incorrect type, expected {}",
206-
builtin, expected
200+
"BuiltIn {builtin:?} has incorrect type, expected {expected}"
207201
));
208202

209203
err.help(format!(
210-
"variables with `#[spirv(builtin = \"{:?}\")]` must have the correct type",
211-
builtin
204+
"variables with `#[spirv(builtin = \"{builtin:?}\")]` must have the correct type"
212205
));
213206
err.note(format!("module `{}`", self.filename.display()));
214207
err.emit();
@@ -217,12 +210,11 @@ impl<'a> ValidationErrorContext<'a> {
217210
fn emit_missing_decoration(&mut self, var_id: u32, decoration_name: &str, hint: &str) {
218211
let span = self.id_to_span(var_id);
219212
let var_name = self.get_name(var_id);
220-
let var_name_fallback = format!("%{}", var_id);
213+
let var_name_fallback = format!("%{var_id}");
221214
let var_name_display = var_name.as_deref().unwrap_or(&var_name_fallback);
222215

223216
let message = format!(
224-
"resource variable `{}` is missing {} decoration",
225-
var_name_display, decoration_name
217+
"resource variable `{var_name_display}` is missing {decoration_name} decoration"
226218
);
227219

228220
let mut err = if let Some(span) = span {
@@ -231,7 +223,7 @@ impl<'a> ValidationErrorContext<'a> {
231223
self.sess.dcx().struct_err(message)
232224
};
233225

234-
err.help(format!("add `{}` to the variable", hint));
226+
err.help(format!("add `{hint}` to the variable"));
235227
err.note(format!("module `{}`", self.filename.display()));
236228
err.emit();
237229
}
@@ -247,11 +239,10 @@ impl<'a> ValidationErrorContext<'a> {
247239

248240
let message = if let Some(idx) = operand_index {
249241
format!(
250-
"operand {} of Op{:?} requires capability {:?}",
251-
idx, opcode, capability
242+
"operand {idx} of Op{opcode:?} requires capability {capability:?}"
252243
)
253244
} else {
254-
format!("Op{:?} requires capability {:?}", opcode, capability)
245+
format!("Op{opcode:?} requires capability {capability:?}")
255246
};
256247

257248
let mut err = if let Some(span) = span {
@@ -261,8 +252,7 @@ impl<'a> ValidationErrorContext<'a> {
261252
};
262253

263254
err.help(format!(
264-
"add `#[spirv(capability({:?}))]` to your entry point function",
265-
capability
255+
"add `#[spirv(capability({capability:?}))]` to your entry point function"
266256
));
267257
err.note(format!("module `{}`", self.filename.display()));
268258
err.emit();
@@ -278,15 +268,14 @@ impl<'a> ValidationErrorContext<'a> {
278268
});
279269

280270
let pointer_name = self.get_name(pointer_id);
281-
let pointer_name_fallback = format!("%{}", pointer_id);
271+
let pointer_name_fallback = format!("%{pointer_id}");
282272
let pointer_name_display = pointer_name.as_deref().unwrap_or(&pointer_name_fallback);
283273

284274
// Get SPIR-V context showing the relevant instructions
285275
let spirv_context = self.get_spirv_context_for_pointer(pointer_id, source_opcode);
286276

287277
let message = format!(
288-
"Op{:?} cannot use pointer `{}` because it was produced by Op{:?}",
289-
instruction, pointer_name_display, source_opcode
278+
"Op{instruction:?} cannot use pointer `{pointer_name_display}` because it was produced by Op{source_opcode:?}"
290279
);
291280

292281
let mut err = if let Some(span) = span {
@@ -296,18 +285,16 @@ impl<'a> ValidationErrorContext<'a> {
296285
};
297286

298287
err.note(format!(
299-
"in SPIR-V's logical addressing mode, pointers for Op{:?} must come from \
300-
specific instructions like OpVariable, OpAccessChain, or OpFunctionParameter",
301-
instruction
288+
"in SPIR-V's logical addressing mode, pointers for Op{instruction:?} must come from \
289+
specific instructions like OpVariable, OpAccessChain, or OpFunctionParameter"
302290
));
303291
err.help(format!(
304-
"Op{:?} cannot produce pointers valid for memory operations in logical addressing",
305-
source_opcode
292+
"Op{source_opcode:?} cannot produce pointers valid for memory operations in logical addressing"
306293
));
307294

308295
// Show SPIR-V context if available
309296
if let Some(context) = spirv_context {
310-
err.note(format!("generated SPIR-V:\n{}", context));
297+
err.note(format!("generated SPIR-V:\n{context}"));
311298
}
312299

313300
err.note(format!("module `{}`", self.filename.display()));
@@ -322,25 +309,23 @@ impl<'a> ValidationErrorContext<'a> {
322309
for func in &m.functions {
323310
for block in &func.blocks {
324311
for inst in &block.instructions {
325-
if inst.class.opcode == opcode {
326-
if let Some(id) = inst.result_id {
327-
if let Some(span) = self.id_to_span(id) {
328-
return Some(span);
329-
}
330-
}
312+
if inst.class.opcode == opcode
313+
&& let Some(id) = inst.result_id
314+
&& let Some(span) = self.id_to_span(id)
315+
{
316+
return Some(span);
331317
}
332318
}
333319
}
334320
}
335321

336322
// Search in types/globals
337323
for inst in &m.types_global_values {
338-
if inst.class.opcode == opcode {
339-
if let Some(id) = inst.result_id {
340-
if let Some(span) = self.id_to_span(id) {
341-
return Some(span);
342-
}
343-
}
324+
if inst.class.opcode == opcode
325+
&& let Some(id) = inst.result_id
326+
&& let Some(span) = self.id_to_span(id)
327+
{
328+
return Some(span);
344329
}
345330
}
346331

@@ -356,10 +341,10 @@ impl<'a> ValidationErrorContext<'a> {
356341
if inst.class.opcode == opcode {
357342
// Check if any operand references our pointer
358343
for operand in &inst.operands {
359-
if let rspirv::dr::Operand::IdRef(id) = operand {
360-
if *id == pointer_id {
361-
return inst.result_id;
362-
}
344+
if let rspirv::dr::Operand::IdRef(id) = operand
345+
&& *id == pointer_id
346+
{
347+
return inst.result_id;
363348
}
364349
}
365350
}
@@ -382,24 +367,23 @@ impl<'a> ValidationErrorContext<'a> {
382367
// Found the instruction that produced the pointer
383368
if inst.result_id == Some(pointer_id) && inst.class.opcode == source_opcode {
384369
context_lines.push(format!(
385-
" %{} = Op{:?} ...",
386-
pointer_id, source_opcode
370+
" %{pointer_id} = Op{source_opcode:?} ..."
387371
));
388372
}
389373
// Found instructions using the pointer
390374
if matches!(inst.class.opcode, Op::Load | Op::Store) {
391375
for operand in &inst.operands {
392-
if let rspirv::dr::Operand::IdRef(id) = operand {
393-
if *id == pointer_id {
394-
let result = inst
395-
.result_id
396-
.map(|r| format!("%{} = ", r))
397-
.unwrap_or_default();
398-
context_lines.push(format!(
399-
" -> {}Op{:?} %{} ...",
400-
result, inst.class.opcode, pointer_id
401-
));
402-
}
376+
if let rspirv::dr::Operand::IdRef(id) = operand
377+
&& *id == pointer_id
378+
{
379+
let result = inst
380+
.result_id
381+
.map(|r| format!("%{r} = "))
382+
.unwrap_or_default();
383+
context_lines.push(format!(
384+
" -> {result}Op{:?} %{pointer_id} ...",
385+
inst.class.opcode
386+
));
403387
}
404388
}
405389
}
@@ -422,7 +406,7 @@ impl<'a> ValidationErrorContext<'a> {
422406
})
423407
}
424408

425-
/// Gets the name of a SPIR-V ID from OpName.
409+
/// Gets the name of a SPIR-V ID from `OpName`.
426410
fn get_name(&self, id: u32) -> Option<String> {
427411
self.module.and_then(|m| {
428412
m.debug_names.iter().find_map(|inst| {
@@ -534,47 +518,36 @@ impl<'a> ValidationErrorContext<'a> {
534518
}
535519

536520
fn format_multi_location_hint(
537-
&self,
538521
var_name: &str,
539522
start_loc: u32,
540523
type_info: Option<&TypeInfo>,
541524
) -> String {
542525
let (type_name, location_count) = type_info
543-
.map(|ti| (ti.name.as_deref(), ti.location_count))
544-
.unwrap_or((None, None));
526+
.map_or((None, None), |ti| (ti.name.as_deref(), ti.location_count));
545527

546528
match (type_name, location_count) {
547529
(Some(name), Some(count)) => {
548530
format!(
549-
"`{}` is at location {} but type `{}` consumes {} locations ({}–{})",
550-
var_name,
551-
start_loc,
552-
name,
553-
count,
531+
"`{var_name}` is at location {start_loc} but type `{name}` consumes {count} locations ({}–{})",
554532
start_loc,
555533
start_loc + count - 1
556534
)
557535
}
558536
(Some(name), None) => {
559537
format!(
560-
"`{}` is at location {} but type `{}` consumes multiple locations",
561-
var_name, start_loc, name
538+
"`{var_name}` is at location {start_loc} but type `{name}` consumes multiple locations"
562539
)
563540
}
564541
(None, Some(count)) => {
565542
format!(
566-
"`{}` is at location {} but its type consumes {} locations ({}–{})",
567-
var_name,
568-
start_loc,
569-
count,
543+
"`{var_name}` is at location {start_loc} but its type consumes {count} locations ({}–{})",
570544
start_loc,
571545
start_loc + count - 1
572546
)
573547
}
574548
(None, None) => {
575549
format!(
576-
"`{}` is at location {} but its type consumes multiple locations",
577-
var_name, start_loc
550+
"`{var_name}` is at location {start_loc} but its type consumes multiple locations"
578551
)
579552
}
580553
}
@@ -583,7 +556,7 @@ impl<'a> ValidationErrorContext<'a> {
583556

584557
/// Information about a SPIR-V type for error reporting.
585558
struct TypeInfo {
586-
/// The type name from OpName, if available.
559+
/// The type name from `OpName`, if available.
587560
name: Option<String>,
588561
/// The number of interface locations consumed by this type.
589562
location_count: Option<u32>,

0 commit comments

Comments
 (0)