https://github.com/arrowten updated https://github.com/llvm/llvm-project/pull/156436
>From e77df2158b52ff474c3ce7b468cf0e6e1ddf4040 Mon Sep 17 00:00:00 2001 From: arrowten <[email protected]> Date: Tue, 2 Sep 2025 15:33:52 +0530 Subject: [PATCH] [Sema] Diagnose use of if condition variable inside else branch(s) --- .../clang/Basic/DiagnosticSemaKinds.td | 3 + clang/include/clang/Sema/Sema.h | 6 + clang/lib/Sema/SemaDecl.cpp | 5 + clang/lib/Sema/SemaExpr.cpp | 27 ++++ clang/test/Parser/warn-conditional-scope.cpp | 111 ++++++++++++++++ clang/test/Sema/warn-conditional-scope.cpp | 125 ++++++++++++++++++ 6 files changed, 277 insertions(+) create mode 100644 clang/test/Parser/warn-conditional-scope.cpp create mode 100644 clang/test/Sema/warn-conditional-scope.cpp diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index c934fed2c7462..5c06db4b2a45b 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -13594,6 +13594,9 @@ def warn_acc_var_referenced_lacks_op "reference has no effect">, InGroup<DiagGroup<"openacc-var-lacks-operation">>, DefaultError; +def warn_out_of_scope_var_usage + : Warning<"variable %0 declared in 'if' block is always false or null here">, + InGroup<DiagGroup<"conditional-scope">>, DefaultIgnore; // AMDGCN builtins diagnostics def err_amdgcn_load_lds_size_invalid_value : Error<"invalid size value">; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index c3fb57774c8dc..2c5167ed4088b 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -1564,6 +1564,12 @@ class Sema final : public SemaBase { Sema(const Sema &) = delete; void operator=(const Sema &) = delete; + /// Tracks condition variables declared in `if` statements, + /// mapping each VarDecl to the `Scope*` where it was created. + /// Used to detect references to these variables outside their + /// lifetime (e.g. in the `else` branch or after the `if`). + llvm::DenseMap<const VarDecl*, Scope*> IfScopeVars; + /// The handler for the FileChanged preprocessor events. /// /// Used for diagnostics that implement custom semantic analysis for #include diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 12bedae05f6f3..31de40f64fbfe 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -8350,6 +8350,11 @@ NamedDecl *Sema::ActOnVariableDeclarator( emitReadOnlyPlacementAttrWarning(*this, NewVD); + if (NewVD) { + if (S->getFlags() & Scope::ControlScope) + IfScopeVars[NewVD] = S; + } + return NewVD; } diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index d383544e54329..ddbb122bd08d6 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -2425,6 +2425,33 @@ Sema::BuildDeclRefExpr(ValueDecl *D, QualType Ty, ExprValueKind VK, if (const auto *BE = BD->getBinding()) E->setObjectKind(BE->getObjectKind()); + auto IsScopeContaining = [](Scope *S, Scope *DeclaredScope) -> bool { + while (S) { + if (S == DeclaredScope) + return true; + + // If we hit an if/else scope boundary, stop searching + if (S->getFlags() & Scope::ControlScope) + return false; + + S = S->getParent(); + } + + return false; + }; + + if (auto *VD = dyn_cast_or_null<VarDecl>(D)) { + auto It = IfScopeVars.find(VD); + + if (It != IfScopeVars.end()) { + Scope *DeclaredScope = It->second; + + if (!IsScopeContaining(CurScope, DeclaredScope)) + Diag(NameInfo.getLoc(), diag::warn_out_of_scope_var_usage) + << VD->getName(); + } + } + return E; } diff --git a/clang/test/Parser/warn-conditional-scope.cpp b/clang/test/Parser/warn-conditional-scope.cpp new file mode 100644 index 0000000000000..a6e3dfb95021d --- /dev/null +++ b/clang/test/Parser/warn-conditional-scope.cpp @@ -0,0 +1,111 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wconditional-scope -DTEST_1 %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wconditional-scope -DTEST_2 %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wconditional-scope -DTEST_3 %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wconditional-scope -DTEST_4 %s + +int *get_something(); +int *get_something_else(); +int *get_something_else_again(); +int *get_something_else_again_now(); + +#ifdef TEST_1 + +int test() { + if (int *ptr = get_something()) { + return ptr[0] * ptr[0]; + } + // expected-warning@+2{{variable ptr declared in 'if' block is either false or null}} + else if (int *ptr2 = get_something_else()) { + return ptr[0] * ptr2[0]; + } + // expected-warning@+3{{variable ptr declared in 'if' block is either false or null}} + // expected-warning@+2{{variable ptr2 declared in 'if' block is either false or null}} + else if (int* ptr3 = get_something_else_again()) { + return ptr[0] * ptr2[0] * ptr3[0]; + } + // expected-warning@+4{{variable ptr declared in 'if' block is either false or null}} + // expected-warning@+3{{variable ptr2 declared in 'if' block is either false or null}} + // expected-warning@+2{{variable ptr3 declared in 'if' block is either false or null}} + else if (int *ptr4 = get_something_else_again_now()) { + return ptr[0] * ptr2[0] * ptr3[0] * ptr4[0]; + } + else { + return -1; + } +} + +#endif + +#ifdef TEST_2 + +int test() { + if (int *ptr = get_something()) { + return ptr[0] * ptr[0]; + } + else if (int *ptr2 = get_something_else()) { + return ptr2[0] * ptr2[0]; + } + // expected-warning@+3{{variable ptr declared in 'if' block is either false or null}} + // expected-warning@+2{{variable ptr2 declared in 'if' block is either false or null}} + else if (int* ptr3 = get_something_else_again()) { + return ptr[0] * ptr2[0] * ptr3[0]; + } + else { + return -1; + } +} + +#endif + +#ifdef TEST_3 + +int test() { + if (int *ptr = get_something()) { + return ptr[0] * ptr[0]; + } + else if (int *ptr2 = get_something_else()) { + return ptr2[0] * ptr2[0]; + } + // expected-warning@+3{{variable ptr2 declared in 'if' block is either false or null}} + // expected-warning@+2{{variable ptr2 declared in 'if' block is either false or null}} + else if (int* ptr3 = get_something_else_again()) { + return ptr2[0] * ptr2[0] * ptr3[0]; + } + else { + return -1; + } +} + +#endif + +#ifdef TEST_4 + +int test() { + int x = 10; + + if (x == 30) { + return x; + } + else if (int *ptr = get_something()) { + return ptr[0]; + } + // expected-warning@+3{{variable ptr declared in 'if' block is either false or null}} + // expected-warning@+2{{variable ptr declared in 'if' block is either false or null}} + else if (x == 20) { + return ptr[0] * ptr[0]; + } + // expected-warning@+2{{variable ptr declared in 'if' block is either false or null}} + else if (int* ptr2 = get_something_else()) { + return ptr[0] * ptr2[0]; + } + // expected-warning@+3{{variable ptr declared in 'if' block is either false or null}} + // expected-warning@+2{{variable ptr2 declared in 'if' block is either false or null}} + else if (int *ptr3 = get_something_else_again()) { + return ptr[0] * ptr2[0] * ptr3[0]; + } + else { + return -1; + } +} + +#endif diff --git a/clang/test/Sema/warn-conditional-scope.cpp b/clang/test/Sema/warn-conditional-scope.cpp new file mode 100644 index 0000000000000..4e11a86d8df35 --- /dev/null +++ b/clang/test/Sema/warn-conditional-scope.cpp @@ -0,0 +1,125 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wconditional-scope -DTEST_1 %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wconditional-scope -DTEST_2 %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wconditional-scope -DTEST_3 %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wconditional-scope -DTEST_4 %s + +int *get_something(); +int *get_something_else(); +int *get_something_else_again(); +int *get_something_else_again_now(); + +#ifdef TEST_1 + +int test() { + if (int *ptr = get_something()) { + return ptr[0] * ptr[0]; + } + // expected-warning@+2{{variable ptr declared in 'if' block is always false or null here}} + else if (int *ptr2 = get_something_else()) { + return ptr[0] * ptr2[0]; + } + // expected-warning@+3{{variable ptr declared in 'if' block is always false or null here}} + // expected-warning@+2{{variable ptr2 declared in 'if' block is always false or null here}} + else if (int* ptr3 = get_something_else_again()) { + return ptr[0] * ptr2[0] * ptr3[0]; + } + // expected-warning@+4{{variable ptr declared in 'if' block is always false or null here}} + // expected-warning@+3{{variable ptr2 declared in 'if' block is always false or null here}} + // expected-warning@+2{{variable ptr3 declared in 'if' block is always false or null here}} + else if (int *ptr4 = get_something_else_again_now()) { + return ptr[0] * ptr2[0] * ptr3[0] * ptr4[0]; + } + else { + return -1; + } +} + +#endif + +#ifdef TEST_2 + +int test() { + if (int *ptr = get_something()) { + return ptr[0] * ptr[0]; + } + else if (int *ptr2 = get_something_else()) { + return ptr2[0] * ptr2[0]; + } + // expected-warning@+3{{variable ptr declared in 'if' block is always false or null here}} + // expected-warning@+2{{variable ptr2 declared in 'if' block is always false or null here}} + else if (int* ptr3 = get_something_else_again()) { + return ptr[0] * ptr2[0] * ptr3[0]; + } + else { + return -1; + } +} + +#endif + +#ifdef TEST_3 + +int test() { + if (int *ptr = get_something()) { + return ptr[0] * ptr[0]; + } + else if (int *ptr2 = get_something_else()) { + return ptr2[0] * ptr2[0]; + } + // expected-warning@+3{{variable ptr2 declared in 'if' block is always false or null here}} + // expected-warning@+2{{variable ptr2 declared in 'if' block is always false or null here}} + else if (int* ptr3 = get_something_else_again()) { + return ptr2[0] * ptr2[0] * ptr3[0]; + } + else { + return -1; + } +} + +#endif + +#ifdef TEST_4 + +int test() { + int x = 10; + + if (x == 30) { + return x; + } + else if (int *ptr = get_something()) { + return ptr[0]; + } + // expected-warning@+3{{variable ptr declared in 'if' block is always false or null here}} + // expected-warning@+2{{variable ptr declared in 'if' block is always false or null here}} + else if (x == 20) { + return ptr[0] * ptr[0]; + } + // expected-warning@+2{{variable ptr declared in 'if' block is always false or null here}} + else if (int* ptr2 = get_something_else()) { + return ptr[0] * ptr2[0]; + } + // expected-warning@+3{{variable ptr declared in 'if' block is always false or null here}} + // expected-warning@+2{{variable ptr2 declared in 'if' block is always false or null here}} + else if (int *ptr3 = get_something_else_again()) { + return ptr[0] * ptr2[0] * ptr3[0]; + } + else { + return -1; + } +} + +#endif + +#ifdef TEST_5 + +void test() { + if (bool x = get_something()) {} + else { + { + bool x = get_something_else(); + if (x) {} + } + } +} + +#endif _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
