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

Defencive copy does not produce warning #43490

Closed
JesOb opened this issue Apr 19, 2020 · 9 comments
Closed

Defencive copy does not produce warning #43490

JesOb opened this issue Apr 19, 2020 · 9 comments
Labels
Area-Compilers Resolution-By Design The behavior reported in the issue matches the current design

Comments

@JesOb
Copy link

JesOb commented Apr 19, 2020

Version Used:
Latest from latest VS 3.5.0-beta2-20057-04 (f43cfcd)
Steps to Reproduce:

compile this code

using System;

namespace _2222222222222222
{
	class Program
	{
		static void Main()
		{
			Console.WriteLine("Hello World!");
		
			Fff sad = new Fff( "sdasdasd" );

			Foo( sad );
		}
		public static Fff Field;

		public static ref readonly Fff Foo( )
		{
			return ref Field;
		}
		public static void Foo( in Fff sd )
		{
			Console.WriteLine(sd.Dad( 2 )); // there must be warning

			Console.WriteLine(sd.Ddd2);

			Console.WriteLine( "__" + Foo( ).Dad( 56 ) );//the same there
		}
	}



	struct Fff
	{
		public Fff( String ddd )
		{
			Ddd = ddd;
			Ddd2 = "wae";
		}

		public readonly String Ddd;
		public String Ddd2;

		public String Dad( Int32 re )
		{
			Ddd2 = "qqqqqqqqqq";

			return re + Ddd2 + "__";
		}
	}
}

Expected Behavior:
I must have warning about defencive copy of struct

Actual Behavior:
No warning

@JesOb JesOb changed the title Defencive copy does not produce warning [Bug]Defencive copy does not produce warning Apr 19, 2020
@CyrusNajmabadi
Copy link
Member

I must have warning about defencive copy of struct

You can write an analyzer for this purpose. :)

@sharwell Were you working on a code-analyzer to catch anything like this?

@sharwell sharwell changed the title [Bug]Defencive copy does not produce warning Defencive copy does not produce warning Apr 19, 2020
@sharwell
Copy link
Member

sharwell commented Apr 19, 2020

The C# language does not include a concept for non-copyable value types. See dotnet/csharplang#859 for a feature request to add this concept.

I have been working on an analyzer for this in dotnet/roslyn-analyzers#3420. Another potential option is ufcpp/NonCopyableAnalyzer.

Keep in mind this is a very difficult analyzer to get correct, since the language itself allows these copies anywhere it sees fit.

@sharwell sharwell added Resolution-By Design The behavior reported in the issue matches the current design Area-Compilers labels Apr 19, 2020
@JesOb
Copy link
Author

JesOb commented Apr 20, 2020

This is all incorrect you know :)

C# 8 out with warning for implicit defensive copies of structs. And it work some times
I dont need non-copyable value types. I just state that feature that suppose to work dont work actually.

see https://docs.microsoft.com/en-us/dotnet/csharp/whats-new/csharp-8#using-declarations
The compiler warns you when it needs to create a defensive copy.

May be issue is in C# docs.

@CyrusNajmabadi
Copy link
Member

I just state that feature that suppose to work dont work actually.

Could you clarify? The compiler/lang is working as spec'ed as far as i can tell. Can you be more specific about what you think is wrong here?

Note that the link you mentioned is specifically about *readonly* member. i.e.:

Notice that the readonly modifier is necessary on a read-only property. The compiler doesn't assume get accessors don't modify state; you must declare readonly explicitly.

In your case, your members aren't readonly so no warning is expected.

@JesOb
Copy link
Author

JesOb commented Apr 20, 2020

In may case member sd is readonly because entire struct is readonly
And in second example return type of method Foo() is explicitly readonly because it is readonly ref returned

warning CS8656: raised when compiler try access non readonly member from readonly
and it work on one case and dont work in another case.

I suppose when I have readonly struct and try to call any non readonly member on it I must be warned about defensive copy creation in this point. this is beggest pain with defensive copies in C#.
C#8 now have readonly members and new warning about defensive copies that dont produced when compiler create defensive copies. What point of this warning if it dont raised on defensive copies?

Dot state that: "The compiler warns you when it needs to create a defensive copy."
But compiler dont do that and it seems like a bug (in compiler or in documentation).

Reading this I was very happy because it is hell to event dont have warning on defensive copies but now unhappy because it is false promise or bug in compiler because it create defensive copy and dont warn us about this.

What I actually want is make all defensive copy creation warning to be errors in project and make they explicit where we actually need.

@CyrusNajmabadi
Copy link
Member

I suppose when I have readonly struct and try to call any non readonly member on it I must be warned about defensive copy creation in this point

this is not how the feature was designed or specified.

Note: as i mentioned here you could always write an analyzer for this. :)

Dot state that: "The compiler warns you when it needs to create a defensive copy."

As i stated, that's for certain types of members (and the linked post explicitly states that). For more types of cases, you'll have to write an analyzer.

What I actually want is make all defensive copy creation warning to be errors in project and make they explicit where we actually need.

You can definitely do this by writing an analyzer that can produce errors that will stop your build. :)

@CyrusNajmabadi
Copy link
Member

Note: this is explicitly called out here: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-8.0/readonly-instance-members#drawbacks

Some drawbacks as exist with readonly struct methods today. Certain code may still cause hidden copies.

Thanks!

@JesOb
Copy link
Author

JesOb commented Apr 20, 2020

Sad for now :(
Looking for better C# future again :)

@CyrusNajmabadi thinks fro your responces :)
Do you know is there proposal to add option to C# to not generate hidden copies?

Can you direct me please how to write additional analyzer for compiler without rewriting compiler itself to add this as plugin into Unity project.

@CyrusNajmabadi
Copy link
Member

Do you know is there proposal to add option to C# to not generate hidden copies?

Sorry, I don't know about any proposal like that.

Can you direct me please how to write additional analyzer

There are definitely looking if resources out there on writing analyzers :-). You can also get help at gitter.im/dotnet/Roslyn, or discord.gg/csharp (#Roslyn).

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Resolution-By Design The behavior reported in the issue matches the current design
Projects
None yet
Development

No branches or pull requests

3 participants