<div class="gmail_extra"><div class="gmail_quote">On Fri, Aug 17, 2012 at 3:12 AM, Hans Wennborg <span dir="ltr"><<a href="mailto:hans@hanshq.net" target="_blank" class="cremed">hans@hanshq.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: hans<br>
Date: Fri Aug 17 05:12:33 2012<br>
New Revision: 162093<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=162093&view=rev" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project?rev=162093&view=rev</a><br>
Log:<br>
Warn about self-initialization of references.<br>
<br>
Initializing a reference with itself, e.g. "int &a = a;" seems like a<br>
very bad idea.<br>
<br>
Modified:<br>
cfe/trunk/lib/Sema/SemaDecl.cpp<br>
cfe/trunk/test/Analysis/stack-addr-ps.cpp<br>
cfe/trunk/test/SemaCXX/convert-to-bool.cpp<br>
cfe/trunk/test/SemaCXX/references.cpp<br>
cfe/trunk/test/SemaCXX/uninitialized.cpp<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=162093&r1=162092&r2=162093&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=162093&r1=162092&r2=162093&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Aug 17 05:12:33 2012<br>
@@ -6174,6 +6174,7 @@<br>
Decl *OrigDecl;<br>
bool isRecordType;<br>
bool isPODType;<br>
+ bool isReferenceType;<br>
<br>
public:<br>
typedef EvaluatedExprVisitor<SelfReferenceChecker> Inherited;<br>
@@ -6182,9 +6183,11 @@<br>
S(S), OrigDecl(OrigDecl) {<br>
isPODType = false;<br>
isRecordType = false;<br>
+ isReferenceType = false;<br>
if (ValueDecl *VD = dyn_cast<ValueDecl>(OrigDecl)) {<br>
isPODType = VD->getType().isPODType(S.Context);<br>
isRecordType = VD->getType()->isRecordType();<br>
+ isReferenceType = VD->getType()->isReferenceType();<br>
}<br>
}<br>
<br>
@@ -6192,9 +6195,9 @@<br>
// to determine which DeclRefExpr's to check. Assume that the casts<br>
// are present and continue visiting the expression.<br>
void HandleExpr(Expr *E) {<br>
- // Skip checking T a = a where T is not a record type. Doing so is a<br>
- // way to silence uninitialized warnings.<br>
- if (isRecordType)<br>
+ // Skip checking T a = a where T is not a record or reference type.<br>
+ // Doing so is a way to silence uninitialized warnings.<br>
+ if (isRecordType || isReferenceType)<br>
if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E))<br>
HandleDeclRefExpr(DRE);<br>
<br>
@@ -6309,11 +6312,11 @@<br>
}<br>
<br>
// Check for self-references within variable initializers.<br>
- // Variables declared within a function/method body are handled<br>
- // by a dataflow analysis.<br>
+ // Variables declared within a function/method body (except for references)<br>
+ // are handled by a dataflow analysis.<br>
// Record types initialized by initializer list are handled here.<br>
// Initialization by constructors are handled in TryConstructorInitialization.<br>
- if (!VDecl->hasLocalStorage() &&<br>
+ if ((!VDecl->hasLocalStorage() || VDecl->getType()->isReferenceType()) &&<br>
(isa<InitListExpr>(Init) || !VDecl->getType()->isRecordType()))<br>
CheckSelfReference(RealDecl, Init);<br>
<br>
<br>
Modified: cfe/trunk/test/Analysis/stack-addr-ps.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/stack-addr-ps.cpp?rev=162093&r1=162092&r2=162093&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/stack-addr-ps.cpp?rev=162093&r1=162092&r2=162093&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Analysis/stack-addr-ps.cpp (original)<br>
+++ cfe/trunk/test/Analysis/stack-addr-ps.cpp Fri Aug 17 05:12:33 2012<br>
@@ -87,6 +87,6 @@<br>
<br>
// rdar://11345441<br>
int* f5() {<br>
- int& i = i; // expected-warning {{Assigned value is garbage or undefined}} expected-note {{binding reference variable 'i' here}}<br>
+ 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}}<br>
return &i; // expected-warning {{address of stack memory associated with local variable 'i' returned}}<br>
}<br>
<br>
Modified: cfe/trunk/test/SemaCXX/convert-to-bool.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/convert-to-bool.cpp?rev=162093&r1=162092&r2=162093&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/convert-to-bool.cpp?rev=162093&r1=162092&r2=162093&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/convert-to-bool.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/convert-to-bool.cpp Fri Aug 17 05:12:33 2012<br>
@@ -62,6 +62,5 @@<br>
<br>
void test_copy_init_conversions(C c) {<br>
A &a = c; // expected-error{{no viable conversion from 'C' to 'A'}}<br>
- B &b = b; // okay<br>
+ B &b = c; // okay<br>
}<br>
-<br>
<br>
Modified: cfe/trunk/test/SemaCXX/references.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/references.cpp?rev=162093&r1=162092&r2=162093&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/references.cpp?rev=162093&r1=162092&r2=162093&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/references.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/references.cpp Fri Aug 17 05:12:33 2012<br>
@@ -136,4 +136,4 @@<br>
}<br>
<br>
// The following crashed trying to recursively evaluate the LValue.<br>
-const int &do_not_crash = do_not_crash;<br>
+const int &do_not_crash = do_not_crash; // expected-warning{{variable 'do_not_crash' is uninitialized when used within its own initialization}}<br>
<br>
Modified: cfe/trunk/test/SemaCXX/uninitialized.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/uninitialized.cpp?rev=162093&r1=162092&r2=162093&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/uninitialized.cpp?rev=162093&r1=162092&r2=162093&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/uninitialized.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/uninitialized.cpp Fri Aug 17 05:12:33 2012<br>
@@ -378,3 +378,22 @@<br>
}<br>
}<br>
}<br>
+<br>
+namespace references {<br>
+ int &a = a; // expected-warning{{variable 'a' is uninitialized when used within its own initialization}}<br></blockquote><div><br></div><div>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?</div>
<div><br></div><div><div>"reference 'a' not yet been bound to a value when used within its own initialization"?</div></div><div><div><div>"reference 'a' is unbound when used within its own initialization"?</div>
</div></div><div><div><br></div></div><div>Unsure. Maybe others have better ideas, or maybe they prefer the existing text....</div></div></div>