Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSC doesn't care if out parameter not assigned #24493

Closed
carlreinke opened this issue Jan 27, 2018 · 12 comments
Closed

CSC doesn't care if out parameter not assigned #24493

carlreinke opened this issue Jan 27, 2018 · 12 comments
Labels
Area-Compilers Blocked Bug Need More Info The issue needs more information to proceed.

Comments

@carlreinke
Copy link
Contributor

carlreinke commented Jan 27, 2018

Version Used: Microsoft (R) Visual C# Compiler version 2.6.0.62329 (5429b35)

Steps to Reproduce:

c:\temp>type Program.cs
class C
{
    static bool M( out System.ReadOnlyMemory<byte> x )
    {
        return false;  // No error here!?
    }
}

c:\temp>csc.exe /reference:"C:\Program Files\dotnet\sdk\NuGetFallbackFolder\microsoft.netcore.app\2.0.0\ref\netcoreapp2.0\mscorlib.dll" /reference:"C:\Program Files\dotnet\sdk\NuGetFallbackFolder\microsoft.netcore.app\2.0.0\ref\netcoreapp2.0\netstandard.dll" /reference:C:\Users\User\.nuget\packages\system.memory\4.5.0-preview1-26011-01\ref\netstandard2.0\System.Memory.dll /reference:"C:\Program Files\dotnet\sdk\NuGetFallbackFolder\microsoft.netcore.app\2.0.0\ref\netcoreapp2.0\System.Runtime.dll" /out:Test.dll /target:library Program.cs
Microsoft (R) Visual C# Compiler version 2.6.0.62329 (5429b35d)
Copyright (C) Microsoft Corporation. All rights reserved.


c:\temp>

Expected Behavior: Show error message and fail to compile.

Actual Behavior: Compilation succeeds.

@gafter
Copy link
Member

gafter commented Jan 27, 2018

Please check to see what fields, if any, are contained in System.Collections.Generic.KeyValuePair inside of C:\Program Files\dotnet\sdk\NuGetFallbackFolder\microsoft.netcore.app\2.0.0\ref\netcoreapp2.0\System.Runtime.dll. If it contains no fields in that reference assembly, then the compiler is correct. In that case the out parameter is definitely assigned because all (zero) of its fields are definitely assigned.

@gafter gafter added Bug Area-Compilers Blocked Need More Info The issue needs more information to proceed. labels Jan 27, 2018
@gafter
Copy link
Member

gafter commented Jan 27, 2018

To clarify, if that is the case then it is a bug in the reference assembly.

@carlreinke carlreinke changed the title CSC doesn't care if struct-type out parameter not assigned if compiling with /nostdlib+ CSC doesn't care if out parameter not assigned Jan 27, 2018
@carlreinke
Copy link
Contributor Author

carlreinke commented Jan 27, 2018

Indeed, there are no fields in KeyValuePair<TKey, TValue> in the reference assembly. I replaced the original comment with a different example. ReadOnlyMemory<T> does have fields.

@carlreinke
Copy link
Contributor Author

carlreinke commented Jan 27, 2018

Err, I guess the reference assembly for ReadOnlyMemory<T> doesn't have fields either... the assembly that gets used when it actually runs has fields; otherwise it wouldn't work.

Why are there no fields in the reference assemblies?

(IntelliSense/MSBuild seem to also use the ref assembly, so when I'm building in Visual Studio, I don't get any error message about the out parameter not being assigned there either.)

@gafter
Copy link
Member

gafter commented Jan 27, 2018

That is a bug. We’ll move this bug to an appropriate repository.

@tannergooding
Copy link
Member

@gafter, I believe the issue has already been dealt with on the CoreFX side: dotnet/corefx#26286 and dotnet/buildtools#1859.

However, it was only merged 15 days ago, so nothing has shipped with the fix yet.

@gafter
Copy link
Member

gafter commented Jan 28, 2018

@tannergooding I don't see anything specifically changing KeyValuePair's reference assembly. Isn't its reference assembly produced based on hand-written source?

@gafter gafter closed this as completed Jan 28, 2018
@gafter
Copy link
Member

gafter commented Jan 28, 2018

Issue moved to dotnet/corefx #26624 via ZenHub

@tannergooding
Copy link
Member

@gafter
Copy link
Member

gafter commented Jan 29, 2018

And the fix for KeyValuePair?

@tannergooding
Copy link
Member

I wasn't able to find the contract assembly for KVP and commented as such on the new issue (forgot to comment as such here as well).

@tannergooding
Copy link
Member

Nevermind, it's in System.Runtime and is also correct: https://github.com/dotnet/corefx/blob/master/src/System.Runtime/ref/System.Runtime.cs#L3907

(GitHub search wouldn't find it, git grep did).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Blocked Bug Need More Info The issue needs more information to proceed.
Projects
None yet
Development

No branches or pull requests

3 participants