Skip to content

Introduce EnumDesc, add ability to output source location attribute #227

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

Merged
merged 4 commits into from
Apr 10, 2021
Merged

Introduce EnumDesc, add ability to output source location attribute #227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
28ae9bd
Introduce EnumDesc, add ability to output source location attribute
andrew-boyarshin Apr 6, 2021
baeba09
Expose config flag and add struct+field test
andrew-boyarshin Apr 10, 2021
32ebc52
Add method+parameter test
andrew-boyarshin Apr 10, 2021
77282d2
Update PInvokeGeneratorConfigurationOptions to use bit shifting
andrew-boyarshin Apr 10, 2021
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: 3 additions & 0 deletions sources/ClangSharp.PInvokeGenerator/Abstractions/ConstantDesc.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Copyright (c) Microsoft and Contributors. All rights reserved. Licensed under the University of Illinois/NCSA Open Source License. See LICENSE.txt in the project root for license information.

using ClangSharp.Interop;

namespace ClangSharp.Abstractions
{
internal struct ConstantDesc
Expand All @@ -9,5 +11,6 @@ internal struct ConstantDesc
public string EscapedName { get; set; }
public string NativeTypeName { get; set; }
public ConstantKind Kind { get; set; }
public CXSourceLocation? Location { get; set; }
}
}
13 changes: 13 additions & 0 deletions sources/ClangSharp.PInvokeGenerator/Abstractions/EnumDesc.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
using ClangSharp.Interop;

namespace ClangSharp.Abstractions
{
public struct EnumDesc
{
public AccessSpecifier AccessSpecifier { get; set; }
public string TypeName { get; set; }
public string EscapedName { get; set; }
public string NativeType { get; set; }
public CXSourceLocation? Location { get; set; }
}
}
3 changes: 3 additions & 0 deletions sources/ClangSharp.PInvokeGenerator/Abstractions/FieldDesc.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Copyright (c) Microsoft and Contributors. All rights reserved. Licensed under the University of Illinois/NCSA Open Source License. See LICENSE.txt in the project root for license information.

using ClangSharp.Interop;

namespace ClangSharp.Abstractions
{
internal struct FieldDesc
Expand All @@ -10,5 +12,6 @@ internal struct FieldDesc
public int? Offset { get; set; }
public bool NeedsNewKeyword { get; set; }
public string InheritedFrom { get; set; }
public CXSourceLocation? Location { get; set; }
}
}
2 changes: 2 additions & 0 deletions sources/ClangSharp.PInvokeGenerator/Abstractions/FunctionOrDelegateDesc.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

using System;
using System.Runtime.InteropServices;
using ClangSharp.Interop;

namespace ClangSharp.Abstractions
{
Expand All @@ -15,6 +16,7 @@ internal struct FunctionOrDelegateDesc<TCustomAttrGeneratorData>
public CallingConvention CallingConvention { get; set; }
public FunctionOrDelegateFlags Flags { get; set; }
public long? VtblIndex { get; set; }
public CXSourceLocation? Location { get; set; }

public bool IsVirtual
{
Expand Down
2 changes: 1 addition & 1 deletion sources/ClangSharp.PInvokeGenerator/Abstractions/IOutputBuilder.VisitDecl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ internal partial interface IOutputBuilder
void EndConstantValue();
void EndConstant(bool isConstant);

void BeginEnum(AccessSpecifier accessSpecifier, string typeName, string escapedName, string nativeTypeName);
void BeginEnum(in EnumDesc desc);
void EndEnum();

void BeginField(in FieldDesc desc);
Expand Down
2 changes: 2 additions & 0 deletions sources/ClangSharp.PInvokeGenerator/Abstractions/ParameterDesc.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

using System;
using System.Collections.Generic;
using ClangSharp.Interop;

namespace ClangSharp.Abstractions
{
Expand All @@ -13,5 +14,6 @@ internal struct ParameterDesc<TCustomAttrGeneratorData>
public IEnumerable<string> CppAttributes { get; set; }
public Action<TCustomAttrGeneratorData> WriteCustomAttrs { get; set; }
public TCustomAttrGeneratorData CustomAttrGeneratorData { get; set; }
public CXSourceLocation? Location { get; set; }
}
}
2 changes: 2 additions & 0 deletions sources/ClangSharp.PInvokeGenerator/Abstractions/StructDesc.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

using System;
using System.Runtime.InteropServices;
using ClangSharp.Interop;

namespace ClangSharp.Abstractions
{
Expand All @@ -14,6 +15,7 @@ internal struct StructDesc<TCustomAttrGeneratorData>
public StructLayoutAttribute Layout { get; set; }
public Guid? Uuid { get; set; }
public StructFlags Flags { get; set; }
public CXSourceLocation? Location { get; set; }

public bool IsUnsafe
{
Expand Down
56 changes: 49 additions & 7 deletions sources/ClangSharp.PInvokeGenerator/CSharp/CSharpOutputBuilder.VisitDecl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Globalization;
using System.Runtime.InteropServices;
using ClangSharp.Abstractions;
using ClangSharp.Interop;

namespace ClangSharp.CSharp
{
Expand All @@ -29,6 +30,9 @@ public void BeginConstant(in ConstantDesc desc)
AddNativeTypeNameAttribute(desc.NativeTypeName);
}

if (desc.Location is {} location)
WriteSourceLocation(location, false);

WriteIndentation();

if ((desc.Kind & ConstantKind.PrimitiveConstant) != 0)
Expand Down Expand Up @@ -66,21 +70,24 @@ public void EndConstantValue()

public void EndConstant(bool isConstant) => WriteLine(isConstant ? ';' : ',');

public void BeginEnum(AccessSpecifier accessSpecifier, string typeName, string escapedName, string nativeTypeName)
public void BeginEnum(in EnumDesc desc)
{
if (nativeTypeName is not null)
if (desc.NativeType is not null)
{
AddNativeTypeNameAttribute(nativeTypeName);
AddNativeTypeNameAttribute(desc.NativeType);
}

WriteIndented(accessSpecifier.AsString());
if (desc.Location is {} location)
WriteSourceLocation(location, false);

WriteIndented(desc.AccessSpecifier.AsString());
Write(" enum ");
Write(escapedName);
Write(desc.EscapedName);

if (!typeName.Equals("int"))
if (!desc.TypeName.Equals("int"))
{
Write(" : ");
Write(typeName);
Write(desc.TypeName);
}

NeedsNewline = true;
Expand All @@ -101,6 +108,9 @@ public void BeginField(in FieldDesc desc)
AddNativeTypeNameAttribute(desc.NativeTypeName);
}

if (desc.Location is {} location)
WriteSourceLocation(location, false);

WriteIndented(desc.AccessSpecifier.AsString());
Write(' ');

Expand Down Expand Up @@ -201,6 +211,9 @@ public void BeginFunctionOrDelegate<TCustomAttrGeneratorData>(in FunctionOrDeleg
WriteLine(")]");
}

if (desc.Location is {} location)
WriteSourceLocation(location, false);

if (desc.IsAggressivelyInlined)
{
AddUsingDirective("System.Runtime.CompilerServices");
Expand Down Expand Up @@ -268,6 +281,29 @@ public void BeginFunctionOrDelegate<TCustomAttrGeneratorData>(in FunctionOrDeleg
}
}

private void WriteSourceLocation(CXSourceLocation location, bool inline)
Copy link
Member

Choose a reason for hiding this comment

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

I'm concerned this is going to end up very noisy across regenerations. The anonymous struct names/remappings is already a pain point for some users

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's opt-in. It is useful for diagnostics (aka "where did this come from?"), but not so much for end-user of the bindings. I don't expect SharpGen-generated bindings or e.g. TerraFX.Interop to contain this info. But e.g. win32metadata and SharpGen might use this information to enrich logging or other forms of diagnostics.

Copy link
Member

Choose a reason for hiding this comment

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

Right, I understand the benefit.

I was just iterating the concern around noise in practice. Many diffs would become nearly unreadable due to minor changes in information such as whitespace addition.

If it's being added here, a corresponding switch and basic test validating it works and doesn't regress should also be added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I'll add public API surface and write some tests this weekend.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I've added 2 tests that cover all important cases.

{
if (!_writeSourceLocation)
return;

if (!inline)
WriteIndentation();

Write("[SourceLocation(\"");
location.GetFileLocation(out var file, out var line, out var column, out _);
Write(PInvokeGenerator.EscapeString(file.Name.ToString()));
Write("\", ");
Write(line);
Write(", ");
Write(column);
Write(")]");

if (!inline)
WriteNewline();
else
Write(' ');
}

public void WriteReturnType(string typeString)
{
Write(typeString);
Expand All @@ -292,6 +328,9 @@ public void BeginParameter<TCustomAttrGeneratorData>(in ParameterDesc<TCustomAtt
AddCppAttributes(info.CppAttributes, prefix: "", postfix: " ");
}

if (info.Location is {} location)
WriteSourceLocation(location, true);

_customAttrIsForParameter = true;
info.WriteCustomAttrs(info.CustomAttrGeneratorData);
_customAttrIsForParameter = false;
Expand Down Expand Up @@ -427,6 +466,9 @@ public void BeginStruct<TCustomAttrGeneratorData>(in StructDesc<TCustomAttrGener
AddNativeInheritanceAttribute(info.NativeInheritance);
}

if (info.Location is {} location)
WriteSourceLocation(location, false);

WriteIndented(info.AccessSpecifier.AsString());
Write(' ');

Expand Down
6 changes: 5 additions & 1 deletion sources/ClangSharp.PInvokeGenerator/CSharp/CSharpOutputBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@ internal sealed partial class CSharpOutputBuilder

private int _indentationLevel;
private readonly MarkerMode _markerMode;
private readonly bool _writeSourceLocation;

public CSharpOutputBuilder(string name, string indentationString = DefaultIndentationString, bool isTestOutput = false, MarkerMode markerMode = MarkerMode.None)
public CSharpOutputBuilder(string name, string indentationString = DefaultIndentationString,
bool isTestOutput = false, MarkerMode markerMode = MarkerMode.None,
bool writeSourceLocation = false)
{
_name = name;
_contents = new List<string>();
Expand All @@ -31,6 +34,7 @@ public CSharpOutputBuilder(string name, string indentationString = DefaultIndent
_indentationString = indentationString;
_isTestOutput = isTestOutput;
_markerMode = markerMode;
_writeSourceLocation = writeSourceLocation;
}

public IEnumerable<string> Contents => _contents;
Expand Down
10 changes: 6 additions & 4 deletions sources/ClangSharp.PInvokeGenerator/OutputBuilderFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ namespace ClangSharp
internal sealed class OutputBuilderFactory
{
private readonly PInvokeGeneratorOutputMode _mode;
private readonly bool _writeSourceLocation;
private readonly Dictionary<string, IOutputBuilder> _outputBuilders;

public OutputBuilderFactory(PInvokeGeneratorOutputMode mode)
public OutputBuilderFactory(PInvokeGeneratorConfiguration mode)
{
_mode = mode;
_mode = mode.OutputMode;
_writeSourceLocation = mode.GenerateSourceLocationAttribute;
_outputBuilders = new Dictionary<string, IOutputBuilder>();
}

Expand All @@ -32,7 +34,7 @@ public IOutputBuilder Create(string name)

var outputBuilder = _mode switch
{
PInvokeGeneratorOutputMode.CSharp => (IOutputBuilder) new CSharpOutputBuilder(name),
PInvokeGeneratorOutputMode.CSharp => (IOutputBuilder) new CSharpOutputBuilder(name, writeSourceLocation: _writeSourceLocation),
PInvokeGeneratorOutputMode.Xml => new XmlOutputBuilder(name),
_ => throw new InvalidOperationException()
};
Expand All @@ -48,7 +50,7 @@ public CSharpOutputBuilder CreateTests(string name)
throw new ArgumentNullException(nameof(name));
}

var outputBuilder = new CSharpOutputBuilder(name, isTestOutput: true);
var outputBuilder = new CSharpOutputBuilder(name, isTestOutput: true, writeSourceLocation: _writeSourceLocation);

_outputBuilders.Add(name, outputBuilder);
return outputBuilder;
Expand Down
Loading