https://github.com/arrowten updated https://github.com/llvm/llvm-project/pull/156436
>From 6cfcc64ef626024438858d244834c7febca75b94 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/Scope.h | 9 ++ clang/include/clang/Sema/Sema.h | 4 + clang/lib/Parse/ParseStmt.cpp | 4 + clang/lib/Sema/SemaExpr.cpp | 22 +++ clang/test/Sema/warn-conditional-scope.cpp | 146 ++++++++++++++++++ 6 files changed, 188 insertions(+) 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..cd7c0e6827eeb 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">>; // AMDGCN builtins diagnostics def err_amdgcn_load_lds_size_invalid_value : Error<"invalid size value">; diff --git a/clang/include/clang/Sema/Scope.h b/clang/include/clang/Sema/Scope.h index 757f3dcc3fe8d..b542680df4df7 100644 --- a/clang/include/clang/Sema/Scope.h +++ b/clang/include/clang/Sema/Scope.h @@ -255,6 +255,9 @@ class Scope { /// available for this variable in the current scope. llvm::SmallPtrSet<VarDecl *, 8> ReturnSlots; + // Condition variable for if + VarDecl *ConditionVar = nullptr; + void setFlags(Scope *Parent, unsigned F); public: @@ -263,6 +266,12 @@ class Scope { Init(Parent, ScopeFlags); } + // getConditionVar - Gets the condition variable of the scope. + VarDecl *getConditionVar() const { return ConditionVar; } + + // setConditionVar - Setter for condition variable of this scope. + void setConditionVar(VarDecl *D) { ConditionVar = D; } + /// getFlags - Return the flags for this scope. unsigned getFlags() const { return Flags; } diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index c3fb57774c8dc..938509962fdb9 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -7171,6 +7171,10 @@ class Sema final : public SemaBase { /// return a reason explaining why. Otherwise, return NOUR_None. NonOdrUseReason getNonOdrUseReasonInCurrentContext(ValueDecl *D); + /// Checks if the given DeclRefExpr refers to a variable declared in an + /// enclosing 'if' condition. + bool isConditionVarReference(const DeclRefExpr *DRE); + DeclRefExpr *BuildDeclRefExpr(ValueDecl *D, QualType Ty, ExprValueKind VK, SourceLocation Loc, const CXXScopeSpec *SS = nullptr); diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp index bf1978c22ee9f..f791e30c34359 100644 --- a/clang/lib/Parse/ParseStmt.cpp +++ b/clang/lib/Parse/ParseStmt.cpp @@ -1526,6 +1526,10 @@ StmtResult Parser::ParseIfStatement(SourceLocation *TrailingElseLoc) { SourceLocation ElseStmtLoc; StmtResult ElseStmt; + if (VarDecl *VD = dyn_cast_or_null<VarDecl>(Cond.get().first); + !Cond.isInvalid() && VD) + getCurScope()->setConditionVar(VD->getCanonicalDecl()); + if (Tok.is(tok::kw_else)) { if (TrailingElseLoc) *TrailingElseLoc = Tok.getLocation(); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index d383544e54329..7345f277011a0 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -2371,6 +2371,24 @@ NonOdrUseReason Sema::getNonOdrUseReasonInCurrentContext(ValueDecl *D) { return NOUR_None; } +bool Sema::isConditionVarReference(const DeclRefExpr *DRE) { + if (!DRE) + return false; + + const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl()); + + if (!VD) + return false; + + for (Scope *S = getCurScope(); S; S = S->getParent()) { + if (VarDecl *CV = S->getConditionVar()) + if (VD == CV) + return true; + } + + return false; +} + DeclRefExpr * Sema::BuildDeclRefExpr(ValueDecl *D, QualType Ty, ExprValueKind VK, const DeclarationNameInfo &NameInfo, @@ -2425,6 +2443,10 @@ Sema::BuildDeclRefExpr(ValueDecl *D, QualType Ty, ExprValueKind VK, if (const auto *BE = BD->getBinding()) E->setObjectKind(BE->getObjectKind()); + if (isConditionVarReference(E)) + Diag(E->getBeginLoc(), diag::warn_out_of_scope_var_usage) + << D->getDeclName(); + return E; } diff --git a/clang/test/Sema/warn-conditional-scope.cpp b/clang/test/Sema/warn-conditional-scope.cpp new file mode 100644 index 0000000000000..0c65c578d1fb5 --- /dev/null +++ b/clang/test/Sema/warn-conditional-scope.cpp @@ -0,0 +1,146 @@ +// 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 +// RUN: %clang_cc1 -fsyntax-only -verify -Wconditional-scope -DTEST_5 %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wconditional-scope -DTEST_6 %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() { + // expected-no-diagnostics + if (bool x = get_something()) {} + else { + { + bool x = get_something_else(); + if (x) {} + } + } +} + +#endif + +#ifdef TEST_6 + +int test() { + if (int *ptr = get_something()) { + return ptr[0]; + } + else if (int *ptr = get_something_else()) { + return ptr[0] * 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 { + return ptr[0] * ptr[0]; + } +} + +#endif _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
