# C# Language Design Meeting for August 23rd, 2021

## Agenda

1. [Nullability differences in partial type base clauses](#nullability-differences-in-partial-type-base-clauses)
2. [Top-level statements default type accessibility](#top-level-statements-default-type-accessibility)
3. [Lambda expression and method group type inference issues](#lambda-expression-and-method-group-type-inference-issues)
    1. [Better function member now ambiguous in some cases](#better-function-member-now-ambiguous-in-some-cases)
    2. [Conversions from method group to `object`](#conversions-from-method-group-to-object)
4. [Interpolated string betterness in older language versions](#interpolated-string-betterness-in-older-language-versions)

## Quote of the Day

- This is the thing that's not weird, so let's handle that

## Discussion

### Nullability differences in partial type base clauses

https://github.com/dotnet/csharplang/issues/5107

We looked at inconsistencies and errors when base and interface clauses differ by nullability today. This is particularly problematic
for source generators, as generated files are always `#nullable disable`d by default, and if the author just copied the class header
over it can cause a compilation error if the original declaration had type parameters and was `#nullable enable`d. The scenarios with
base types and interfaces are similar but not identical, as interfaces are allowed to be duplicated in metadata, but base types are not.
While the compiler is resilient to importing types with this duplicate metadata, we're not certain that relaxing anything with regards
to interface deduplication would be good for the whole ecosystem, as not every tool is written with the same level of error-recovery
as Roslyn is. We'll need to look more into the interface scenario before making any more changes.

For base types, the proposal suggests that we report warnings when there are differences between nullable and non-nullable type
parameters, but not for any other scenario. However, we think that this is a bit too big of an exception. As an example:

```cs
#nullable enable
public partial class Derived : Base<
    object,
#nullable disable
    object
#nullable enable
> {}

#nullable enable
public partial class Derived : Base<
#nullable disable
    object,
#nullable enable
    object
> {}
```

The type parameters only differ by obliviousness in this code, but we'd have to do a merge of all the declarations to get the "true"
nullability of all type parameters. We think this is an extremely complex case that gets away from the root of the problem: one
declaration is entirely oblivious. Therefore, a narrower, less complex compromise is to simply say that any entirely-oblivious base
type declarations are ignored when checking to see if the nullability of base types on all partial parts matches. This carves out the
simple case for source generators, and leaves the complex case for manual authors to get correct.

#### Conclusion

Entirely oblivious base type declarations are ignored when ensuring the nullability of all base type clauses in a partial type match.

### Top-level statements default type accessibility

ASP.NET has been testing the changes with [speakable types in top-level statements](https://github.com/dotnet/csharplang/blob/dcbaa815253df779d1ecc206c446c9eb6b059b82/meetings/2021/LDM-2021-07-26.md#speakable-names-for-top-level-statements),
and some initial feedback has been that, because the default for types in C# is internal, it requires `InternalsVisibleTo` for all test
projects. However, we don't think that this case is important enough to complicate the feature: making it `public` by default is different
than anything else in C#, and makes it a much more complex feature to specify, explain, and implement. We also think that the likelihood
is that most test projects are going to need IVT to their original at some point, so we wouldn't be saving much in the long run by making
`Program` public by default anyway.

#### Conclusion

Decision from the last meeting stands.

### Lambda expression and method group type inference issues

We looked at issues from https://github.com/dotnet/csharplang/issues/5095. The first item we determined to be an implementation bug before
the meeting and skipped it.

#### Better function member now ambiguous in some cases

Another entry in the continuing saga of breaking changes we're discovering from giving lambdas a natural type is a new way in which functions
that were previously unambiguous for lambda expressions are now ambiguous:

```cs
static void F(object obj, Action callback) { }
static void F(int i, Delegate callback) { }

// C#9: F(object, Action)
// C#10: ambiguous
F(0, () => { });
```

The reason for this is because, in C# 9, the `M(int, Delegate)` overload was not applicable at the call site, and only the `M(object, Action)`
was. However, with lambda expressions now having a natural type and being convertible to `Delegate`, we have 2 applicable overloads, with the
following conversions:

* `M(object, Action)`: implicit boxing numeric conversion from `int` to `object`, identity conversion from `Action` to `Action`.
* `M(int, Delegate)`: identity conversion from `int` to `int`, implicit reference conversion from `Action` to `Delegate`.

Neither of these sets of conversions is better than the other, so the method is now ambiguous. This was reported from ASP.NET, as they have
an unfortunate set of shipped methods that can hit this problem:

```cs
static class AppBuilderExtensions
{
    public static IAppBuilder Map(this IAppBuilder app, PathSring path, Action<IAppBuilder> callback) => app;
}
static class RouteBuilderExtensions
{
    public static IRouteBuilder Map(this IRouteBuilder routes, string path, Delegate callback) => routes;
}
```

where both `IAppBuilder` and `IRouteBuilder` are defined on `WebApp` instances. When a user calls `app.Map("sub", (IAppBuilder b) => {})`, this
is now ambiguous because each overload has 2 implicit non-identity conversions and 1 identity conversion, and the overloads are ambiguous.

We note, however, that these overloads are _already_ ambiguous if, instead of passing a method group or a lambda expression, the user instead
passes a variable of type `Action<IAppBuilder>`, with the same ambiguity. There is also a fairly easy workaround for ASP.NET, as they can add a
most-specific extension method of `Map(this IAppBuilder, string, Action<IAppBuilder>)`, which will have 2 identity conversions and 1 reference
conversion, making it more specific than the other two. We think this is a fine workaround, especially since this API is already ambiguous in
many cases, and will not be making any changes here.

##### Conclusion

No changes.

#### Conversions from method group to `object`

Allowing method groups to be convertible to `object` has a potentially deleterious effect on this simple scenario:

```cs
static object GetValue() => 0;
object o = GetValue;
```

In C# 9, the user would get an error stating that method groups cannot be converted to `object`. In C# 10, this is legal code and will compile.
However, we think the likelihood of this being intentional is much lower than the likelihood of it being a mistype. We have a few options for
addressing this:

- Require an explicit cast to object for such cases.
- Let an analyzer detect this case.
- Accept it as is.
- Have the compiler warn for this case.

We think that hard-requiring a cast to `object` is too prescriptive here, as this code is legal and well-formed. However, we also think that
either leaving it as is or leaving it to some other analyzer to detect this case is not ideal either. Given that, we think we should warn when
a method group is _implicitly_ converted to `object`. Suppressing the warning via an explicit cast or via a standard warning suppression
mechanism should be enough to ensure that there is clarity for these cases.

##### Conclusion

The compiler will warn on implicit method group conversions to `object`.

### Interpolated string betterness in older language versions

https://github.com/dotnet/roslyn/issues/55345

Finally today, we looked at a bug reported by a user when upgrading the compiler and target framework, but setting `LangVersion` back to 9 or
earlier. This causes an error for common calls such as `stringBuilder.Append($"")`, as they now pick the interpolated string handler overload
instead of the string overload, which then causes a language version error. We solved this similarly to how we dealt with lambda expressions,
where we made the better conversion code conditional on whether the language version supported interpolated string handlers. LDM had no issues
with this decision.

#### Conclusion

Decision upheld.
