Skip to content

C++: Add support for default member initializers. #6541

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
Aug 26, 2021

Conversation

sashabu
Copy link
Contributor

@sashabu sashabu commented Aug 24, 2021

No description provided.

@sashabu sashabu added enhancement New feature or request C++ depends on internal PR This PR should only be merged in sync with an internal Semmle PR labels Aug 24, 2021
@sashabu sashabu requested a review from a team as a code owner August 24, 2021 16:07
@sashabu sashabu requested a review from jbj August 24, 2021 16:19
@jbj
Copy link
Contributor

jbj commented Aug 25, 2021

The new consistency errors are expected, but they're real enough. We'll have to come back and fix them later.

How do the data-flow libraries of other languages assign an enclosing callable to field initializers like these? @hvitved, you probably know.

@hvitved
Copy link
Contributor

hvitved commented Aug 25, 2021

How do the data-flow libraries of other languages assign an enclosing callable to field initializers like these? @hvitved, you probably know.

For C#, we inline field initializers into the relevant contstructor(s) in the CFG. So, if we have something like

class C
{
    int Field = 0;

    public C()
    {
        Console.WriteLine("C");
    }
}

then the CFG for the constructor will be based on this rewrite

class C
{
    int Field;

    public C()
    {
        Field = 0;
        Console.WriteLine("C");
    }
}

jbj
jbj previously approved these changes Aug 25, 2021
Copy link
Contributor

@jbj jbj left a comment

Choose a reason for hiding this comment

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

We can merge this PR together with the internal PR when that's approved.

@sashabu
Copy link
Contributor Author

sashabu commented Aug 26, 2021

@jbj Could you please have another look since I've pulled changes from main?

@sashabu sashabu requested a review from jbj August 26, 2021 11:53
Copy link
Contributor

@jbj jbj left a comment

Choose a reason for hiding this comment

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

LGTM. We can merge this when the internal PR is ready.

@sashabu sashabu merged commit f18e8a4 into github:main Aug 26, 2021
@sashabu sashabu deleted the sashabu/init branch August 26, 2021 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ depends on internal PR This PR should only be merged in sync with an internal Semmle PR enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants