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

C#: infinite recursive setter should be warned at compile time #7695

Closed
zhangtairan opened this issue Dec 24, 2015 · 8 comments
Closed

C#: infinite recursive setter should be warned at compile time #7695

zhangtairan opened this issue Dec 24, 2015 · 8 comments

Comments

@zhangtairan
Copy link

For example:

    class Test
    {
         public int Abc
         {
             set
             {
                 this.Abc = value;
             }
         }
    }
    Test test = new Test();
    test.Abc = 1;

This behavior will cause StackOverflowException, why not disable infinite recursive setter by the compiler?
Thanks!

@markhurd
Copy link

I would guess this becomes by-design or otherwise don't change, because the setter might be more complex, and I'd guess it quickly becomes a halting problem to work out if there's an issue to report.

Simply modifying your code:

private bool secondCall = false;
private int abc = 0;

public int Abc
{
    set
    {
        if (secondCall)
        {
            this.abc = value;
            secondCall = false;
        }
        else
        {
            secondCall = true;
            this.Abc = value;
        }
    }
}

@kindermannhubert
Copy link

Yes, it can't be checked every time, but simple cases could be reported (similar case is infinite recursive getter).

@HaloFour
Copy link

If not a warning I think it would be very useful to have an analyzer that could detect and warn for potential infinite recursion.

Introducing a new compiler warning for this scenario has the potential to break existing code bases where this problem may be silently lurking and where warnings are set to be treated as errors. Avoiding adding said warnings is not a policy with which I agree but I can understand where Microsoft is coming from.

@alrz
Copy link
Member

alrz commented Dec 24, 2015

This one doesn't throw but involves recursion and the resultant behavior would not be intended.

class A { public static int a { get; } = B.b + 1; }
class B { public static int b { get; } = A.a + 1; }

Then the value of seemingly readonly variables depends on the order of static constructor execution. This kind of recursion things should be reported by some analyzer.

@whoisj
Copy link

whoisj commented Dec 25, 2015

👍 having been bit by this more often than I care to admit. While the issue usually results in a very quick stack overflow exception, it's still something the compiler could/should warn about. Agreed about it not being an error unless the compiler can prove that it is in fact a SOE.

@svick
Copy link
Contributor

svick commented Dec 25, 2015

@HaloFour

Introducing a new compiler warning for this scenario has the potential to break existing code bases where this problem may be silently lurking and where warnings are set to be treated as errors.

I thought warning waves (#1580) were supposed for fix that.

@AdamSpeight2008
Copy link
Contributor

Analyser could get or walk the call map, reporting a cycle.

@srivatsn
Copy link
Contributor

srivatsn commented Jan 6, 2016

Moved to the roslyn-analyzers repo - dotnet/roslyn-analyzers#777

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants