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.

Sign up for GitHub

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

Jump to bottom

Mark ternary macros that can be evaluated as constants as const #221

Merged
merged 1 commit into from
Apr 8, 2021
Merged

Mark ternary macros that can be evaluated as constants as const #221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion sources/ClangSharp.PInvokeGenerator/PInvokeGenerator.VisitDecl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2861,7 +2861,8 @@ bool IsConstant(Expr initExpr)

case CX_StmtClass.CX_StmtClass_ConditionalOperator:
{
return false;
var conditionalOperator = (ConditionalOperator)initExpr;
return IsConstant(conditionalOperator.Cond) && IsConstant(conditionalOperator.LHS) && IsConstant(conditionalOperator.RHS);
Copy link
Member

@tannergooding tannergooding Mar 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

nit: This isn't quite right. If IsConstant(conditionalOperator.Cond) is true and the evaluated value of Cond is true, then only IsConstant(conditionalOperator.LHS) needs to be true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

a ? b :c does that mean Cond is a and LHS is b : c? Otherwise confused by why only 2 of the 3 would need to be true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code doesn't check that Cond evaluates to true. It just checks that it evaluates to a constant.

Copy link
Member

Choose a reason for hiding this comment

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

cond ? LHS : RHS

x = cond ? LHS : RHS is basically shorthand for:

if (cond)
{
    x = LHS;
}
else
{
    x = RHS;
}

So if you have x = trueCond ? 5 : sqrt(globalValue), then you want it to still evaluate to be "constant". As it is now, it won't since IsConstant(conditionalOperator.RHS) will return false

Copy link
Member

Choose a reason for hiding this comment

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

Likewise if you had x = falseCond ? sqrt(globalValue) : 5, you want it to evaluate as constant while IsConstant(conditionalOperator.LHS) will be the only thing to return false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the generator actually generates the ternary even if the Cond is constant, wouldn't the current impl need both LHS and RHS to be true? I agree if the generator actually lowered the ternary then only the evaluated condition would need to be checked, but thats not what gets generated currently.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like yes actually. I thought C# would drop the dead code path here, but looks like its not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. I agree if the generator is changed to lower everything, then this evaluation could be more fine grained. But for now, it will generate the whole thing, so all 3 need to be constant evaluated in order to generate const.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, this looks good and I'll merge once CI finishes

}

// case CX_StmtClass.CX_StmtClass_AddrLabelExpr:
Expand Down
3 changes: 3 additions & 0 deletions tests/ClangSharp.PInvokeGenerator.UnitTests/Base/VarDeclarationTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,5 +74,8 @@ public abstract class VarDeclarationTest : PInvokeGeneratorTest

[Fact]
public abstract Task MultidimensionlArrayTest();

[Fact]
public abstract Task ConditionalDefineConstTest();
}
}
20 changes: 20 additions & 0 deletions tests/ClangSharp.PInvokeGenerator.UnitTests/CSharpCompatibleUnix/VarDeclarationTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,5 +300,25 @@ public static partial class Methods

return ValidateGeneratedCSharpCompatibleUnixBindingsAsync(inputContents, expectedOutputContents);
}

public override Task ConditionalDefineConstTest()
{
var inputContents = @"typedef int TESTRESULT;
#define TESTRESULT_FROM_WIN32(x) ((TESTRESULT)(x) <= 0 ? ((TESTRESULT)(x)) : ((TESTRESULT) (((x) & 0x0000FFFF) | (7 << 16) | 0x80000000)))
#define ADDRESS_IN_USE TESTRESULT_FROM_WIN32(10048)";

var expectedOutputContents = $@"namespace ClangSharp.Test
{{
public static partial class Methods
{{
[NativeTypeName(""#define ADDRESS_IN_USE TESTRESULT_FROM_WIN32(10048)"")]
public const int ADDRESS_IN_USE = unchecked((int)(10048) <= 0 ? ((int)(10048)) : ((int)(((10048) & 0x0000FFFF) | (7 << 16) | 0x80000000)));
}}
}}
";
var diagnostics = new Diagnostic[] { new Diagnostic(DiagnosticLevel.Warning, "Function like macro definition records are not supported: 'TESTRESULT_FROM_WIN32'. Generated bindings may be incomplete.", "Line 2, Column 9 in ClangUnsavedFile.h") };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a better way to do this?

Copy link
Member

Choose a reason for hiding this comment

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

Not really, this is about how everything works for validating diagnostics.


return ValidateGeneratedCSharpCompatibleUnixBindingsAsync(inputContents, expectedOutputContents, expectedDiagnostics: diagnostics);
}
}
}
20 changes: 20 additions & 0 deletions tests/ClangSharp.PInvokeGenerator.UnitTests/CSharpCompatibleWindows/VarDeclarationTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,5 +307,25 @@ public static partial class Methods

return ValidateGeneratedCSharpCompatibleWindowsBindingsAsync(inputContents, expectedOutputContents);
}

public override Task ConditionalDefineConstTest()
{
var inputContents = @"typedef int TESTRESULT;
#define TESTRESULT_FROM_WIN32(x) ((TESTRESULT)(x) <= 0 ? ((TESTRESULT)(x)) : ((TESTRESULT) (((x) & 0x0000FFFF) | (7 << 16) | 0x80000000)))
#define ADDRESS_IN_USE TESTRESULT_FROM_WIN32(10048)";

var expectedOutputContents = $@"namespace ClangSharp.Test
{{
public static partial class Methods
{{
[NativeTypeName(""#define ADDRESS_IN_USE TESTRESULT_FROM_WIN32(10048)"")]
public const int ADDRESS_IN_USE = unchecked((int)(10048) <= 0 ? ((int)(10048)) : ((int)(((10048) & 0x0000FFFF) | (7 << 16) | 0x80000000)));
}}
}}
";
var diagnostics = new Diagnostic[] { new Diagnostic(DiagnosticLevel.Warning, "Function like macro definition records are not supported: 'TESTRESULT_FROM_WIN32'. Generated bindings may be incomplete.", "Line 2, Column 9 in ClangUnsavedFile.h") };

return ValidateGeneratedCSharpCompatibleWindowsBindingsAsync(inputContents, expectedOutputContents, expectedDiagnostics: diagnostics);
}
}
}
20 changes: 20 additions & 0 deletions tests/ClangSharp.PInvokeGenerator.UnitTests/CSharpLatestUnix/VarDeclarationTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,5 +296,25 @@ public static partial class Methods

return ValidateGeneratedCSharpLatestUnixBindingsAsync(inputContents, expectedOutputContents);
}

public override Task ConditionalDefineConstTest()
{
var inputContents = @"typedef int TESTRESULT;
#define TESTRESULT_FROM_WIN32(x) ((TESTRESULT)(x) <= 0 ? ((TESTRESULT)(x)) : ((TESTRESULT) (((x) & 0x0000FFFF) | (7 << 16) | 0x80000000)))
#define ADDRESS_IN_USE TESTRESULT_FROM_WIN32(10048)";

var expectedOutputContents = $@"namespace ClangSharp.Test
{{
public static partial class Methods
{{
[NativeTypeName(""#define ADDRESS_IN_USE TESTRESULT_FROM_WIN32(10048)"")]
public const int ADDRESS_IN_USE = unchecked((int)(10048) <= 0 ? ((int)(10048)) : ((int)(((10048) & 0x0000FFFF) | (7 << 16) | 0x80000000)));
}}
}}
";
var diagnostics = new Diagnostic[] { new Diagnostic(DiagnosticLevel.Warning, "Function like macro definition records are not supported: 'TESTRESULT_FROM_WIN32'. Generated bindings may be incomplete.", "Line 2, Column 9 in ClangUnsavedFile.h") };

return ValidateGeneratedCSharpLatestUnixBindingsAsync(inputContents, expectedOutputContents, expectedDiagnostics: diagnostics);
}
}
}
20 changes: 20 additions & 0 deletions tests/ClangSharp.PInvokeGenerator.UnitTests/CSharpLatestWindows/VarDeclarationTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,5 +307,25 @@ public static partial class Methods

return ValidateGeneratedCSharpLatestWindowsBindingsAsync(inputContents, expectedOutputContents);
}

public override Task ConditionalDefineConstTest()
{
var inputContents = @"typedef int TESTRESULT;
#define TESTRESULT_FROM_WIN32(x) ((TESTRESULT)(x) <= 0 ? ((TESTRESULT)(x)) : ((TESTRESULT) (((x) & 0x0000FFFF) | (7 << 16) | 0x80000000)))
#define ADDRESS_IN_USE TESTRESULT_FROM_WIN32(10048)";

var expectedOutputContents = $@"namespace ClangSharp.Test
{{
public static partial class Methods
{{
[NativeTypeName(""#define ADDRESS_IN_USE TESTRESULT_FROM_WIN32(10048)"")]
public const int ADDRESS_IN_USE = unchecked((int)(10048) <= 0 ? ((int)(10048)) : ((int)(((10048) & 0x0000FFFF) | (7 << 16) | 0x80000000)));
}}
}}
";
var diagnostics = new Diagnostic[] { new Diagnostic(DiagnosticLevel.Warning, "Function like macro definition records are not supported: 'TESTRESULT_FROM_WIN32'. Generated bindings may be incomplete.", "Line 2, Column 9 in ClangUnsavedFile.h") };

return ValidateGeneratedCSharpLatestWindowsBindingsAsync(inputContents, expectedOutputContents, expectedDiagnostics: diagnostics);
}
}
}
27 changes: 27 additions & 0 deletions tests/ClangSharp.PInvokeGenerator.UnitTests/XmlCompatibleUnix/VarDeclarationTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,5 +383,32 @@ public override Task MultidimensionlArrayTest()

return ValidateGeneratedXmlCompatibleUnixBindingsAsync(inputContents, expectedOutputContents);
}

public override Task ConditionalDefineConstTest()
{
var inputContents = @"typedef int TESTRESULT;
#define TESTRESULT_FROM_WIN32(x) ((TESTRESULT)(x) <= 0 ? ((TESTRESULT)(x)) : ((TESTRESULT) (((x) & 0x0000FFFF) | (7 << 16) | 0x80000000)))
#define ADDRESS_IN_USE TESTRESULT_FROM_WIN32(10048)";

var expectedOutputContents = $@"<?xml version=""1.0"" encoding=""UTF-8"" standalone=""yes"" ?>
<bindings>
<namespace name=""ClangSharp.Test"">
<class name=""Methods"" access=""public"" static=""true"">
<constant name=""ADDRESS_IN_USE"" access=""public"">
<type primitive=""True"">int</type>
<value>
<unchecked>
<code>((int)(10048) &lt;= 0 ? ((int)(10048)) : ((int)(((10048) &amp; 0x0000FFFF) | (7 &lt;&lt; 16) | 0x80000000)))</code>
</unchecked>
</value>
</constant>
</class>
</namespace>
</bindings>
";
var diagnostics = new Diagnostic[] { new Diagnostic(DiagnosticLevel.Warning, "Function like macro definition records are not supported: 'TESTRESULT_FROM_WIN32'. Generated bindings may be incomplete.", "Line 2, Column 9 in ClangUnsavedFile.h") };

return ValidateGeneratedXmlCompatibleUnixBindingsAsync(inputContents, expectedOutputContents, expectedDiagnostics: diagnostics);
}
}
}
27 changes: 27 additions & 0 deletions tests/ClangSharp.PInvokeGenerator.UnitTests/XmlCompatibleWindows/VarDeclarationTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,5 +403,32 @@ public override Task MultidimensionlArrayTest()

return ValidateGeneratedXmlCompatibleWindowsBindingsAsync(inputContents, expectedOutputContents);
}

public override Task ConditionalDefineConstTest()
{
var inputContents = @"typedef int TESTRESULT;
#define TESTRESULT_FROM_WIN32(x) ((TESTRESULT)(x) <= 0 ? ((TESTRESULT)(x)) : ((TESTRESULT) (((x) & 0x0000FFFF) | (7 << 16) | 0x80000000)))
#define ADDRESS_IN_USE TESTRESULT_FROM_WIN32(10048)";

var expectedOutputContents = $@"<?xml version=""1.0"" encoding=""UTF-8"" standalone=""yes"" ?>
<bindings>
<namespace name=""ClangSharp.Test"">
<class name=""Methods"" access=""public"" static=""true"">
<constant name=""ADDRESS_IN_USE"" access=""public"">
<type primitive=""True"">int</type>
<value>
<unchecked>
<code>((int)(10048) &lt;= 0 ? ((int)(10048)) : ((int)(((10048) &amp; 0x0000FFFF) | (7 &lt;&lt; 16) | 0x80000000)))</code>
</unchecked>
</value>
</constant>
</class>
</namespace>
</bindings>
";
var diagnostics = new Diagnostic[] { new Diagnostic(DiagnosticLevel.Warning, "Function like macro definition records are not supported: 'TESTRESULT_FROM_WIN32'. Generated bindings may be incomplete.", "Line 2, Column 9 in ClangUnsavedFile.h") };

return ValidateGeneratedXmlCompatibleWindowsBindingsAsync(inputContents, expectedOutputContents, expectedDiagnostics: diagnostics);
}
}
}
27 changes: 27 additions & 0 deletions tests/ClangSharp.PInvokeGenerator.UnitTests/XmlLatestUnix/VarDeclarationTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,5 +387,32 @@ public override Task MultidimensionlArrayTest()

return ValidateGeneratedXmlLatestUnixBindingsAsync(inputContents, expectedOutputContents);
}

public override Task ConditionalDefineConstTest()
{
var inputContents = @"typedef int TESTRESULT;
#define TESTRESULT_FROM_WIN32(x) ((TESTRESULT)(x) <= 0 ? ((TESTRESULT)(x)) : ((TESTRESULT) (((x) & 0x0000FFFF) | (7 << 16) | 0x80000000)))
#define ADDRESS_IN_USE TESTRESULT_FROM_WIN32(10048)";

var expectedOutputContents = $@"<?xml version=""1.0"" encoding=""UTF-8"" standalone=""yes"" ?>
<bindings>
<namespace name=""ClangSharp.Test"">
<class name=""Methods"" access=""public"" static=""true"">
<constant name=""ADDRESS_IN_USE"" access=""public"">
<type primitive=""True"">int</type>
<value>
<unchecked>
<code>((int)(10048) &lt;= 0 ? ((int)(10048)) : ((int)(((10048) &amp; 0x0000FFFF) | (7 &lt;&lt; 16) | 0x80000000)))</code>
</unchecked>
</value>
</constant>
</class>
</namespace>
</bindings>
";
var diagnostics = new Diagnostic[] { new Diagnostic(DiagnosticLevel.Warning, "Function like macro definition records are not supported: 'TESTRESULT_FROM_WIN32'. Generated bindings may be incomplete.", "Line 2, Column 9 in ClangUnsavedFile.h") };

return ValidateGeneratedXmlLatestUnixBindingsAsync(inputContents, expectedOutputContents, expectedDiagnostics: diagnostics);
}
}
}
27 changes: 27 additions & 0 deletions tests/ClangSharp.PInvokeGenerator.UnitTests/XmlLatestWindows/VarDeclarationTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,5 +403,32 @@ public override Task MultidimensionlArrayTest()

return ValidateGeneratedXmlLatestWindowsBindingsAsync(inputContents, expectedOutputContents);
}

public override Task ConditionalDefineConstTest()
{
var inputContents = @"typedef int TESTRESULT;
#define TESTRESULT_FROM_WIN32(x) ((TESTRESULT)(x) <= 0 ? ((TESTRESULT)(x)) : ((TESTRESULT) (((x) & 0x0000FFFF) | (7 << 16) | 0x80000000)))
#define ADDRESS_IN_USE TESTRESULT_FROM_WIN32(10048)";

var expectedOutputContents = $@"<?xml version=""1.0"" encoding=""UTF-8"" standalone=""yes"" ?>
<bindings>
<namespace name=""ClangSharp.Test"">
<class name=""Methods"" access=""public"" static=""true"">
<constant name=""ADDRESS_IN_USE"" access=""public"">
<type primitive=""True"">int</type>
<value>
<unchecked>
<code>((int)(10048) &lt;= 0 ? ((int)(10048)) : ((int)(((10048) &amp; 0x0000FFFF) | (7 &lt;&lt; 16) | 0x80000000)))</code>
</unchecked>
</value>
</constant>
</class>
</namespace>
</bindings>
";
var diagnostics = new Diagnostic[] { new Diagnostic(DiagnosticLevel.Warning, "Function like macro definition records are not supported: 'TESTRESULT_FROM_WIN32'. Generated bindings may be incomplete.", "Line 2, Column 9 in ClangUnsavedFile.h") };

return ValidateGeneratedXmlLatestWindowsBindingsAsync(inputContents, expectedOutputContents, expectedDiagnostics: diagnostics);
}
}
}