Skip to content

C# 11: Check that we get AST for structs that doesn't initialise all fields. #12058

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

Merged
merged 1 commit into from
Feb 9, 2023

Conversation

michaelnebel
Copy link
Contributor

This is a test only PR.
In C# 11 it is allowed to declare structs that doesn't initialise all fields/properties.

@github-actions github-actions bot added the C# label Feb 1, 2023
@michaelnebel michaelnebel force-pushed the csharp/structdefaults branch from 7136f6c to ae10a6b Compare February 2, 2023 11:53
@michaelnebel michaelnebel changed the title C# 11: Check that we get AST for struct that doesn't initialize all fields. C# 11: Check that we get AST for structs that doesn't initialise all fields. Feb 2, 2023
@michaelnebel michaelnebel marked this pull request as ready for review February 2, 2023 11:53
@michaelnebel michaelnebel requested a review from a team as a code owner February 2, 2023 11:53
@michaelnebel michaelnebel requested review from tamasvajk and mbg February 2, 2023 11:53
Copy link
Contributor

@tamasvajk tamasvajk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In IL, the non-initialized fields/auto-properties are explicitly assigned their default value. Do we need to synthesize these implicit assignments in QL?

@michaelnebel
Copy link
Contributor Author

In IL, the non-initialized fields/auto-properties are explicitly assigned their default value. Do we need to synthesize these implicit assignments in QL?

Good question.
This will require doing some definite assignment analysis, which has to be equivalent to that of the C# compiler itself.
Do you know of any other examples where we do this?

@tamasvajk
Copy link
Contributor

Good question. This will require doing some definite assignment analysis, which has to be equivalent to that of the C# compiler itself. Do you know of any other examples where we do this?

I don't know. I found two PRs that are somewhat related:

@michaelnebel
Copy link
Contributor Author

cc @hvitved : What do you think?

@michaelnebel michaelnebel requested a review from hvitved February 7, 2023 10:06
@hvitved
Copy link
Contributor

hvitved commented Feb 8, 2023

If we were to synthesize default assignments, that would have to be done in the extractor.

However, we also don't generate default assignments for class fields, so I don't think we want to do it for structs.

@michaelnebel
Copy link
Contributor Author

If we were to synthesize default assignments, that would have to be done in the extractor.

However, we also don't generate default assignments for class fields, so I don't think we want to do it for structs.

The difference is that with C# 11 it is possible to not explicitly initialize all fields of a struct. If we decides to not assign a value then the compiler will explicitly insert an instruction to do so.
That is for classes,

public class C {
    private readonly int A;
    private readonly int B;
    
    public C() { 
        A = 0;
    }
}

Compiling and de-compiling is the identify, but for following declaration

public struct S {
    private readonly int A;
    private readonly int B;
    
    public S() { 
        A = 0;
    }
}

compiling and de-compiling yields

public struct S
{
    private readonly int A;

    private readonly int B;

    public S()
    {
        B = 0;
        A = 0;
    }
}

@hvitved
Copy link
Contributor

hvitved commented Feb 9, 2023

The difference is that with C# 11 it is possible to not explicitly initialize all fields of a struct. If we decides to not assign a value then the compiler will explicitly insert an instruction to do so.

Still, fields of classes also get a default value (i.e., the field B of the class C in your example will have value 0), but we don't synthesize the implicit assignment

private readonly int B = default(int);

@michaelnebel
Copy link
Contributor Author

The difference is that with C# 11 it is possible to not explicitly initialize all fields of a struct. If we decides to not assign a value then the compiler will explicitly insert an instruction to do so.

Still, fields of classes also get a default value (i.e., the field B of the class C in your example will have value 0), but we don't synthesize the implicit assignment

private readonly int B = default(int);

I am convinced :-)
If you approve the PR, I will merge.

@michaelnebel michaelnebel merged commit b895065 into github:main Feb 9, 2023
@michaelnebel michaelnebel deleted the csharp/structdefaults branch February 9, 2023 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants