https://github.com/vabridgers updated https://github.com/llvm/llvm-project/pull/175429
>From 64a84343c35e572138a8e69c28ed1470b2cd00bb Mon Sep 17 00:00:00 2001 From: Vince Bridgers <[email protected]> Date: Sun, 11 Jan 2026 12:21:43 +0100 Subject: [PATCH 1/3] [clang-tidy] Add new check 'misc-scope-reduction' Adds a new clang-tidy check that does a scope reduction analysis, supporting SEI DCL19-C, MISRA C++:2008 Rule 3-4-1, and MISRA +C:2012 Rule 8-9. --- .../clang-tidy/misc/CMakeLists.txt | 1 + .../clang-tidy/misc/MiscTidyModule.cpp | 2 + .../clang-tidy/misc/ScopeReductionCheck.cpp | 282 ++++++++++++++++++ .../clang-tidy/misc/ScopeReductionCheck.h | 28 ++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../checks/misc/scope-reduction.rst | 50 ++++ .../checkers/misc/scope-reduction.cpp | 181 +++++++++++ 8 files changed, 550 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/misc/scope-reduction.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/misc/scope-reduction.cpp diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt index e34b0cf687be3..c1c3dd656b811 100644 --- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt @@ -37,6 +37,7 @@ add_clang_library(clangTidyMiscModule STATIC OverrideWithDifferentVisibilityCheck.cpp PredictableRandCheck.cpp RedundantExpressionCheck.cpp + ScopeReductionCheck.cpp StaticAssertCheck.cpp ThrowByValueCatchByReferenceCheck.cpp UnconventionalAssignOperatorCheck.cpp diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp index f8550b30b9789..899c27033ba3b 100644 --- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp @@ -26,6 +26,7 @@ #include "OverrideWithDifferentVisibilityCheck.h" #include "PredictableRandCheck.h" #include "RedundantExpressionCheck.h" +#include "ScopeReductionCheck.h" #include "StaticAssertCheck.h" #include "ThrowByValueCatchByReferenceCheck.h" #include "UnconventionalAssignOperatorCheck.h" @@ -75,6 +76,7 @@ class MiscModule : public ClangTidyModule { CheckFactories.registerCheck<PredictableRandCheck>("misc-predictable-rand"); CheckFactories.registerCheck<RedundantExpressionCheck>( "misc-redundant-expression"); + CheckFactories.registerCheck<ScopeReductionCheck>("misc-scope-reduction"); CheckFactories.registerCheck<StaticAssertCheck>("misc-static-assert"); CheckFactories.registerCheck<ThrowByValueCatchByReferenceCheck>( "misc-throw-by-value-catch-by-reference"); diff --git a/clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.cpp b/clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.cpp new file mode 100644 index 0000000000000..51fb0598fa765 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.cpp @@ -0,0 +1,282 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// This checker uses a 7-step algorithm to accomplish scope analysis of a +// variable and determine if it's in too large a scope. Note that the +// clang-tidy framework is aimed mainly at supporting text-manipulation, +// diagnostics, or common AST patterns. Scope reduction analysis is +// quite specialized, and there's not much support specifically for +// those steps. Perhaps someone else knows better and can help simplify +// this code in a more concrete way other than simply suggesting it can +// be simpler. +// +// The 7-step algorithm used by this checker for scope reduction analysis is: +// 1) Filter out variables declared in for-loop initializations +// - Those variables are already in optimal scope, and can be skipped +// 2) Collect variable uses +// - find all DeclRefExpr nodes that reference the variable +// 3) Build scope chains +// - for each use, find all compound statements that contain it (from +// innermost to outermost) +// 4) Find the innermost compound statement that contains all uses +// - This is the smallest scope where the variable could be declared +// 5) Find declaration scope +// - Locate the compound statement containing the variable declaration +// 6) Verify nesting +// - Ensure the usage scope is actually nested within the declaration scope +// 7) Alternate analysis - check for for-loop initialization opportunity +// - This is only run if compound stmt analysis didn't find smaller scope +// - Only check local variables, not parameters +// - Determine if all uses are within the same for-loop and suggest +// for-loop initialization +// +// The algo works by finding the smallest scope that could contain the variable +// declaration while still encompassing all it's uses. + +#include "ScopeReductionCheck.h" +#include "../utils/ASTUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::misc { + +static void +collectVariableUses(const Stmt *S, const VarDecl *Var, + llvm::SmallVector<const DeclRefExpr *, 8> &Uses) { + if (!S) + return; + + if (const auto *DRE = dyn_cast<DeclRefExpr>(S)) { + if (DRE->getDecl() == Var) + Uses.push_back(DRE); + } + + for (const Stmt *Child : S->children()) + collectVariableUses(Child, Var, Uses); +} + +void ScopeReductionCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(varDecl(hasLocalStorage()).bind("var"), this); +} + +void ScopeReductionCheck::check( + const ast_matchers::MatchFinder::MatchResult &Result) { + const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var"); + if (!Var) + return; + + // Step 1: Filter out variables declared in for-loop initializations + // These variables are already in their optimal scope and shouldn't be + // analyzed + auto &Parents = Result.Context->getParentMapContext(); + auto ParentNodes = Parents.getParents(DynTypedNode::create(*Var)); + + if (!ParentNodes.empty()) { + if (const auto *Parent = ParentNodes[0].get<Stmt>()) { + if (isa<DeclStmt>(Parent)) { + // Check if DeclStmt's parent is ForStmt + auto GrandParentNodes = Parents.getParents(*Parent); + if (!GrandParentNodes.empty()) { + if (const auto *GrandParent = GrandParentNodes[0].get<Stmt>()) { + if (isa<ForStmt>(GrandParent)) + return; // Skip for-loop declared variables + } + } + } + } + } + + // auto *Context = Result.Context; + auto *Function = dyn_cast<FunctionDecl>(Var->getDeclContext()); + if (!Function || !Function->hasBody()) + return; + + // Step 2: Collect all uses of this variable within the function + llvm::SmallVector<const DeclRefExpr *, 8> Uses; + collectVariableUses(Function->getBody(), Var, Uses); + + // No uses, return with no diagnostics + if (Uses.empty()) + return; + + // Step 3: For each variable use, find all compound statements that contain it + // This builds a "scope chain" from innermost to outermost for each use + const CompoundStmt *InnermostScope = nullptr; + + // For each use, find all compound statements that contain it + llvm::SmallVector<llvm::SmallVector<const CompoundStmt *, 4>, 8> + UseScopeChains; + + for (const auto *Use : Uses) { + llvm::SmallVector<const CompoundStmt *, 4> ScopeChain; + const Stmt *Current = Use; + + // Walk up the AST from this use to fins all containing compound stmts + while (Current) { + auto ParentNodes = Parents.getParents(*Current); + if (ParentNodes.empty()) + break; + + const Stmt *Parent = ParentNodes[0].get<Stmt>(); + if (!Parent) { + // Try to get Decl parent and continue from there + if (const auto *DeclParent = ParentNodes[0].get<Decl>()) { + auto DeclParentNodes = Parents.getParents(*DeclParent); + if (!DeclParentNodes.empty()) + Parent = DeclParentNodes[0].get<Stmt>(); + } + if (!Parent) + break; + } + + if (const auto *CS = dyn_cast<CompoundStmt>(Parent)) + ScopeChain.push_back(CS); + + Current = Parent; + } + + if (!ScopeChain.empty()) + UseScopeChains.push_back(ScopeChain); + } + + // Step 4: Find the innermost scope that contains all uses + // This is the smallest scope where var could be declared + if (!UseScopeChains.empty()) { + // Start with first use's innermost scope + InnermostScope = UseScopeChains[0][0]; + + // For each subsequent use, find common ancestor scope + for (size_t i = 1; i < UseScopeChains.size(); ++i) { + const CompoundStmt *CommonScope = nullptr; + + // Find first scope that appears in both chains (common ancestor) + for (const auto *Scope1 : UseScopeChains[0]) { + for (const auto *Scope2 : UseScopeChains[i]) { + if (Scope1 == Scope2) { + CommonScope = Scope1; + break; + } + } + if (CommonScope) + break; + } + + if (CommonScope) + InnermostScope = CommonScope; + } + } + + // Step 5: Check if current var declaration is broader than necessary + if (InnermostScope) { + // Find the compound statement containing the variable declaration + const DynTypedNode Current = DynTypedNode::create(*Var); + const CompoundStmt *VarScope = nullptr; + + auto ParentNodes = Parents.getParents(Current); + while (!ParentNodes.empty()) { + const Stmt *Parent = ParentNodes[0].get<Stmt>(); + if (!Parent) + break; + + if (const auto *CS = dyn_cast<CompoundStmt>(Parent)) { + VarScope = CS; + break; + } + ParentNodes = Parents.getParents(*Parent); + } + + // Step 6: Verify that usage scope is nested within decl scope + if (VarScope && VarScope != InnermostScope) { + // Walk up from innermost usage to see if the decl scope is reached + const Stmt *CheckScope = InnermostScope; + bool IsNested = false; + + while (CheckScope) { + auto CheckParents = Parents.getParents(*CheckScope); + if (CheckParents.empty()) + break; + + const Stmt *CheckParent = CheckParents[0].get<Stmt>(); + if (CheckParent == VarScope) { + IsNested = true; + break; + } + CheckScope = CheckParent; + } + + // Only report if the usage scope is truly nested within the decl scope + if (IsNested) { + diag(Var->getLocation(), + "variable '%0' can be declared in a smaller scope") + << Var->getName(); + return; // early exit + } + } + } + + // Step 7: Alternative analysis - check for for-loop initialization + // opportunity This only runs if the compound statement analysis didn't find a + // smaller scope Only check local variables, not parameters + if (!isa<ParmVarDecl>(Var)) { + const ForStmt *CommonForLoop = nullptr; + bool AllUsesInSameForLoop = true; + + for (const auto *Use : Uses) { + const ForStmt *ContainingForLoop = nullptr; + const Stmt *Current = Use; + + // Walk up the AST to find a containing ForStmt + while (Current) { + auto ParentNodes = Parents.getParents(*Current); + if (ParentNodes.empty()) + break; + + if (const auto *FS = ParentNodes[0].get<ForStmt>()) { + ContainingForLoop = FS; + break; + } + + const Stmt *Parent = ParentNodes[0].get<Stmt>(); + if (!Parent) { + // Handle Decl parents like we do in the existing logic + if (const auto *DeclParent = ParentNodes[0].get<Decl>()) { + auto DeclParentNodes = Parents.getParents(*DeclParent); + if (!DeclParentNodes.empty()) + Parent = DeclParentNodes[0].get<Stmt>(); + } + if (!Parent) + break; + } + Current = Parent; + } + + if (!ContainingForLoop) { + AllUsesInSameForLoop = false; + break; + } + + if (!CommonForLoop) { + CommonForLoop = ContainingForLoop; + } else if (CommonForLoop != ContainingForLoop) { + AllUsesInSameForLoop = false; + break; + } + } + + if (AllUsesInSameForLoop && CommonForLoop) { + diag(Var->getLocation(), + "variable '%0' can be declared in for-loop initialization") + << Var->getName(); + return; + } + } +} + +} // namespace clang::tidy::misc diff --git a/clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.h b/clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.h new file mode 100644 index 0000000000000..690f58feb93f7 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.h @@ -0,0 +1,28 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SCOPEREDUCTIONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SCOPEREDCUTIONCHECK_H + +#include "../../clang-tidy/utils/DeclRefExprUtils.h" +#include "../ClangTidyCheck.h" +#include "clang/AST/ASTContext.h" + +namespace clang::tidy::misc { + +class ScopeReductionCheck : public ClangTidyCheck { +public: + ScopeReductionCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace clang::tidy::misc + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SCOPEREDCUTIONCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 7747c5d0e96a7..2566b4f5ca730 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -294,6 +294,11 @@ New checks Finds virtual function overrides with different visibility than the function in the base class. +- New :doc:`misc-scope-reduction + <clang-tidy/checks/misc/scope-reduction>` check. + + Checks for opportunities to minimize scope of local variables. + - New :doc:`readability-inconsistent-ifelse-braces <clang-tidy/checks/readability/inconsistent-ifelse-braces>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 3dabf887dc2e1..248f237d9c318 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -280,6 +280,7 @@ Clang-Tidy Checks :doc:`misc-override-with-different-visibility <misc/override-with-different-visibility>`, :doc:`misc-predictable-rand <misc/predictable-rand>`, :doc:`misc-redundant-expression <misc/redundant-expression>`, "Yes" + :doc:`misc-scope-reduction <misc/scope-reduction>`, :doc:`misc-static-assert <misc/static-assert>`, "Yes" :doc:`misc-throw-by-value-catch-by-reference <misc/throw-by-value-catch-by-reference>`, :doc:`misc-unconventional-assign-operator <misc/unconventional-assign-operator>`, diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/scope-reduction.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/scope-reduction.rst new file mode 100644 index 0000000000000..11a85f406f5bc --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/misc/scope-reduction.rst @@ -0,0 +1,50 @@ +.. title:: clang-tidy - misc-scope-reduction + +misc-scope-reduction +=========================== + +Detects local variables in functions whose scopes can be minimized. This check +covers guidelines described by SEI DCL19-C, MISRA C++:2008 Rule 3-4-1, and MISRA +C:2012 Rule 8-9. + +Examples: + +.. code-block:: cpp + + void test_deep_nesting() { + int deep = 1; // 'deep' can be declared in a smaller scope + if (true) { + if (true) { + if (true) { + if (true) { + int result = deep * 4; + } + } + } + } + } + + void test_switch_multiple_cases(int value) { + int accumulator = 0; // 'accumulator' can be declared in a smaller scope + switch (value) { + case 1: + accumulator += 10; + break; + case 2: + accumulator += 20; + break; + } + } + + void test_for_loop_expressions() { + int i; // 'i' can be declared in the for-loop initialization + for (i = 0; i < 5; i++) { + // loop body + } + } + +References +---------- +This check corresponds to the CERT C Coding Standard rules +`DCL19-C. Minimize the scope of variables and functions +<https://wiki.sei.cmu.edu/confluence/spaces/c/pages/87152335/DCL19-C.+Minimize+the+scope+of+variables+and+functions>`_. diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/scope-reduction.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/scope-reduction.cpp new file mode 100644 index 0000000000000..ba9e0e9d3c466 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/scope-reduction.cpp @@ -0,0 +1,181 @@ +// RUN: %check_clang_tidy %s misc-scope-reduction %t -- -- + +// Test case 1: Variable can be moved to smaller scope (if-block) +void test_if_scope() { + int x = 42; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'x' can be declared in a smaller scope + if (true) { + int y = x + 1; + } +} + +// Test case 2: Variable used across multiple scopes - should NOT warn +int test_multiple_scopes(int v) { + int y = 0; // Should NOT warn - used in if-block and return + if (v) { + y = 10; + } + return y; +} + +// Test case 3: Variable can be moved to nested if-block +void test_nested_if() { + int a = 5; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'a' can be declared in a smaller scope + if (true) { + if (true) { + int b = a * 2; + } + } +} + +// Test case 4: Variable used in same scope - should NOT warn +void test_same_scope() { + int x = 10; // Should NOT warn - used in same scope + int y = x + 5; +} + +// Test case 5: Variable can be moved to while loop body +void test_while_loop() { + int counter = 0; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'counter' can be declared in a smaller scope + while (true) { + counter++; + if (counter > 10) break; + } +} + +// Test case 6: Variable used in multiple branches of same if-statement +void test_if_branches(bool condition) { + int value = 100; // Should NOT warn - used in both branches + if (condition) { + value *= 2; + } else { + value /= 2; + } +} + +// Test case 7: Variable can be moved to for-loop body +void test_for_loop_body() { + int temp = 0; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'temp' can be declared in a smaller scope + for (int i = 0; i < 10; i++) { + temp = i * i; + } +} + +// Test case 8: Variable used in for-loop expressions - should NOT warn (current limitation) +void test_for_loop_expressions() { + int i; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'i' can be declared in for-loop initialization + for (i = 0; i < 5; i++) { + // loop body + } +} + +// Test case 9: Variable can be moved to switch case +void test_switch_case(int value) { + int result = 0; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'result' can be declared in a smaller scope + switch (value) { + case 1: + result = 10; + break; + default: + break; + } +} + +// Test case 10: Variable used across multiple switch cases - should NOT warn +void test_switch_multiple_cases(int value) { + int accumulator = 0; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'accumulator' can be declared in a smaller scope + switch (value) { + case 1: + accumulator += 10; + break; + case 2: + accumulator += 20; + break; + } +} + +// Test case 11: Variable with complex initialization can be moved +void test_complex_init() { + int complex = (5 + 3) * 2; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'complex' can be declared in a smaller scope + if (true) { + int doubled = complex * 2; + } +} + +// Test case 12: Multiple variables, some can be moved, some cannot +int test_mixed_variables(bool flag) { + int movable = 10; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'movable' can be declared in a smaller scope + int unmovable = 20; // Should NOT warn - used across scopes + + if (flag) { + int local = movable + 5; + unmovable += 1; + } + + return unmovable; +} + +// Test case 13: Variable in try-catch block +void test_try_catch() { + int error_code = 0; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'error_code' can be declared in a smaller scope + try { + error_code = 404; + } catch (...) { + // handle exception + } +} + +// Test case 14: Variable used in catch block and try block - should NOT warn +void test_try_catch_shared() { + int shared = 0; // Should NOT warn - used in both try and catch + try { + shared = 100; + } catch (...) { + shared = -1; + } +} + +// Test case 15: Deeply nested scopes +void test_deep_nesting() { + int deep = 1; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'deep' can be declared in a smaller scope + if (true) { + if (true) { + if (true) { + if (true) { + int result = deep * 4; + } + } + } + } +} + +// Test case 16: Variable declared but never used - should NOT warn (different checker's job) +void test_unused_variable() { + int unused = 42; // Should NOT warn - this checker only handles scope reduction +} + +// Test case 17: Global variable - should NOT be processed +int global_var = 100; + +// Test case 18: Static local variable - should NOT warn +void test_static_variable() { + static int static_var = 0; // Should NOT warn - static variables have different semantics + if (true) { + static_var++; + } +} + +// Test case 19: Function parameter - should NOT be processed +void test_parameter(int param) { + if (true) { + int local = param + 1; + } +} + +// Test case 20: Variable used in lambda - should NOT warn (complex case) +void test_lambda() { + int captured = 10; // Should NOT warn - used in lambda + auto lambda = [&]() { + return captured * 2; + }; + lambda(); +} >From 69347a5cbf9d5dc15d500c8e495e997a739118c8 Mon Sep 17 00:00:00 2001 From: Vince Bridgers <[email protected]> Date: Sun, 11 Jan 2026 15:20:50 +0100 Subject: [PATCH 2/3] First round of code review comments * Address CI failures seen initial PR * Move all CHECK-MESSAGES to line below findings * Correct documentation format issues in rst files * Add TODO for false positive in LIT * Remove unnecessary headers, use more llvm idiom to iterate SmallVector * Annotate code with TODOs from code review (helps me) --- .../clang-tidy/misc/ScopeReductionCheck.cpp | 9 ++--- .../clang-tidy/misc/ScopeReductionCheck.h | 1 - .../checks/misc/scope-reduction.rst | 3 +- .../checkers/misc/scope-reduction.cpp | 35 +++++++++++++------ 4 files changed, 31 insertions(+), 17 deletions(-) diff --git a/clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.cpp b/clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.cpp index 51fb0598fa765..5034a426c43e3 100644 --- a/clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.cpp @@ -39,14 +39,13 @@ // declaration while still encompassing all it's uses. #include "ScopeReductionCheck.h" -#include "../utils/ASTUtils.h" -#include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" using namespace clang::ast_matchers; namespace clang::tidy::misc { +// TODO: Try using utils::decl_ref_expr::allDeclRefExprs here. static void collectVariableUses(const Stmt *S, const VarDecl *Var, llvm::SmallVector<const DeclRefExpr *, 8> &Uses) { @@ -63,6 +62,8 @@ collectVariableUses(const Stmt *S, const VarDecl *Var, } void ScopeReductionCheck::registerMatchers(MatchFinder *Finder) { + // TODO: Try adding unless(hasParent(declStmt(hasParent(forStmt( to matcher + // to simplify check code. Finder->addMatcher(varDecl(hasLocalStorage()).bind("var"), this); } @@ -153,12 +154,12 @@ void ScopeReductionCheck::check( InnermostScope = UseScopeChains[0][0]; // For each subsequent use, find common ancestor scope - for (size_t i = 1; i < UseScopeChains.size(); ++i) { + for (const auto &ScopeChain : llvm::drop_begin(UseScopeChains)) { const CompoundStmt *CommonScope = nullptr; // Find first scope that appears in both chains (common ancestor) for (const auto *Scope1 : UseScopeChains[0]) { - for (const auto *Scope2 : UseScopeChains[i]) { + for (const auto *Scope2 : ScopeChain) { if (Scope1 == Scope2) { CommonScope = Scope1; break; diff --git a/clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.h b/clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.h index 690f58feb93f7..ef3c898f9c95f 100644 --- a/clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.h +++ b/clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.h @@ -11,7 +11,6 @@ #include "../../clang-tidy/utils/DeclRefExprUtils.h" #include "../ClangTidyCheck.h" -#include "clang/AST/ASTContext.h" namespace clang::tidy::misc { diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/scope-reduction.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/scope-reduction.rst index 11a85f406f5bc..5c8090fe7c274 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/misc/scope-reduction.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/misc/scope-reduction.rst @@ -1,7 +1,7 @@ .. title:: clang-tidy - misc-scope-reduction misc-scope-reduction -=========================== +==================== Detects local variables in functions whose scopes can be minimized. This check covers guidelines described by SEI DCL19-C, MISRA C++:2008 Rule 3-4-1, and MISRA @@ -45,6 +45,7 @@ Examples: References ---------- + This check corresponds to the CERT C Coding Standard rules `DCL19-C. Minimize the scope of variables and functions <https://wiki.sei.cmu.edu/confluence/spaces/c/pages/87152335/DCL19-C.+Minimize+the+scope+of+variables+and+functions>`_. diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/scope-reduction.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/scope-reduction.cpp index ba9e0e9d3c466..fcc6ea2613495 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/scope-reduction.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/scope-reduction.cpp @@ -2,7 +2,8 @@ // Test case 1: Variable can be moved to smaller scope (if-block) void test_if_scope() { - int x = 42; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'x' can be declared in a smaller scope + int x = 42; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'x' can be declared in a smaller scope if (true) { int y = x + 1; } @@ -19,7 +20,8 @@ int test_multiple_scopes(int v) { // Test case 3: Variable can be moved to nested if-block void test_nested_if() { - int a = 5; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'a' can be declared in a smaller scope + int a = 5; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'a' can be declared in a smaller scope if (true) { if (true) { int b = a * 2; @@ -34,8 +36,11 @@ void test_same_scope() { } // Test case 5: Variable can be moved to while loop body +// TODO: This is a false positive. Correcting this will require +// loop semantic comprehension and var lifetime analysis. void test_while_loop() { - int counter = 0; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'counter' can be declared in a smaller scope + int counter = 0; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'counter' can be declared in a smaller scope while (true) { counter++; if (counter > 10) break; @@ -54,7 +59,8 @@ void test_if_branches(bool condition) { // Test case 7: Variable can be moved to for-loop body void test_for_loop_body() { - int temp = 0; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'temp' can be declared in a smaller scope + int temp = 0; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'temp' can be declared in a smaller scope for (int i = 0; i < 10; i++) { temp = i * i; } @@ -62,7 +68,8 @@ void test_for_loop_body() { // Test case 8: Variable used in for-loop expressions - should NOT warn (current limitation) void test_for_loop_expressions() { - int i; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'i' can be declared in for-loop initialization + int i; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'i' can be declared in for-loop initialization for (i = 0; i < 5; i++) { // loop body } @@ -70,7 +77,8 @@ void test_for_loop_expressions() { // Test case 9: Variable can be moved to switch case void test_switch_case(int value) { - int result = 0; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'result' can be declared in a smaller scope + int result = 0; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'result' can be declared in a smaller scope switch (value) { case 1: result = 10; @@ -82,7 +90,8 @@ void test_switch_case(int value) { // Test case 10: Variable used across multiple switch cases - should NOT warn void test_switch_multiple_cases(int value) { - int accumulator = 0; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'accumulator' can be declared in a smaller scope + int accumulator = 0; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'accumulator' can be declared in a smaller scope switch (value) { case 1: accumulator += 10; @@ -95,7 +104,8 @@ void test_switch_multiple_cases(int value) { // Test case 11: Variable with complex initialization can be moved void test_complex_init() { - int complex = (5 + 3) * 2; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'complex' can be declared in a smaller scope + int complex = (5 + 3) * 2; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'complex' can be declared in a smaller scope if (true) { int doubled = complex * 2; } @@ -103,7 +113,8 @@ void test_complex_init() { // Test case 12: Multiple variables, some can be moved, some cannot int test_mixed_variables(bool flag) { - int movable = 10; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'movable' can be declared in a smaller scope + int movable = 10; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'movable' can be declared in a smaller scope int unmovable = 20; // Should NOT warn - used across scopes if (flag) { @@ -116,7 +127,8 @@ int test_mixed_variables(bool flag) { // Test case 13: Variable in try-catch block void test_try_catch() { - int error_code = 0; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'error_code' can be declared in a smaller scope + int error_code = 0; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'error_code' can be declared in a smaller scope try { error_code = 404; } catch (...) { @@ -136,7 +148,8 @@ void test_try_catch_shared() { // Test case 15: Deeply nested scopes void test_deep_nesting() { - int deep = 1; // CHECK-MESSAGES: :[[@LINE]]:7: warning: variable 'deep' can be declared in a smaller scope + int deep = 1; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'deep' can be declared in a smaller scope if (true) { if (true) { if (true) { >From 63c653a76be822552495101edc364b3c8c41b423 Mon Sep 17 00:00:00 2001 From: Vince Bridgers <[email protected]> Date: Sun, 11 Jan 2026 16:04:07 +0100 Subject: [PATCH 3/3] Update to correct CI issues missed in last update * Remove include of DeclRefExprUtils.h * Correct spelling in header guard --- clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.h b/clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.h index ef3c898f9c95f..15fb82cc02fb5 100644 --- a/clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.h +++ b/clang-tools-extra/clang-tidy/misc/ScopeReductionCheck.h @@ -7,9 +7,8 @@ //===----------------------------------------------------------------------===// #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SCOPEREDUCTIONCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SCOPEREDCUTIONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SCOPEREDUCTIONCHECK_H -#include "../../clang-tidy/utils/DeclRefExprUtils.h" #include "../ClangTidyCheck.h" namespace clang::tidy::misc { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
