Skip to content

First try at moving isimplicit to declaration from method. #604

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
Jan 11, 2016

Conversation

genuinelucifer
Copy link
Contributor

I have yet to add a test.
.
Please see if I am going in right direction or not.
Also, i was a bit confused by the 2 fields IsImplicit and IsExplicit. I think they should be opposite of each other but they are implemented separately. Also I could not find a WalkRecord which fills the IsExplicit field.

@ddobrev
Copy link
Contributor

ddobrev commented Jan 11, 2016

@genuinelucifer I am discussing about IsExplicit/IsImplicit with @tritao . In the mean time, please convert your tabs to 4 spaces and add () to D->isImplicit in Parser.cpp. The latter is not a field but a function, I am pretty sure your code does not compile at the moment. You can also start with the test. We have a special test project for checking the obtained AST, CppSharp.Generator.Tests. You can see how it works in the existing tests.

public void TestImplicitDeclaration()
{
Assert.AreEqual(false, AstContext.FindFunction("testExplicitFunc").Single().IsImplicit);
Assert.AreEqual(true, AstContext.FindFunction("testImplicitFunc").Single().IsImplicit);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The second assert is failing. Am I missing something about Implicit and Explicit declarations?

[Test]
public void TestImplicitDeclaration()
{
Assert.IsTrue(AstContext.FindClass("ImplicitCtor").First().Constructors.First(c => c.Parameters.Count == 0).IsImplicit);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please break this line - after "First(" is a good location - and I'll merge.

ddobrev added a commit that referenced this pull request Jan 11, 2016
First try at moving isimplicit to declaration from method.
@ddobrev ddobrev merged commit 3c44798 into mono:master Jan 11, 2016
@ddobrev
Copy link
Contributor

ddobrev commented Jan 11, 2016

@genuinelucifer thank you for your work. Let me know when you are ready for a new task.

@genuinelucifer
Copy link
Contributor Author

genuinelucifer commented Jan 11, 2016 via email

@ddobrev
Copy link
Contributor

ddobrev commented Jan 11, 2016

@genuinelucifer since you are more experienced now compared to 3 or 4 months ago, would you like to try the one for StringBuilder (#508) again?

@genuinelucifer
Copy link
Contributor Author

genuinelucifer commented Jan 11, 2016 via email

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

Successfully merging this pull request may close these issues.

3 participants