Confusing errors for a confusing feature, part three

One of the nice things about a project as large as the Roslyn project is that you have an opportunity to really think hard about your past mistakes and hopefully fix them. When I was working on getting these error messages reported in Roslyn I realized that trying to match exactly the behavior of the original compiler would be actively making the world a worse place, so I took a big step back and started sending a lot of emails to Mads, Neal, Anthony and the rest of the Roslyn gang to try and get a better design worked out. All the godawful nonsense I told you about in the previous two episodes will be fixed in Roslyn.

Let’s go back to our original, simplest case: a local variable shadows another:

class C
{
  static void M1()
  {
    int x2;
    {
      int x2;
    }
  }

Recall that the pre-Roslyn compiler gave the error on the inner x2:

error CS0136: A local variable named 'x2' cannot be declared in
this scope because it would give a different meaning to 'x2', which is
already used in a 'parent or current' scope to denote something else

Roslyn will give the error (again, on the inner x2)

error CS0136: A local or parameter named 'x2' cannot be declared in
this scope because that name is used in an enclosing local scope to
define a local or parameter

OMG so much better. I am still not thrilled with the “local or parameter” there; Roslyn knows which it was and could say so, but hey, this is so much more understandable that let’s not quibble. Note that the error still doesn’t say what we’re conflicting with, but what would be the point? We’re conflicting with another local or parameter, so all it would be able to say is “a local or parameter named ‘x2′” and we already know that. Note also that “enclosing” is not in quotes, and that’s because it is always the inner of the two conflicting definitions that gets the error. As I mentioned before, that is the one that is likely to need to change, so that’s where Roslyn should consistently report the error no matter whether the outer declaration is lexically before or after the inner one. The next most likely case is that a local conflicts with a member:

class C
{
  static int x8;
  static void M9()
  {
    {
      int x8;
    }
    int x10 = x8;
  }

Recall that the pre-Roslyn compiler gives this awful error on the initialization of x10:

error CS0135: 'x8' conflicts with the declaration 'C.x8'

Roslyn will give this error on the declaration of the inner local x8:

error CS0135: A local, parameter or range variable named 'x8' cannot be 
declared in this scope because that name is used in an enclosing local
scope to refer to 'C.x8' 

Again, I am not thrilled with the “local, parameter or range variable”, but let’s not quibble; this sentence is actually understandable by mortal beings, it tells you what the conflict is, and it identifies the line that must change. Since I’ve reworded both CS0135 and CS0136 to refer specifically to locals, we now no longer have an error message for the rare case where a name has two meanings and neither is a local:

class C
{
  static void x11();
  class N
  {
    static int x11;
    static void M12()
    {
      x11();
      int x13 = x11;
    }
  }

Again, the pre-Roslyn compiler produces

error CS0135: 'x11' conflicts with the declaration 'C.N.x11'

Roslyn will produce a new error:

error CS7039: 'x11' cannot be used in this local scope because 
that name has been used to refer to method 'C.x11()'

Identifying that the conflict is a method I thought was a nice touch. It would have been nice to also call out what the other x11 refers to; that would be a good feature for someone to add to Roslyn.

Finally, one more new error message for the case where the conflicting names are in different (but nested) scopes:

class C
{
  static void x11();
  class N
  {
    static int x11;
    static void M12()
    {
      x11();
      {
        int x13 = x11;
      }
    }
  }
error CS7040: 'x11' cannot be used in this local scope because 
that name has been used in an enclosing scope to refer to 
method 'C.x11()'

I said that I’d be exploring this feature at absurd depth; surprisingly, there is still farther we could go here. The interaction of this feature with the “Color Color” feature — which has the opposite behavior of allowing simple name Color to refer to both a type and a member — is quite interesting. But I think I will leave it here for now and come back to Color Color another day.

9 thoughts on “Confusing errors for a confusing feature, part three

  1. It’s not uncommon for the outer declaration to be the “new” party to the conflict. Consider:

    if (condition1)
    {
      int temp=whatever; ...
    }
    if (condition2)
    {
      int temp=whatever; ...
    }
    if (condition3)
    {
      int temp=whatever; ...
    }
    

    If one of the conditions turns out to always be true, factoring out the “if” and its associated scope will cause an error, but the fault would be with the outer scope declaration, not the inner ones.

    Actually, changing the outer scope declaration when there’s a conflict could be advantageous regardless of who’s “at fault”: changing either declaration would require that all associated references be changed to match, but the compiler will catch an accidental failure to change an outer-scope reference to match its new declaration. The compiler won’t squawk at a failure to correct an inner-scope reference to match what’s supposed to be its declaration; it will just bind it to the outer scope definition.

  2. Pingback: The Morning Brew - Chris Alcock » The Morning Brew #1708

  3. I’d prefer to see error on the latest line, regardless is it outer or inner scope.
    It’s because you usually write the code downwards, so it’s more likely, that you’ve made a collision writing further line of parent scope, and would fix it there, but not in a child scope above.

  4. I think everyone will have different opinions on which line should hold the error (inner most, latest line, first line etc), and the answer as to which is more useful is often going to be on a case by case basis anyway.

    I’d argue that since the error is that they are conflicting then both lines are in error. This also lets you see the two definitions without having to look for the second definition.

    • It seems nice, but isn’t it a way leading to tons of errors in various places, but caused by adding of a single line?

      • True but so doss typing a statement and giving a shirt pause before finishing it and adding a semicolon.

        This way you are highlighted to the error as you type it as the statement you are on is marked regardless of which position it is in and one extra exception isn’t the end of the world when it’s so easily noticed and fixed.

    • So long as it does not mean reporting two errors where there is one, I agree: just phrase the message so that it mentions both lines and (stronlgy) suggests you fix at least one of them. But then I’ve always been a bit of a freedom fighter and would rather give language users as much freedom as possible, while the vast majority of my colleagues would prefer strict rules that give as little as possible. 🙂

  5. Here is another interesting thing:

    static class Program
    {
    static int x1 = 0;
    static void Main(string[] args)
    {
    int x1 = 1; //remove ‘int’, it still compiles, obviously, x1 has two meanings
    }
    }

  6. Pingback: Confusing errors for a confusing feature, part two | Fabulous adventures in coding

Leave a comment