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
19.05k stars 4.04k forks source link

Dev nuget Package Microsoft.Net.Compilers.Toolset v 3.8.0-1.20325.3 when using `init` feature in c# 9 raise error: Predefined type 'System.Runtime.CompilerServices.IsExternalInit' is not defined or imported #45510

Closed moh-hassan closed 4 years ago

moh-hassan commented 4 years ago

Version Used: Dev nuget package: Microsoft.Net.Compilers.Toolset v 3.8.0-1.20325.3

Steps to Reproduce: 1- Install Net5 Preview 5 SDK. 2- Create a console project and add the next class to use c# 9 init feature

public class Person
    {        
        public string FirstName { get; init; }
        public string LastName { get; init; }         
    }

3- Add a reference to Microsoft.Net.Compilers.Toolset to the project. .NET Core SDK Net5 Preview 5 is not supporting init of C# 9 feature yet.

<ItemGroup>     
    <PackageReference Include="Microsoft.Net.Compilers.Toolset" Version="3.8.0-1.20325.3">
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
      <PrivateAssets>all</PrivateAssets>
    </PackageReference>
  </ItemGroup>

4- build

dotnet build

Expected Behavior: Build success.

Actual Behavior: The compiler raise error

error CS0518: Predefined type 'System.Runtime.CompilerServices.IsExternalInit' is not defined or imported

5- Add this dummy class to the project:

namespace System.Runtime.CompilerServices
{
    public sealed class IsExternalInit
    {
    }
}

6- build again Build success and application run sucessfully.

Is IsExternalInit a missing class? If IsExternalInit class has no role remove any calling to it.

joshlang commented 4 years ago

Ahhh, I just posted a similar question here:

https://stackoverflow.com/questions/62648189/testing-c-sharp-9-0-in-vs2019-cs0518-isexternalinit-is-not-defined-or-imported

(I am using preview6 sdk though)

pkanavos commented 4 years ago

I've encountered the same problem, and even the latest master branch (June 27) in Sharplab.io generates the same error. I've used the same fix.

jaredpar commented 4 years ago

Thanks for reporting the issue. This is not a bug in the compiler. The IsExternalInit type is a new type added to .NET in the net5 time frame which is necessary to properly emit record types and init properties. That type was added but did not make it into the .NET 5 SDK you used. Once you upgrade to a version which has the type, believe Preview 7, this will go away.

holc-tkomp commented 4 years ago

(SDK Preview 8 + VS Pro 16.8 Preview 2.0)

This works fine now for net5.0 target framework, but if trying to target any of the net461, net472, net48, netcoreapp2.1, netcoreapp3.1, netstandard2.0, netstandard2.1 error will still be thrown.

If dummy class is added, the other tfms works fine.

kzu commented 4 years ago

Trivial workaround: just declare the missing type in your project:

// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.ComponentModel;

namespace System.Runtime.CompilerServices
{
    /// <summary>
    /// Reserved to be used by the compiler for tracking metadata.
    /// This class should not be used by developers in source code.
    /// </summary>
    [EditorBrowsable(EditorBrowsableState.Never)]
    public static class IsExternalInit
    {
    }
}

If you're multitargeting net5.0 and other TFMs, just add this bit of MSBuild to remove it for net5.0 since it's built-in:

<ItemGroup>
  <Compile Remove="IsExternalInit.cs" Condition="'$(TargetFramework)' == 'net5.0'" />
</ItemGroup>
BoasE commented 4 years ago

I stil have this within net standard 2.1 projects with the release from today. an anyone confirm?

hey-red commented 4 years ago

@BoasE I can confirm that. Edit: As I can see it's not a bug https://developercommunity.visualstudio.com/content/problem/1244809/error-cs0518-predefined-type-systemruntimecompiler.html

More about target framework https://docs.microsoft.com/en-us/dotnet/standard/net-standard#when-to-target-net50-vs-netstandard

brantburnett commented 4 years ago

I recommend adding this file to your project, based on the example above from @kzu. It will handle multi-targeting and marks the attribute as internal to avoid conflicts with other packages.

// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#if NETSTANDARD2_0 || NETSTANDARD2_1 || NETCOREAPP2_0 || NETCOREAPP2_1 || NETCOREAPP2_2 || NETCOREAPP3_0 || NETCOREAPP3_1 || NET45 || NET451 || NET452 || NET46 || NET461 || NET462 || NET47 || NET471 || NET472 || NET48

using System.ComponentModel;

// ReSharper disable once CheckNamespace
namespace System.Runtime.CompilerServices
{
    /// <summary>
    /// Reserved to be used by the compiler for tracking metadata.
    /// This class should not be used by developers in source code.
    /// </summary>
    [EditorBrowsable(EditorBrowsableState.Never)]
    internal static class IsExternalInit
    {
    }
}

#endif
igabesz commented 4 years ago

Thanks for the workaround. Still feels like a hack though.

I doubt that it should be the responsibility of the developer to provide internal system classes to make the compiler work properly...

brantburnett commented 4 years ago

Thanks for the workaround. Still feels like a hack though.

I doubt that it should be the responsibility of the developer to provide internal system classes to make the compiler work properly...

I generally agree, but this isn't the first feature like this. A similar (and larger) workaround file is necessary for nullable reference types on older frameworks. I know some language features aren't really usable on older versions of the framework, but this one is. Feels like Microsoft could publish a NuGet package that gives us what we need for older frameworks, much like was done with IAsyncEnumerable in Microsoft.Bcl.AsyncInterfaces. It could probably even be a source code package instead of a binary package. I see a couple of packages from 3rd parties that may do this, but I'd prefer something official and maintained.

jaredpar commented 4 years ago

I doubt that it should be the responsibility of the developer to provide internal system classes to make the compiler work properly...

This is a C# 9 feature and hence only officially supported when targeting net5.0 or higher. The IsExternalInit type is defined there and hence requires no extra work by developers. When targeting other TFs the developer is required to provide the type.

This is how C# has functioned for many features over the years. Extension methods are a very analogous example here. Unless you target a target framework with the suporting ExtensionAttribute type you must define it yourself.

kzu commented 4 years ago

What I added to my approach is bit of msbuild:

<ItemGroup>
  <Compile Remove="IsExternalInit.cs" Condition="'$(TargetFramework)' == 'net5.0'" />
</ItemGroup>
moh-hassan commented 4 years ago

@jaredpar

This class should not be used by developers in source code.

But we have to use the class with Microsoft.Net.Compilers.Toolset in netcore 3.1 to support c# 9. I checked the version 3.8.0-5.final and also it is not available. I think the above phrase should be removed from the source and it's better if the missed class is added to Microsoft.Net.Compilers.Toolset.

tullo-x86 commented 3 years ago

@brantburnett

That stray NET6 in your preprocessor directive is going to surprise all of the people who blindly copy + paste code from the Internet.

brantburnett commented 3 years ago

@brantburnett

That stray NET6 in your preprocessor directive is going to surprise all of the people who blindly copy + paste code from the Internet.

@tullo-x86 Thanks for pointing that out, fixed.

Jorenkv commented 3 years ago

@brantburnett Wouldn't it be enough to just do #if !NET since NET is defined in .NET 5 or later?

brantburnett commented 3 years ago

@Jorenkv Yes, that would also work. Personally, I like this more declarative approach a bit better, it makes it a bit more clear where it's used. But that's just a preference.