Skip to content

Commit 061196b

Browse files
burakemirburakemir
committed
unused but used
1 parent 4ef79dd commit 061196b

File tree

3 files changed

+104
-19
lines changed

3 files changed

+104
-19
lines changed

‎toolchain/check/dataflow_analysis.cpp‎

Lines changed: 59 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,6 @@
1818

1919
namespace Carbon::Check {
2020

21-
CARBON_DIAGNOSTIC(UnusedVariable, Warning, "variable `{0}` is unused",
22-
std::string);
23-
2421
// Represents a single fact with two IDs.
2522
// The meaning of id1 and id2 depends on the FactType.
2623
// Using int32_t to store the raw index values of the various Id types.
@@ -263,36 +260,79 @@ static auto BuildDataflowFacts(const SemIR::File& sem_ir,
263260
static auto CheckUnusedVariables(Context& context, const DataflowFacts& facts)
264261
-> void {
265262
auto& sem_ir = context.sem_ir();
266-
// Collect all used variable IDs (EntityNameId indices).
267-
Set<int32_t> used_vars;
268-
facts.uses.ForEach([&](const Fact& use) { used_vars.Insert(use.id2); });
269263

270-
// Collect unused definitions.
264+
// Collect usage locations. We track the first source-location use for each
265+
// variable.
266+
llvm::DenseMap<int32_t, SemIR::InstId> first_use;
267+
facts.uses.ForEach([&](const Fact& use) {
268+
auto [it, inserted] = first_use.insert({use.id2, SemIR::InstId(use.id1)});
269+
if (!inserted) {
270+
// Keep the earliest instruction ID.
271+
if (use.id1 < it->second.index) {
272+
it->second = SemIR::InstId(use.id1);
273+
}
274+
}
275+
});
276+
277+
// Collect definitions to diagnose.
271278
llvm::SmallVector<Fact> unused_defs;
279+
llvm::SmallVector<Fact> unused_but_used_defs;
280+
272281
facts.defs.ForEach([&](const Fact& def) {
273282
auto var_id = def.id2;
274-
if (!used_vars.Contains(var_id)) {
275-
unused_defs.push_back(def);
283+
auto entity_name_id = SemIR::EntityNameId(var_id);
284+
const auto& entity_name = sem_ir.entity_names().Get(entity_name_id);
285+
286+
if (first_use.find(var_id) == first_use.end()) {
287+
if (!entity_name.is_unused) {
288+
unused_defs.push_back(def);
289+
}
290+
} else {
291+
if (entity_name.is_unused) {
292+
unused_but_used_defs.push_back(def);
293+
}
276294
}
277295
});
278296

279297
// Sort by instruction ID (location).
280-
std::sort(unused_defs.begin(), unused_defs.end(),
281-
[](const Fact& a, const Fact& b) { return a.id1 < b.id1; });
298+
auto sort_facts = [](const Fact& a, const Fact& b) { return a.id1 < b.id1; };
299+
std::sort(unused_defs.begin(), unused_defs.end(), sort_facts);
300+
std::sort(unused_but_used_defs.begin(), unused_but_used_defs.end(),
301+
sort_facts);
282302

283303
// Emit diagnostics.
304+
for (const auto& def : unused_but_used_defs) {
305+
auto var_id = def.id2;
306+
auto entity_name_id = SemIR::EntityNameId(var_id);
307+
const auto& entity_name = sem_ir.entity_names().Get(entity_name_id);
308+
auto name_id = entity_name.name_id;
309+
llvm::StringRef name = sem_ir.names().GetFormatted(name_id);
310+
auto inst_id = SemIR::InstId(def.id1);
311+
auto loc_id = sem_ir.insts().GetCanonicalLocId(inst_id);
312+
CARBON_DIAGNOSTIC(UnusedButUsed, Error,
313+
"variable `{0}` is marked `unused` but is used",
314+
std::string);
315+
auto diag = context.emitter().Build(LocIdForDiagnostics(loc_id),
316+
UnusedButUsed, name.str());
317+
auto use_inst_id = first_use.find(var_id)->second;
318+
auto use_loc_id = sem_ir.insts().GetCanonicalLocId(use_inst_id);
319+
CARBON_DIAGNOSTIC(UnusedButUsedHere, Note, "usage is here");
320+
diag.Note(LocIdForDiagnostics(use_loc_id), UnusedButUsedHere);
321+
diag.Emit();
322+
}
323+
284324
for (const auto& def : unused_defs) {
285325
auto var_id = def.id2;
286326
auto entity_name_id = SemIR::EntityNameId(var_id);
287327
const auto& entity_name = sem_ir.entity_names().Get(entity_name_id);
288-
if (!entity_name.is_unused) {
289-
auto name_id = entity_name.name_id;
290-
llvm::StringRef name = sem_ir.names().GetFormatted(name_id);
291-
auto inst_id = SemIR::InstId(def.id1);
292-
auto loc_id = sem_ir.insts().GetCanonicalLocId(inst_id);
293-
context.emitter().Emit(LocIdForDiagnostics(loc_id), UnusedVariable,
294-
name.str());
295-
}
328+
auto name_id = entity_name.name_id;
329+
llvm::StringRef name = sem_ir.names().GetFormatted(name_id);
330+
auto inst_id = SemIR::InstId(def.id1);
331+
auto loc_id = sem_ir.insts().GetCanonicalLocId(inst_id);
332+
CARBON_DIAGNOSTIC(UnusedVariable, Warning, "variable `{0}` is unused",
333+
std::string);
334+
context.emitter().Emit(LocIdForDiagnostics(loc_id), UnusedVariable,
335+
name.str());
296336
}
297337
}
298338

‎toolchain/check/testdata/dataflow/unused_but_used.carbon‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
2+
// Exceptions. See /LICENSE for license information.
3+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
4+
//
5+
// INCLUDE-FILE: toolchain/testing/testdata/min_prelude/primitives.carbon
6+
//
7+
// AUTOUPDATE
8+
// TIP: To test this file alone, run:
9+
// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/dataflow/unused_but_used.carbon
10+
// TIP: To dump output, run:
11+
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/dataflow/unused_but_used.carbon
12+
13+
fn F() {
14+
// CHECK:STDERR: unused_but_used.carbon:[[@LINE+3]]:14: error: variable `x` is marked `unused` but is used [UnusedButUsed]
15+
// CHECK:STDERR: var unused x: i32 = 1;
16+
// CHECK:STDERR: ^
17+
var unused x: i32 = 1;
18+
// CHECK:STDERR: unused_but_used.carbon:[[@LINE+8]]:16: note: usage is here [UnusedButUsedHere]
19+
// CHECK:STDERR: var y: i32 = x;
20+
// CHECK:STDERR: ^
21+
// CHECK:STDERR:
22+
// CHECK:STDERR: unused_but_used.carbon:[[@LINE+4]]:7: warning: variable `y` is unused [UnusedVariable]
23+
// CHECK:STDERR: var y: i32 = x;
24+
// CHECK:STDERR: ^
25+
// CHECK:STDERR:
26+
var y: i32 = x;
27+
}
28+
29+
fn G() {
30+
// CHECK:STDERR: unused_but_used.carbon:[[@LINE+3]]:14: error: variable `x` is marked `unused` but is used [UnusedButUsed]
31+
// CHECK:STDERR: let unused x: i32 = 1;
32+
// CHECK:STDERR: ^
33+
let unused x: i32 = 1;
34+
// CHECK:STDERR: unused_but_used.carbon:[[@LINE+8]]:16: note: usage is here [UnusedButUsedHere]
35+
// CHECK:STDERR: let y: i32 = x;
36+
// CHECK:STDERR: ^
37+
// CHECK:STDERR:
38+
// CHECK:STDERR: unused_but_used.carbon:[[@LINE+4]]:7: warning: variable `y` is unused [UnusedVariable]
39+
// CHECK:STDERR: let y: i32 = x;
40+
// CHECK:STDERR: ^
41+
// CHECK:STDERR:
42+
let y: i32 = x;
43+
}

‎toolchain/diagnostics/diagnostic_kind.def‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,8 @@ CARBON_DIAGNOSTIC_KIND(GenericMissingExplicitParameters)
540540
CARBON_DIAGNOSTIC_KIND(TuplePatternSizeDoesntMatchLiteral)
541541

542542
// Dataflow diagnostics.
543+
CARBON_DIAGNOSTIC_KIND(UnusedButUsed)
544+
CARBON_DIAGNOSTIC_KIND(UnusedButUsedHere)
543545
CARBON_DIAGNOSTIC_KIND(UnusedParamInDeclaration)
544546
CARBON_DIAGNOSTIC_KIND(UnusedPatternNoBindings)
545547
CARBON_DIAGNOSTIC_KIND(UnusedVariable)

0 commit comments

Comments
 (0)