dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.95k stars 4.02k forks source link

LanguageVersion errors should not result in cascaded binding errors #20120

Open jcouv opened 7 years ago

jcouv commented 7 years ago

When binding the default value for parameter x in void M(int x = default) {...} with LangVersion=7, "default" ends up bound as a BoundBadExpression. That's because of the logic below, which is sensitive to errors in the syntax tree. The proposal is to relax this logic, so that it ignores errors produced by LangVersion checks.

@AlekseyTs can comment on the importance/priority of fixing this.

Relates to https://github.com/dotnet/roslyn/pull/20077#discussion_r120985019

        internal bool CheckValueKind(BoundExpression expr, BindValueKind valueKind, DiagnosticBag diagnostics)
        {
            if (expr.HasAnyErrors)
            {
                return false;
            }
            ...
        }

        private BoundExpression CheckValue(BoundExpression expr, BindValueKind valueKind, DiagnosticBag diagnostics)
        {
            ...
            if (!hasResolutionErrors && CheckValueKind(expr, valueKind, diagnostics) ||
                expr.HasAnyErrors && valueKind == BindValueKind.RValueOrMethodGroup)
            {
                return expr;
            }

            var resultKind = (valueKind == BindValueKind.RValue || valueKind == BindValueKind.RValueOrMethodGroup) ?
                LookupResultKind.NotAValue :
                LookupResultKind.NotAVariable;

            return ToBadExpression(expr, resultKind);
        }
AlekseyTs commented 7 years ago

I suggest we relax implementation of HasAnyErrors helper to ignore errors caused by language version mismatch.

gafter commented 7 years ago

We should probably move language version checks out of the parser. However, when the language version affects the behavior of the parser (i.e. changes the tree produced) that should remain in the parser.

jcouv commented 7 years ago

FYI @TyOverby since he's currently making changes in that area.

TyOverby commented 7 years ago

Repro:

using System;
public class C {
    public void M(int x = default) {
    }
}
TyOverby commented 7 years ago

Blocked on https://github.com/dotnet/roslyn/pull/19705