From 3267347ccac4009d14a2be63bde5cc471de244bf Mon Sep 17 00:00:00 2001 From: Richard Trieu Date: Mon, 1 Oct 2012 17:39:51 +0000 Subject: [PATCH] Cleaning up the self initialization checker. -Allow Sema to do more processing on the initial Expr before checking it. -Remove the special conditions in HandleExpr() -Move the code so that only one call site is needed. -Removed the function from Sema and only call it locally. -Warn on potentially evaluated reference variables, not just casts to r-values. -Update tests. llvm-svn: 164951 --- clang/include/clang/Sema/Sema.h | 1 - clang/lib/Sema/SemaDecl.cpp | 69 +++++++++++++++++++---------------- clang/lib/Sema/SemaInit.cpp | 8 ---- clang/test/Parser/cxx0x-condition.cpp | 9 +++-- clang/test/SemaCXX/uninitialized.cpp | 10 +++++ 5 files changed, 53 insertions(+), 44 deletions(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 97d425c..c53c4f4 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -1308,7 +1308,6 @@ public: bool SetParamDefaultArgument(ParmVarDecl *Param, Expr *DefaultArg, SourceLocation EqualLoc); - void CheckSelfReference(Decl *OrigDecl, Expr *E); void AddInitializerToDecl(Decl *dcl, Expr *init, bool DirectInit, bool TypeMayContainAuto); void ActOnUninitializedDecl(Decl *dcl, bool TypeMayContainAuto); diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 019a7f1..593c110 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -6259,28 +6259,12 @@ namespace { } } - // Sometimes, the expression passed in lacks the casts that are used - // to determine which DeclRefExpr's to check. Assume that the casts - // are present and continue visiting the expression. - void HandleExpr(Expr *E) { - // Skip checking T a = a where T is not a record or reference type. - // Doing so is a way to silence uninitialized warnings. - if (isRecordType || isReferenceType) - if (DeclRefExpr *DRE = dyn_cast(E)) - HandleDeclRefExpr(DRE); - - if (ConditionalOperator *CO = dyn_cast(E)) { - HandleValue(CO->getTrueExpr()); - HandleValue(CO->getFalseExpr()); - } - - Visit(E); - } - // For most expressions, the cast is directly above the DeclRefExpr. // For conditional operators, the cast can be outside the conditional // operator if both expressions are DeclRefExpr's. void HandleValue(Expr *E) { + if (isReferenceType) + return; E = E->IgnoreParenImpCasts(); if (DeclRefExpr* DRE = dyn_cast(E)) { HandleDeclRefExpr(DRE); @@ -6293,6 +6277,13 @@ namespace { } } + // Reference types are handled here since all uses of references are + // bad, not just r-value uses. + void VisitDeclRefExpr(DeclRefExpr *E) { + if (isReferenceType) + HandleDeclRefExpr(E); + } + void VisitImplicitCastExpr(ImplicitCastExpr *E) { if ((!isRecordType && E->getCastKind() == CK_LValueToRValue) || (isRecordType && E->getCastKind() == CK_NoOp)) @@ -6339,11 +6330,28 @@ namespace { << DRE->getSourceRange()); } }; -} -/// CheckSelfReference - Warns if OrigDecl is used in expression E. -void Sema::CheckSelfReference(Decl* OrigDecl, Expr *E) { - SelfReferenceChecker(*this, OrigDecl).HandleExpr(E); + /// CheckSelfReference - Warns if OrigDecl is used in expression E. + static void CheckSelfReference(Sema &S, Decl* OrigDecl, Expr *E, + bool DirectInit) { + // Parameters arguments are occassionially constructed with itself, + // for instance, in recursive functions. Skip them. + if (isa(OrigDecl)) + return; + + E = E->IgnoreParens(); + + // Skip checking T a = a where T is not a record or reference type. + // Doing so is a way to silence uninitialized warnings. + if (!DirectInit && !cast(OrigDecl)->getType()->isRecordType()) + if (ImplicitCastExpr *ICE = dyn_cast(E)) + if (ICE->getCastKind() == CK_LValueToRValue) + if (DeclRefExpr *DRE = dyn_cast(ICE->getSubExpr())) + if (DRE->getDecl() == OrigDecl) + return; + + SelfReferenceChecker(S, OrigDecl).Visit(E); + } } /// AddInitializerToDecl - Adds the initializer Init to the @@ -6380,15 +6388,6 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, return; } - // Check for self-references within variable initializers. - // Variables declared within a function/method body (except for references) - // are handled by a dataflow analysis. - // Record types initialized by initializer list are handled here. - // Initialization by constructors are handled in TryConstructorInitialization. - if ((!VDecl->hasLocalStorage() || VDecl->getType()->isReferenceType()) && - (isa(Init) || !VDecl->getType()->isRecordType())) - CheckSelfReference(RealDecl, Init); - ParenListExpr *CXXDirectInit = dyn_cast(Init); // C++11 [decl.spec.auto]p6. Deduce the type which 'auto' stands in for. @@ -6573,6 +6572,14 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, Init = Result.takeAs(); } + // Check for self-references within variable initializers. + // Variables declared within a function/method body (except for references) + // are handled by a dataflow analysis. + if (!VDecl->hasLocalStorage() || VDecl->getType()->isRecordType() || + VDecl->getType()->isReferenceType()) { + CheckSelfReference(*this, RealDecl, Init, DirectInit); + } + // If the type changed, it means we had an incomplete type that was // completed by the initializer. For example: // int ary[] = { 1, 3, 5 }; diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 42fc84d..f7a173f 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -2819,14 +2819,6 @@ static void TryConstructorInitialization(Sema &S, assert((!InitListSyntax || (NumArgs == 1 && isa(Args[0]))) && "InitListSyntax must come with a single initializer list argument."); - // Check constructor arguments for self reference. - if (DeclaratorDecl *DD = Entity.getDecl()) - // Parameters arguments are occassionially constructed with itself, - // for instance, in recursive functions. Skip them. - if (!isa(DD)) - for (unsigned i = 0; i < NumArgs; ++i) - S.CheckSelfReference(DD, Args[i]); - // The type we're constructing needs to be complete. if (S.RequireCompleteType(Kind.getLocation(), DestType, 0)) { Sequence.setIncompleteTypeFailure(DestType); diff --git a/clang/test/Parser/cxx0x-condition.cpp b/clang/test/Parser/cxx0x-condition.cpp index e45cd86..8b64bcf 100644 --- a/clang/test/Parser/cxx0x-condition.cpp +++ b/clang/test/Parser/cxx0x-condition.cpp @@ -27,10 +27,11 @@ void f() { if (S b(n) = 0) {} // expected-error {{a function type is not allowed here}} if (S b(n) == 0) {} // expected-error {{a function type is not allowed here}} expected-error {{did you mean '='?}} - if (S{a}) {} // ok - if (S a{a}) {} // ok - if (S a = {a}) {} // ok - if (S a == {a}) {} // expected-error {{did you mean '='?}} + S s(a); + if (S{s}) {} // ok + if (S a{s}) {} // ok + if (S a = {s}) {} // ok + if (S a == {s}) {} // expected-error {{did you mean '='?}} if (S(b){a}) {} // ok if (S(b) = {a}) {} // ok diff --git a/clang/test/SemaCXX/uninitialized.cpp b/clang/test/SemaCXX/uninitialized.cpp index 6725bf9..6148449 100644 --- a/clang/test/SemaCXX/uninitialized.cpp +++ b/clang/test/SemaCXX/uninitialized.cpp @@ -114,6 +114,8 @@ void setupA(bool x) { A a17(a17.get2()); // expected-warning {{variable 'a17' is uninitialized when used within its own initialization}} A a18 = x ? a18 : a17; // expected-warning {{variable 'a18' is uninitialized when used within its own initialization}} A a19 = getA(x ? a19 : a17); // expected-warning {{variable 'a19' is uninitialized when used within its own initialization}} + A a20{a20}; // expected-warning {{variable 'a20' is uninitialized when used within its own initialization}} + A a21 = {a21}; // expected-warning {{variable 'a21' is uninitialized when used within its own initialization}} } bool x; @@ -138,6 +140,8 @@ A a16(&a16.num); // expected-warning {{variable 'a16' is uninitialized when use A a17(a17.get2()); // expected-warning {{variable 'a17' is uninitialized when used within its own initialization}} A a18 = x ? a18 : a17; // expected-warning {{variable 'a18' is uninitialized when used within its own initialization}} A a19 = getA(x ? a19 : a17); // expected-warning {{variable 'a19' is uninitialized when used within its own initialization}} +A a20{a20}; // expected-warning {{variable 'a20' is uninitialized when used within its own initialization}} +A a21 = {a21}; // expected-warning {{variable 'a21' is uninitialized when used within its own initialization}} struct B { // POD struct. @@ -400,6 +404,9 @@ namespace in_class_initializers { namespace references { int &a = a; // expected-warning{{reference 'a' is not yet bound to a value when used within its own initialization}} + int &b(b); // expected-warning{{reference 'b' is not yet bound to a value when used within its own initialization}} + int &c = a ? b : c; // expected-warning{{reference 'c' is not yet bound to a value when used within its own initialization}} + int &d{d}; // expected-warning{{reference 'd' is not yet bound to a value when used within its own initialization}} struct S { S() : a(a) {} // expected-warning{{reference 'a' is not yet bound to a value when used here}} @@ -408,6 +415,9 @@ namespace references { void f() { int &a = a; // expected-warning{{reference 'a' is not yet bound to a value when used within its own initialization}} + int &b(b); // expected-warning{{reference 'b' is not yet bound to a value when used within its own initialization}} + int &c = a ? b : c; // expected-warning{{reference 'c' is not yet bound to a value when used within its own initialization}} + int &d{d}; // expected-warning{{reference 'd' is not yet bound to a value when used within its own initialization}} } struct T { -- 2.7.4