Bug 317420 (MONO73485) - [ANONYMOUS METHODS] Flow analysis missing on anonymous methods.
Summary: [ANONYMOUS METHODS] Flow analysis missing on anonymous methods.
Status: RESOLVED FIXED
Alias: MONO73485
Product: Mono: Compilers
Classification: Mono
Component: C# (show other bugs)
Version: unspecified
Hardware: Other Other
: P3 - Medium : Minor
Target Milestone: ---
Assignee: Marek Safar
QA Contact: Mono Bugs
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-03-08 22:42 UTC by Daniel Silva
Modified: 2013-11-25 15:09 UTC (History)
0 users

See Also:
Found By: ---
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Wiest 2007-09-15 19:10:18 UTC


---- Reported by dsilva@ccs.neu.edu 2005-03-08 15:42:51 MST ----

Please fill in this template when reporting a bug, unless you know what you
are doing.
Description of Problem:

Inconsistent C# compiler behavior (gmcs).

Steps to reproduce the problem:

This program compiles with no warnings:

public class Test1 {
  public static void Main(string[] args) {
    check_t check = delegate (object[] instrs, int MC) {
      if (MC < instrs.Length) {
        object instr = instrs[MC];
        System.Console.WriteLine("Hello world: {0}", instr);
        check(instrs, MC + 1);
      }
    };
    check(new object[]{"foo", "bar"}, 0);
  }

  delegate void check_t(object[] instrs, int MC);
}

This equivalent program does not compile:

public class Test2 {
  public static void Main(string[] args) {
    // the following line is the only change
    object[] instrs = new object[]{"one", "two"};
    check_t check = delegate (object[] instrs, int MC) {
      if (MC < instrs.Length) {
        object instr = instrs[MC];
        System.Console.WriteLine("Hello world: {0}", instr);
        check(instrs, MC + 1);
      }
    };
    check(new object[]{"foo", "bar"}, 0);
  }

  delegate void check_t(object[] instrs, int MC);
}

Actual Results:

$ gmcs Test2.cs
Test2.cs(9) error CS0103: The name `check' could not be found in `Test2'
Compilation failed: 1 error(s), 0 warnings



---- Additional Comments From rharinath@novell.com 2005-03-09 06:56:38 MST ----

This is reproducible with MCS too.  If I move the 'object[] instrs'
line below the anonymous delegate, it works again.  That suggests to
me that somewhere, the code isn't handling implicit blocks.



---- Additional Comments From miguel@ximian.com 2005-04-19 02:52:22 MST ----

Just to track the problem.

This is a simpler program:

public class Test1 {
  public static void Main() {
    int a // = 1;
    check_t m = delegate () {
        m ();
    };
  }

  delegate void check_t();
}

The above compiles, if we remove the // from that program it will fail.

The problem is in the lookup process for locals, currently we have
linked two TopLevelBlocks like this:

Delegate.TopLevelBlock.Parent = Main.TopLevelBlock

But it should be:

Delegate.ToplevelBlock.Parent = Main.ToplevelBlock.FirstDeclaration

With IDs:

1 public class Test1 {
2  public static void Main() {
3    int a = 1;
4    check_t m = delegate () {
5        m ();
6    };
7  }

2: the first toplevel block
3: implicit block created for the assignemtn of 1 to a.
4: second toplevel block, with its parent pointing to `2'

Have not found where the linking gets messed up yet.







---- Additional Comments From rharinath@novell.com 2005-04-19 04:45:00 MST ----

I think it may be using ToplevelBlock.Container instead of
ToplevelBlock.Parent.



---- Additional Comments From miguel@ximian.com 2005-04-19 21:11:55 MST ----

This is a bug in mcs, but for the wrong reasons.

The program pasted here is actually an invalid program.  Will explain
in a second.

* Currently:

So the situation is that when you insert the variable declaration
before the delegate, what happens is this:

the "delegate () { }" is parsed in its entirety, creating internally a
ToplevelBlock, and as part of this process, the new toplevel block is
assigned a Parent that points to the current_block (at this point this
is the Main toplevel block.

Then, once the "m = delegate {}" is parsed, we call the
"declare_local_variables" that will actually introduce "m" into the scope.

We introduce variables into the scope by creating a new implicit block
, and this is where the delegate ToplevelBlock should have pointed to
if it is to resolve the name "m".

So that is the bug source at this point.  Fixing it would require us
to either re-link the parent at declare_local_variables.  Ideally we
would create implicit blocks as the variables are declared, but the
way our compiler works, I think that would be a world of pain (just
because these blocks would be created from within expression parsing,
am not thrilled about that).

* Microsoft's

Microsoft is doing flow analysis and in both cases (the working and
failing cases) CSC reports `Use of unassigned variable WWWWWW'.

* Short term fix:

The user test case should be fixed to decouple the declaration from
the initialization, like this:

check_t m = null;
m = delegate (...) { m (); }

Notice that `m' is assigned in this case.

So the bug is that mcs should now warn that the variable is not
definitely assigned (it could be argued that it is, but software
compiled with our compiler would not compile on MS).

I think we need the help of Martin here to sort out the flow analysis
issues.  Sadly when he helped me, he helped me on the old codebase of
anonymous methods, and in the rewrite I lost some of it.

We still would have to fix the problem indicated before just to get
flow analysis working correctly, but it is a lower priority issue.





---- Additional Comments From martin@ximian.com 2005-05-02 14:44:21 MST ----

Miguel, are you working on this or should I have a look ?



---- Additional Comments From miguel@ximian.com 2005-05-13 01:28:13 MST ----

Passing to Martin.



---- Additional Comments From martin@ximian.com 2005-05-17 11:44:39 MST ----

So the bug in here is that we are not reporting the CS0165, right ?
I think that's easy, I'll have a look at it tomorrow.




---- Additional Comments From martin@ximian.com 2005-05-17 11:48:32 MST ----

I think the reasoning behind the CS0165 is to be consistent with

=====
public class X
{
	public static X Foo (X x)
	{
		return x;
	}

	public static void Main()
	{
		X x = Foo (x);
	}
}
====



Unknown operating system unknown. Setting to default OS "Other".

Comment 2 Susanne Oberhauser-Hirschoff 2011-07-20 12:29:27 UTC
Reassigning to Miguel to drive further resolution...
Comment 3 Marek Safar 2013-11-25 15:09:10 UTC
Fixed