carbon-language/carbon-lang
1
0
Fork 0
mirror of https://github.com/carbon-language/carbon-lang.git synced 2026-10-04 21:31:04 +01:00
Code Issues Packages Projects Releases 10 Wiki Activity

Apply the unused-without-definition check to implicit parameters (#7270)

Browse Source 
The `unused` modifier is rejected on parameters of a function
declaration, but the check only covered the explicit parameter list, so
an implicit parameter (such as self or a compile-time binding) could
carry unused without a definition. Check the implicit parameter list
too.

The code changes and the test updates are split into two commits for
easier review.

Assisted-by: Claude Code with Claude Opus 4.7

---------

Co-authored-by: Christopher Di Bella <cjdb.ns@gmail.com>
This commit is contained in:
Chandler Carruth
2026-05-30 08:26:09 +00:00
committed by GitHub
co-authored by Christopher Di Bella
parent 8a59f2a76b
commit 14a213d095
8 changed files with 61 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
toolchain/check/handle_function.cpp
+19 -14
View File
@@ -609,8 +609,8 @@ static auto BuildFunctionDecl(Context& context,
return {function_decl.function_id, decl_id};
}
// Checks that "unused" marker is only used in definitions, and emits a
// diagnostic for every binding that is marked unused.
// Checks that the `unused` modifier is only used when there is a definition,
// and emits a diagnostic for every binding that is marked `unused`.
static auto CheckUnusedBindingsInPattern(Context& context,
SemIR::InstId pattern_id) -> void {
llvm::SmallVector<SemIR::InstId> work_list;
@@ -626,12 +626,12 @@ static auto CheckUnusedBindingsInPattern(Context& context,
case CARBON_KIND_ANY(SemIR::AnyBindingPattern, bind): {
auto& entity_name = context.entity_names().Get(bind.entity_name_id);
// We need special treatment for the name "_" which is implicitly
// unused but actually permitted in declarations.
// unused but actually permitted without a definition.
if (entity_name.is_unused &&
entity_name.name_id != SemIR::NameId::Underscore) {
CARBON_DIAGNOSTIC(UnusedModifierOnDeclaration, Error,
"`unused` modifier on declaration");
context.emitter().Emit(current_id, UnusedModifierOnDeclaration);
CARBON_DIAGNOSTIC(UnusedModifierWithoutDefinition, Error,
"`unused` modifier without a definition");
context.emitter().Emit(current_id, UnusedModifierWithoutDefinition);
}
if (bind.kind == SemIR::WrapperBindingPattern::Kind) {
work_list.push_back(bind.subpattern_id);
@@ -655,14 +655,19 @@ static auto CheckUnusedBindingsInPattern(Context& context,
}
}
static auto DiagnoseUnusedMarkersInDeclaration(Context& context,
SemIR::FunctionId function_id)
-> void {
static auto DiagnoseUnusedMarkersWithoutDefinition(
Context& context, SemIR::FunctionId function_id) -> void {
const auto& function = context.functions().Get(function_id);
if (function.param_patterns_id.has_value()) {
for (auto pattern_id :
context.inst_blocks().Get(function.param_patterns_id)) {
CheckUnusedBindingsInPattern(context, pattern_id);
// The `unused` modifier requires a definition, so it is not valid on any
// parameter when there is none. This applies to implicit parameters (such as
// `self`) too, so check the implicit parameter list as well as the explicit
// one.
for (auto param_patterns_id :
{function.implicit_param_patterns_id, function.param_patterns_id}) {
if (param_patterns_id.has_value()) {
for (auto pattern_id : context.inst_blocks().Get(param_patterns_id)) {
CheckUnusedBindingsInPattern(context, pattern_id);
}
}
}
}
@@ -670,7 +675,7 @@ static auto DiagnoseUnusedMarkersInDeclaration(Context& context,
auto HandleParseNode(Context& context, Parse::FunctionDeclId node_id) -> bool {
auto [function_id, decl_id] =
BuildFunctionDecl(context, node_id, /*is_definition=*/false);
DiagnoseUnusedMarkersInDeclaration(context, function_id);
DiagnoseUnusedMarkersWithoutDefinition(context, function_id);
context.decl_name_stack().PopScope();
return true;
}