[cfe-commits] r162093 - in /cfe/trunk: lib/Sema/SemaDecl.cpp test/Analysis/stack-addr-ps.cpp test/SemaCXX/convert-to-bool.cpp test/SemaCXX/references.cpp test/SemaCXX/uninitialized.cpp

[Chandler Carruth]

On Fri, Aug 17, 2012 at 3:12 AM, Hans Wennborg <hans at hanshq.net> wrote:

> Author: hans
> Date: Fri Aug 17 05:12:33 2012
> New Revision: 162093
>
> URL: http://llvm.org/viewvc/llvm-project?rev=162093&view=rev
> Log:
> Warn about self-initialization of references.
>
> Initializing a reference with itself, e.g. "int &a = a;" seems like a
> very bad idea.
>
> Modified:
>     cfe/trunk/lib/Sema/SemaDecl.cpp
>     cfe/trunk/test/Analysis/stack-addr-ps.cpp
>     cfe/trunk/test/SemaCXX/convert-to-bool.cpp
>     cfe/trunk/test/SemaCXX/references.cpp
>     cfe/trunk/test/SemaCXX/uninitialized.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=162093&r1=162092&r2=162093&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Aug 17 05:12:33 2012
> @@ -6174,6 +6174,7 @@
>      Decl *OrigDecl;
>      bool isRecordType;
>      bool isPODType;
> +    bool isReferenceType;
>
>    public:
>      typedef EvaluatedExprVisitor<SelfReferenceChecker> Inherited;
> @@ -6182,9 +6183,11 @@
>                                                      S(S),
> OrigDecl(OrigDecl) {
>        isPODType = false;
>        isRecordType = false;
> +      isReferenceType = false;
>        if (ValueDecl *VD = dyn_cast<ValueDecl>(OrigDecl)) {
>          isPODType = VD->getType().isPODType(S.Context);
>          isRecordType = VD->getType()->isRecordType();
> +        isReferenceType = VD->getType()->isReferenceType();
>        }
>      }
>
> @@ -6192,9 +6195,9 @@
>      // 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 type.  Doing so is
> a
> -      // way to silence uninitialized warnings.
> -      if (isRecordType)
> +      // 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<DeclRefExpr>(E))
>            HandleDeclRefExpr(DRE);
>
> @@ -6309,11 +6312,11 @@
>    }
>
>    // Check for self-references within variable initializers.
> -  // Variables declared within a function/method body are handled
> -  // by a dataflow analysis.
> +  // 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() &&
> +  if ((!VDecl->hasLocalStorage() || VDecl->getType()->isReferenceType())
> &&
>        (isa<InitListExpr>(Init) || !VDecl->getType()->isRecordType()))
>      CheckSelfReference(RealDecl, Init);
>
>
> Modified: cfe/trunk/test/Analysis/stack-addr-ps.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/stack-addr-ps.cpp?rev=162093&r1=162092&r2=162093&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Analysis/stack-addr-ps.cpp (original)
> +++ cfe/trunk/test/Analysis/stack-addr-ps.cpp Fri Aug 17 05:12:33 2012
> @@ -87,6 +87,6 @@
>
>  // rdar://11345441
>  int* f5() {
> -  int& i = i; // expected-warning {{Assigned value is garbage or
> undefined}} expected-note {{binding reference variable 'i' here}}
> +  int& i = i; // expected-warning {{Assigned value is garbage or
> undefined}} expected-note {{binding reference variable 'i' here}}
> expected-warning{{variable 'i' is uninitialized when used within its own
> initialization}}
>    return &i; // expected-warning {{address of stack memory associated
> with local variable 'i' returned}}
>  }
>
> Modified: cfe/trunk/test/SemaCXX/convert-to-bool.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/convert-to-bool.cpp?rev=162093&r1=162092&r2=162093&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/convert-to-bool.cpp (original)
> +++ cfe/trunk/test/SemaCXX/convert-to-bool.cpp Fri Aug 17 05:12:33 2012
> @@ -62,6 +62,5 @@
>
>  void test_copy_init_conversions(C c) {
>    A &a = c; // expected-error{{no viable conversion from 'C' to 'A'}}
> -  B &b = b; // okay
> +  B &b = c; // okay
>  }
> -
>
> Modified: cfe/trunk/test/SemaCXX/references.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/references.cpp?rev=162093&r1=162092&r2=162093&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/references.cpp (original)
> +++ cfe/trunk/test/SemaCXX/references.cpp Fri Aug 17 05:12:33 2012
> @@ -136,4 +136,4 @@
>  }
>
>  // The following crashed trying to recursively evaluate the LValue.
> -const int &do_not_crash = do_not_crash;
> +const int &do_not_crash = do_not_crash; // expected-warning{{variable
> 'do_not_crash' is uninitialized when used within its own initialization}}
>
> Modified: cfe/trunk/test/SemaCXX/uninitialized.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/uninitialized.cpp?rev=162093&r1=162092&r2=162093&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/uninitialized.cpp (original)
> +++ cfe/trunk/test/SemaCXX/uninitialized.cpp Fri Aug 17 05:12:33 2012
> @@ -378,3 +378,22 @@
>      }
>    }
>  }
> +
> +namespace references {
> +  int &a = a; // expected-warning{{variable 'a' is uninitialized when
> used within its own initialization}}
>

While completely correct (we are initializing the reference to an
uninitialized reference), I wonder if this will confuse people into
thinking there is some kind of copy or uninitialized *value* underlying the
reference... Is there a better / more clear / more specific warning text we
can use here?

"reference 'a' not yet been bound to a value when used within its own
initialization"?
"reference 'a' is unbound when used within its own initialization"?

Unsure. Maybe others have better ideas, or maybe they prefer the existing
text....
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120817/bb72801d/attachment.html>