[cfe-commits] r69114 - in /cfe/trunk: include/clang/AST/Expr.h lib/AST/Expr.cpp lib/Sema/SemaExpr.cpp test/Sema/exprs.c

[Daniel Dunbar]

Author: ddunbar
Date: Tue Apr 14 19:08:05 2009
New Revision: 69114

URL: http://llvm.org/viewvc/llvm-project?rev=69114&view=rev
Log:
Improve "assignment to cast" diagnostic.
 - Strip off extra parens when looking for casts.
 - Change the location info to point at the cast (instead of the
   assignment).

For example, on
  
  int *b;
  #define a ((void*) b)
  void f0() {
    a = 10;
  }
  
we now emit:
  
  /tmp/t.c:4:3: error: assignment to cast is illegal, lvalue casts are not supported
    a = 10;
    ^ ~
  /tmp/t.c:2:12: note: instantiated from:
  #define a ((void*) b)
            ~^~~~~~~~~~
  
instead of:
  
  /tmp/t.c:4:5: error: expression is not assignable
    a = 10;
    ~ ^

Modified:
    cfe/trunk/include/clang/AST/Expr.h
    cfe/trunk/lib/AST/Expr.cpp
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/Sema/exprs.c

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=69114&r1=69113&r2=69114&view=diff

==============================================================================
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Tue Apr 14 19:08:05 2009
@@ -152,6 +152,10 @@
   /// and if it is a structure or union, does not have any member (including, 
   /// recursively, any member or element of all contained aggregates or unions)
   /// with a const-qualified type.
+  ///
+  /// \param Loc [in] [out] - A source location which *may* be filled
+  /// in with the location of the expression making this a
+  /// non-modifiable lvalue, if specified.
   enum isModifiableLvalueResult {
     MLV_Valid,
     MLV_NotObjectType,
@@ -167,7 +171,8 @@
     MLV_NoSetterProperty,
     MLV_MemberFunction
   };
-  isModifiableLvalueResult isModifiableLvalue(ASTContext &Ctx) const;
+  isModifiableLvalueResult isModifiableLvalue(ASTContext &Ctx,
+                                              SourceLocation *Loc = 0) const;
   
   bool isBitField();
 

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=69114&r1=69113&r2=69114&view=diff

==============================================================================
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Tue Apr 14 19:08:05 2009
@@ -756,7 +756,8 @@
 /// if it is a structure or union, does not have any member (including, 
 /// recursively, any member or element of all contained aggregates or unions)
 /// with a const-qualified type.
-Expr::isModifiableLvalueResult Expr::isModifiableLvalue(ASTContext &Ctx) const {
+Expr::isModifiableLvalueResult 
+Expr::isModifiableLvalue(ASTContext &Ctx, SourceLocation *Loc) const {
   isLvalueResult lvalResult = isLvalue(Ctx);
     
   switch (lvalResult) {
@@ -775,9 +776,13 @@
     // lvalue, then this is probably a use of the old-school "cast as lvalue"
     // GCC extension.  We don't support it, but we want to produce good
     // diagnostics when it happens so that the user knows why.
-    if (const CStyleCastExpr *CE = dyn_cast<CStyleCastExpr>(IgnoreParens()))
-      if (CE->getSubExpr()->isLvalue(Ctx) == LV_Valid)
+    if (const CStyleCastExpr *CE = dyn_cast<CStyleCastExpr>(IgnoreParens())) {
+      if (CE->getSubExpr()->isLvalue(Ctx) == LV_Valid) {
+        if (Loc)
+          *Loc = CE->getLParenLoc();
         return MLV_LValueCast;
+      }
+    }
     return MLV_InvalidExpression;
   case LV_MemberFunction: return MLV_MemberFunction;
   }

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=69114&r1=69113&r2=69114&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Apr 14 19:08:05 2009
@@ -3725,7 +3725,9 @@
 /// CheckForModifiableLvalue - Verify that E is a modifiable lvalue.  If not,
 /// emit an error and return true.  If so, return false.
 static bool CheckForModifiableLvalue(Expr *E, SourceLocation Loc, Sema &S) {
-  Expr::isModifiableLvalueResult IsLV = E->isModifiableLvalue(S.Context);
+  SourceLocation OrigLoc = Loc;
+  Expr::isModifiableLvalueResult IsLV = E->isModifiableLvalue(S.Context, 
+                                                              &Loc);
   if (IsLV == Expr::MLV_Valid && IsReadonlyProperty(E, S))
     IsLV = Expr::MLV_ReadonlyProperty;
   if (IsLV == Expr::MLV_Valid)
@@ -3769,10 +3771,13 @@
     break;
   }
 
+  SourceRange Assign;
+  if (Loc != OrigLoc)
+    Assign = SourceRange(OrigLoc, OrigLoc);
   if (NeedType)
-    S.Diag(Loc, Diag) << E->getType() << E->getSourceRange();
+    S.Diag(Loc, Diag) << E->getType() << E->getSourceRange() << Assign;
   else
-    S.Diag(Loc, Diag) << E->getSourceRange();
+    S.Diag(Loc, Diag) << E->getSourceRange() << Assign; 
   return true;
 }
 

Modified: cfe/trunk/test/Sema/exprs.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/exprs.c?rev=69114&r1=69113&r2=69114&view=diff

==============================================================================
--- cfe/trunk/test/Sema/exprs.c (original)
+++ cfe/trunk/test/Sema/exprs.c Tue Apr 14 19:08:05 2009
@@ -34,7 +34,8 @@
 // rdar://6319320
 void test5(int *X, float *P) {
   (float*)X = P;   // expected-error {{assignment to cast is illegal, lvalue casts are not supported}}
-  ((float*)X) = P;   // expected-error {{assignment to cast is illegal, lvalue casts are not supported}}
+#define FOO ((float*) X)
+  FOO = P;   // expected-error {{assignment to cast is illegal, lvalue casts are not supported}}
 }
 
 void test6() {