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

Doing some minor code cleanup and adding some PInvokeGenerator tests #50

Merged
merged 4 commits into from
May 26, 2019

Conversation

tannergooding
Copy link
Member

This performs some minor code cleanup so that the structured traverser no long silently stops process a branch if the node kind is not explicitly recognized (this is primarily impactful for release mode).

This also adds a ClangSharpPInvokeGenerate.Test project and a couple basic tests.

public sealed class EnumDeclarationTest : PInvokeGeneratorTest
{
[Fact]
public async Task BasicTest()
Copy link
Member Author

Choose a reason for hiding this comment

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

These tests are very basic and the general infrastructure involves creating two temporary files on disk per test (the PInvokeGenerateTest class is itself IDisposable so xUnit should ensure they are cleaned up per test as well).

For every test, it just takes an input and generates an expected output.

It should be possible to refactor things in the ClangSharpPInvokeGenerator to work on Streams instead, in which case everything could be in memory, but that might be a good thing to do in a future PR (you likely need to provide some kind of factory so that the multi-file configuration works with this setup as well).

@@ -971,7 +980,7 @@ private void Visit(Cursor cursor, Cursor parent)

foreach (var child in cursor.Children)
{
Debug.Assert(_visitedCursors.Contains(child));
Debug.Assert(_visitedCursors.Contains(child) || !child.IsFromMainFile);
Copy link
Member Author

Choose a reason for hiding this comment

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

I am going to clean up, in a future PR, the CursorWriter to no longer silently fail to process things either and will likely set up a way to report Diagnostics so that we can tell the user what we failed to process.

@@ -5,6 +5,54 @@ namespace ClangSharpPInvokeGenerator
{
internal class Attr : Cursor
{
public static new Attr Create(CXCursor handle, Cursor parent)
Copy link
Member Author

Choose a reason for hiding this comment

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

The Cursor.Create method was split out into various sub Create methods so that we can try to return the "best" base type for anything we don't currently recognize.

@@ -19,11 +19,6 @@ protected override CXChildVisitResult VisitChildren(CXCursor childHandle, CXCurs
{
ValidateVisit(ref handle);

if (!childHandle.Location.IsFromMainFile)
Copy link
Member Author

Choose a reason for hiding this comment

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

We need the traversal to actually cover all nodes, including those not in the main file. This is important for handling things like base class declarations that exist in an included file.

@@ -151,12 +153,12 @@ private static void AddAdditionalOption(RootCommand rootCommand)
{
var argument = new Argument();
argument.ArgumentType = typeof(string);
argument.Arity = ArgumentArity.ZeroOrMore;
argument.Arity = ArgumentArity.OneOrMore;
Copy link
Member Author

Choose a reason for hiding this comment

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

ZeroOrMore was actually allowing you to specify --additional with no argument

@@ -12,7 +12,7 @@ public class Program
{
private static RootCommand s_rootCommand;

public static async Task<int> Main(string[] args)
public static async Task<int> Main(params string[] args)
Copy link
Member Author

Choose a reason for hiding this comment

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

It might be nice to make the PInvokeGenerator a simple wrapper over some library APIs. That would make it easier to test and for other people to use/extend...

@tannergooding tannergooding merged commit 413a1c8 into dotnet:master May 26, 2019
@tannergooding tannergooding deleted the prod-cleanup branch May 29, 2019 00:28
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.

2 participants