Issue a warning about re-assigned locked local variable
authorMarek Safar <marek.safar@gmail.com>
Wed, 3 Nov 2010 15:53:12 +0000 (15:53 +0000)
committerMarek Safar <marek.safar@gmail.com>
Wed, 3 Nov 2010 15:53:12 +0000 (15:53 +0000)
mcs/errors/cs0728-2.cs [new file with mode: 0644]
mcs/errors/cs0728-3.cs [new file with mode: 0644]
mcs/errors/cs0728.cs [new file with mode: 0644]
mcs/mcs/ecore.cs
mcs/mcs/expression.cs
mcs/mcs/report.cs
mcs/mcs/statement.cs

diff --git a/mcs/errors/cs0728-2.cs b/mcs/errors/cs0728-2.cs
new file mode 100644 (file)
index 0000000..2f361a5
--- /dev/null
@@ -0,0 +1,15 @@
+// CS0728: Possibly incorrect assignment to `d' which is the argument to a using or lock statement
+// Line: 12
+// Compiler options: -warnaserror
+
+using System;
+
+public class Foo
+{
+       public static void Test (IDisposable d)
+       {
+               using (d) {
+                       d = null;
+               }
+       }
+}
diff --git a/mcs/errors/cs0728-3.cs b/mcs/errors/cs0728-3.cs
new file mode 100644 (file)
index 0000000..640f8da
--- /dev/null
@@ -0,0 +1,14 @@
+// CS0728: Possibly incorrect assignment to `s' which is the argument to a using or lock statement
+// Line: 12
+// Compiler options: -warnaserror
+
+public class Foo
+{
+       public static void Test (ref string s)
+       {
+               lock (s) {
+                       lock (s) {}
+                       s = null;
+               }
+       }
+}
diff --git a/mcs/errors/cs0728.cs b/mcs/errors/cs0728.cs
new file mode 100644 (file)
index 0000000..1e4b848
--- /dev/null
@@ -0,0 +1,19 @@
+// CS0728: Possibly incorrect assignment to `token' which is the argument to a using or lock statement
+// Line: 11
+// Compiler options: -warnaserror
+
+public class Foo
+{
+       public static void Main ()
+       {
+               object token = new object ();
+               lock (token)
+               {
+                       Foo2 (ref token);
+               }
+       }
+       
+       static void Foo2 (ref object o)
+       {
+       }
+}
index f6a248660b193c17c79c5c02bae73e4db0f8df52..8f1b5a1aad19e838027cb3440dd662441f2d2d14 100644 (file)
@@ -5646,6 +5646,14 @@ namespace Mono.CSharp {
                        this.loc = loc;
                }
 
+               public override bool IsLockedByStatement {
+                       get {
+                               return false;
+                       }
+                       set {
+                       }
+               }
+
                public LocalVariable LocalInfo {
                    get {
                        return li;
index 5bdc8fa6c3c72eb29b6ebc583853b0502213a31e..6edfbc7e25cc6d20aa9379def75190af09762e57 100644 (file)
@@ -4448,6 +4448,9 @@ namespace Mono.CSharp {
 
                #region Abstract
                public abstract HoistedVariable GetHoistedVariable (AnonymousExpression ae);
+
+               public abstract bool IsLockedByStatement { get; set; }
+
                public abstract bool IsFixed { get; }
                public abstract bool IsRef { get; }
                public abstract string Name { get; }
@@ -4475,19 +4478,15 @@ namespace Mono.CSharp {
                        Variable.EmitAddressOf (ec);
                }
 
-               public HoistedVariable GetHoistedVariable (ResolveContext rc)
-               {
-                       return GetHoistedVariable (rc.CurrentAnonymousMethod);
-               }
-
-               public HoistedVariable GetHoistedVariable (EmitContext ec)
+               public override Expression DoResolveLValue (ResolveContext rc, Expression right_side)
                {
-                       return GetHoistedVariable (ec.CurrentAnonymousMethod);
-               }
+                       if (IsLockedByStatement) {
+                               rc.Report.Warning (728, 2, loc,
+                                       "Possibly incorrect assignment to `{0}' which is the argument to a using or lock statement",
+                                       Name);
+                       }
 
-               public override string GetSignatureForError ()
-               {
-                       return Name;
+                       return this;
                }
 
                public override void Emit (EmitContext ec)
@@ -4586,6 +4585,22 @@ namespace Mono.CSharp {
                        }
                }
 
+
+               public HoistedVariable GetHoistedVariable (ResolveContext rc)
+               {
+                       return GetHoistedVariable (rc.CurrentAnonymousMethod);
+               }
+
+               public HoistedVariable GetHoistedVariable (EmitContext ec)
+               {
+                       return GetHoistedVariable (ec.CurrentAnonymousMethod);
+               }
+
+               public override string GetSignatureForError ()
+               {
+                       return Name;
+               }
+
                public bool IsHoisted {
                        get { return GetHoistedVariable ((AnonymousExpression) null) != null; }
                }
@@ -4613,11 +4628,24 @@ namespace Mono.CSharp {
                        return local_info.HoistedVariant;
                }
 
+               #region Properties
+
                //              
                // A local variable is always fixed
                //
                public override bool IsFixed {
-                       get { return true; }
+                       get {
+                               return true;
+                       }
+               }
+
+               public override bool IsLockedByStatement {
+                       get {
+                               return local_info.IsLocked;
+                       }
+                       set {
+                               local_info.IsLocked = value;
+                       }
                }
 
                public override bool IsRef {
@@ -4628,6 +4656,8 @@ namespace Mono.CSharp {
                        get { return local_info.Name; }
                }
 
+               #endregion
+
                public bool VerifyAssigned (ResolveContext ec)
                {
                        VariableInfo variable_info = local_info.VariableInfo;
@@ -4650,7 +4680,7 @@ namespace Mono.CSharp {
                        return CreateExpressionFactoryCall (ec, "Constant", arg);
                }
 
-               Expression DoResolveBase (ResolveContext ec)
+               void DoResolveBase (ResolveContext ec)
                {
                        VerifyAssigned (ec);
 
@@ -4670,14 +4700,14 @@ namespace Mono.CSharp {
 
                        eclass = ExprClass.Variable;
                        type = local_info.Type;
-                       return this;
                }
 
                protected override Expression DoResolve (ResolveContext ec)
                {
                        local_info.SetIsUsed ();
 
-                       return DoResolveBase (ec);
+                       DoResolveBase (ec);
+                       return this;
                }
 
                public override Expression DoResolveLValue (ResolveContext ec, Expression right_side)
@@ -4705,7 +4735,9 @@ namespace Mono.CSharp {
                                VariableInfo.SetAssigned (ec);
                        }
 
-                       return DoResolveBase (ec);
+                       DoResolveBase (ec);
+
+                       return base.DoResolveLValue (ec, right_side);
                }
 
                public override int GetHashCode ()
@@ -4753,6 +4785,15 @@ namespace Mono.CSharp {
 
                #region Properties
 
+               public override bool IsLockedByStatement {
+                       get {
+                               return pi.IsLocked;
+                       }
+                       set     {
+                               pi.IsLocked = value;
+                       }
+               }
+
                public override bool IsRef {
                        get { return (pi.Parameter.ModFlags & Parameter.Modifier.ISBYREF) != 0; }
                }
@@ -4914,13 +4955,13 @@ namespace Mono.CSharp {
                        return this;
                }
 
-               override public Expression DoResolveLValue (ResolveContext ec, Expression right_side)
+               public override Expression DoResolveLValue (ResolveContext ec, Expression right_side)
                {
                        if (!DoResolveBase (ec))
                                return null;
 
                        SetAssigned (ec);
-                       return this;
+                       return base.DoResolveLValue (ec, right_side);
                }
 
                static public void EmitLdArg (EmitContext ec, int x)
@@ -6647,6 +6688,14 @@ namespace Mono.CSharp {
                        get { return "this"; }
                }
 
+               public override bool IsLockedByStatement {
+                       get {
+                               return false;
+                       }
+                       set {
+                       }
+               }
+
                public override bool IsRef {
                        get { return type.IsStruct; }
                }
index 9f989954da5485a1e4f028bf848ee3a936444d44..b63b74d539b7841b0c1d804a66f86687ea7d0924 100644 (file)
@@ -59,6 +59,7 @@ namespace Mono.CSharp {
                        219, 251, 252, 253, 278, 282,
                        402, 414, 419, 420, 429, 436, 440, 458, 464, 465, 467, 469, 472,
                        612, 618, 626, 628, 642, 649, 652, 658, 659, 660, 661, 665, 672, 675, 693,
+                       728,
                        809,
                        1030, 1058, 1066,
                        1522, 1570, 1571, 1572, 1573, 1574, 1580, 1581, 1584, 1587, 1589, 1590, 1591, 1592,
index cdbd28f559f075a021d7726b48376dc509273bf9..d57f2d92231084c7c5dbfa48afa1ce989b1e301c 100644 (file)
@@ -1466,6 +1466,7 @@ namespace Mono.CSharp {
                        FixedVariable = 1 << 6,
                        UsingVariable = 1 << 7,
 //                     DefinitelyAssigned = 1 << 8,
+                       IsLocked = 1 << 9,
 
                        ReadonlyMask = ForeachVariable | FixedVariable | UsingVariable
                }
@@ -1505,6 +1506,11 @@ namespace Mono.CSharp {
 
                #region Properties
 
+               public bool AddressTaken {
+                       get { return (flags & Flags.AddressTaken) != 0; }
+                       set { flags |= Flags.AddressTaken; }
+               }
+
                public Block Block {
                        get {
                                return block;
@@ -1544,6 +1550,15 @@ namespace Mono.CSharp {
                        }
                }
 
+               public bool IsLocked {
+                       get {
+                               return (flags & Flags.IsLocked) != 0;
+                       }
+                       set {
+                               flags = value ? flags | Flags.IsLocked : flags & ~Flags.IsLocked;
+                       }
+               }
+
                public bool IsThis {
                        get {
                                return (flags & Flags.IsThis) != 0;
@@ -1656,6 +1671,20 @@ namespace Mono.CSharp {
                        ec.Emit (OpCodes.Ldloca, builder);
                }
 
+               public string GetReadOnlyContext ()
+               {
+                       switch (flags & Flags.ReadonlyMask) {
+                       case Flags.FixedVariable:
+                               return "fixed variable";
+                       case Flags.ForeachVariable:
+                               return "foreach iteration variable";
+                       case Flags.UsingVariable:
+                               return "using variable";
+                       }
+
+                       throw new InternalErrorException ("Variable is not readonly");
+               }
+
                public bool IsThisAssigned (BlockContext ec, Block block)
                {
                        if (VariableInfo == null)
@@ -1695,29 +1724,10 @@ namespace Mono.CSharp {
                        flags |= Flags.Used;
                }
 
-               public bool AddressTaken {
-                       get { return (flags & Flags.AddressTaken) != 0; }
-                       set { flags |= Flags.AddressTaken; }
-               }
-
                public override string ToString ()
                {
                        return string.Format ("LocalInfo ({0},{1},{2},{3})", name, type, VariableInfo, Location);
                }
-
-               public string GetReadOnlyContext ()
-               {
-                       switch (flags & Flags.ReadonlyMask) {
-                       case Flags.FixedVariable:
-                               return "fixed variable";
-                       case Flags.ForeachVariable:
-                               return "foreach iteration variable";
-                       case Flags.UsingVariable:
-                               return "using variable";
-                       }
-
-                       throw new InternalErrorException ("Variable is not readonly");
-               }
        }
 
        /// <summary>
@@ -2274,6 +2284,7 @@ namespace Mono.CSharp {
                        readonly ParametersBlock block;
                        readonly int index;
                        public VariableInfo VariableInfo;
+                       bool is_locked;
 
                        public ParameterInfo (ParametersBlock block, int index)
                        {
@@ -2295,6 +2306,15 @@ namespace Mono.CSharp {
                                }
                        }
 
+                       public bool IsLocked {
+                               get {
+                                       return is_locked;
+                               }
+                               set {
+                                       is_locked = value;
+                               }
+                       }
+
                        public Location Location {
                                get {
                                        return Parameter.Location;
@@ -4090,11 +4110,25 @@ namespace Mono.CSharp {
                                        expr.Type.GetSignatureForError ());
                        }
 
+                       VariableReference lv = expr as VariableReference;
+                       bool locked;
+                       if (lv != null) {
+                               locked = lv.IsLockedByStatement;
+                               lv.IsLockedByStatement = true;
+                       } else {
+                               lv = null;
+                               locked = false;
+                       }
+
                        ec.StartFlowBranching (this);
-                       bool ok = Statement.Resolve (ec);
+                       Statement.Resolve (ec);
                        ec.EndFlowBranching ();
 
-                       ok &= base.Resolve (ec);
+                       if (lv != null) {
+                               lv.IsLockedByStatement = locked;
+                       }
+
+                       base.Resolve (ec);
 
                        //
                        // Have to keep original lock value around to unlock same location
@@ -4111,7 +4145,7 @@ namespace Mono.CSharp {
                                lock_taken.Resolve (ec);
                        }
 
-                       return ok;
+                       return true;
                }
                
                protected override void EmitPreTryBody (EmitContext ec)
@@ -4918,14 +4952,15 @@ namespace Mono.CSharp {
                                return base.Resolve (bc);
                        }
 
-                       public void ResolveExpression (BlockContext bc)
+                       public Expression ResolveExpression (BlockContext bc)
                        {
                                var e = Initializer.Resolve (bc);
                                if (e == null)
-                                       return;
+                                       return null;
 
                                li = LocalVariable.CreateCompilerGenerated (e.Type, bc.CurrentBlock, loc);
                                Initializer = ResolveInitializer (bc, Variable, e);
+                               return e;
                        }
 
                        protected override Expression ResolveInitializer (BlockContext bc, LocalVariable li, Expression initializer)
@@ -5067,9 +5102,16 @@ namespace Mono.CSharp {
 
                public override bool Resolve (BlockContext ec)
                {
+                       VariableReference vr;
+                       bool vr_locked = false;
+
                        using (ec.Set (ResolveContext.Options.UsingInitializerScope)) {
                                if (decl.Variable == null) {
-                                       decl.ResolveExpression (ec);
+                                       vr = decl.ResolveExpression (ec) as VariableReference;
+                                       if (vr != null) {
+                                               vr_locked = vr.IsLockedByStatement;
+                                               vr.IsLockedByStatement = true;
+                                       }
                                } else {
                                        if (!decl.Resolve (ec))
                                                return false;
@@ -5077,18 +5119,23 @@ namespace Mono.CSharp {
                                        if (decl.Declarators != null) {
                                                stmt = decl.RewriteForDeclarators (ec, stmt);
                                        }
+
+                                       vr = null;
                                }
                        }
 
                        ec.StartFlowBranching (this);
 
-                       bool ok = stmt.Resolve (ec);
+                       stmt.Resolve (ec);
 
                        ec.EndFlowBranching ();
 
-                       ok &= base.Resolve (ec);
+                       if (vr != null)
+                               vr.IsLockedByStatement = vr_locked;
 
-                       return ok;
+                       base.Resolve (ec);
+
+                       return true;
                }
 
                protected override void CloneTo (CloneContext clonectx, Statement t)