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

Improve diagnostics messages for invalid custom native type marshalling and cyclical element info. #1306

Merged
merged 4 commits into from
Jul 9, 2021
Merged

Improve diagnostics messages for invalid custom native type marshalling and cyclical element info. #1306

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
f646d9b
Improve diagnostics messages for invalid custom native type marshalli…
jkoritzinsky Jul 7, 2021
4763e25
Merge branch 'feature/DllImportGenerator' of github.com:dotnet/runtim…
jkoritzinsky Jul 8, 2021
66a1a9d
Clean up the rest of the marshalling info diagnostics.
jkoritzinsky Jul 8, 2021
5c2fc40
Apply suggestions from code review
jkoritzinsky Jul 9, 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
21 changes: 21 additions & 0 deletions DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,16 @@ public class Ids
isEnabledByDefault: true,
description: GetResourceString(nameof(Resources.ConfigurationNotSupportedDescription)));

public readonly static DiagnosticDescriptor MarshallingAttributeConfigurationNotSupported =
new DiagnosticDescriptor(
Ids.ConfigurationNotSupported,
GetResourceString(nameof(Resources.ConfigurationNotSupportedTitle)),
GetResourceString(nameof(Resources.ConfigurationNotSupportedMessageMarshallingInfo)),
Category,
DiagnosticSeverity.Error,
isEnabledByDefault: true,
description: GetResourceString(nameof(Resources.ConfigurationNotSupportedDescription)));

public readonly static DiagnosticDescriptor TargetFrameworkNotSupported =
new DiagnosticDescriptor(
Ids.TargetFrameworkNotSupported,
Expand Down Expand Up @@ -280,6 +290,17 @@ internal void ReportMarshallingNotSupported(
}
}

internal void ReportInvalidMarshallingAttributeInfo(
AttributeData attributeData,
string reasonResourceName,
params string[] reasonArgs)
{
this.context.ReportDiagnostic(
attributeData.CreateDiagnostic(
GeneratorDiagnostics.MarshallingAttributeConfigurationNotSupported,
new LocalizableResourceString(reasonResourceName, Resources.ResourceManager, typeof(Resources), reasonArgs)));
}

/// <summary>
/// Report diagnostic for targeting a framework that is not supported
/// </summary>
Expand Down
33 changes: 21 additions & 12 deletions DllImportGenerator/DllImportGenerator/MarshallingAttributeInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ private MarshallingInfo ParseMarshallingInfo(
{
if (marshallingAttributesByIndirectionLevel.ContainsKey(indirectionLevel))
{
_diagnostics.ReportConfigurationNotSupported(attribute, "Marshalling Data for Indirection Level", indirectionLevel.ToString());
_diagnostics.ReportInvalidMarshallingAttributeInfo(attribute, nameof(Resources.DuplicateMarshallingInfo), indirectionLevel.ToString());
return NoMarshallingInfo.Instance;
}
marshallingAttributesByIndirectionLevel.Add(indirectionLevel, attribute);
Expand All @@ -203,7 +203,11 @@ private MarshallingInfo ParseMarshallingInfo(
ref maxIndirectionLevelUsed);
if (maxIndirectionLevelUsed < maxIndirectionLevelDataProvided)
{
_diagnostics.ReportConfigurationNotSupported(marshallingAttributesByIndirectionLevel[maxIndirectionLevelDataProvided], ManualTypeMarshallingHelper.MarshalUsingProperties.ElementIndirectionLevel, maxIndirectionLevelDataProvided.ToString());
_diagnostics.ReportInvalidMarshallingAttributeInfo(
marshallingAttributesByIndirectionLevel[maxIndirectionLevelDataProvided],
nameof(Resources.ExtraneousMarshallingInfo),
maxIndirectionLevelDataProvided.ToString(),
maxIndirectionLevelUsed.ToString());
}
return info;
}
Expand Down Expand Up @@ -232,7 +236,7 @@ private MarshallingInfo GetMarshallingInfo(
{
if (parsedCountInfo != NoCountInfo.Instance)
{
_diagnostics.ReportConfigurationNotSupported(useSiteAttribute, "Duplicate Count Info");
_diagnostics.ReportInvalidMarshallingAttributeInfo(useSiteAttribute, nameof(Resources.DuplicateCountInfo));
return NoMarshallingInfo.Instance;
}
parsedCountInfo = CreateCountInfo(useSiteAttribute, inspectedElements);
Expand Down Expand Up @@ -333,7 +337,7 @@ CountInfo CreateCountInfo(AttributeData marshalUsingData, ImmutableHashSet<strin

if (constSize is not null && elementName is not null)
{
_diagnostics.ReportConfigurationNotSupported(marshalUsingData, $"{ManualTypeMarshallingHelper.MarshalUsingProperties.ConstantElementCount} and {ManualTypeMarshallingHelper.MarshalUsingProperties.CountElementName} combined");
_diagnostics.ReportInvalidMarshallingAttributeInfo(marshalUsingData, nameof(Resources.ConstantAndElementCountInfoDisallowed));
}
else if (constSize is not null)
{
Expand All @@ -360,7 +364,7 @@ CountInfo CreateCountInfo(AttributeData marshalUsingData, ImmutableHashSet<strin
// This ensures that we've unwound the whole cycle so when we return NoCountInfo.Instance, there will be no cycles in the count info.
catch (CyclicalCountElementInfoException ex) when (ex.StartOfCycle == elementName)
{
_diagnostics.ReportConfigurationNotSupported(marshalUsingData, $"Cyclical {ManualTypeMarshallingHelper.MarshalUsingProperties.CountElementName}");
_diagnostics.ReportInvalidMarshallingAttributeInfo(marshalUsingData, nameof(Resources.CyclicalCountInfo), elementName);
return NoCountInfo.Instance;
}
}
Expand Down Expand Up @@ -392,7 +396,7 @@ CountInfo CreateCountInfo(AttributeData marshalUsingData, ImmutableHashSet<strin
// This ensures that we've unwound the whole cycle so when we return, there will be no cycles in the count info.
catch (CyclicalCountElementInfoException ex) when (ex.StartOfCycle == param.Name)
{
_diagnostics.ReportConfigurationNotSupported(attrData, $"Cyclical {ManualTypeMarshallingHelper.MarshalUsingProperties.CountElementName}");
_diagnostics.ReportInvalidMarshallingAttributeInfo(attrData, nameof(Resources.CyclicalCountInfo), param.Name);
return SizeAndParamIndexInfo.UnspecifiedParam;
}
}
Expand Down Expand Up @@ -571,14 +575,14 @@ MarshallingInfo CreateNativeMarshallingInfo(
{
if (isMarshalUsingAttribute)
{
_diagnostics.ReportConfigurationNotSupported(attrData, "Native Type", nativeType.ToDisplayString());
_diagnostics.ReportInvalidMarshallingAttributeInfo(attrData, nameof(Resources.NativeGenericTypeMustBeClosedOrMatchArityMessage), nativeType.ToDisplayString());
return NoMarshallingInfo.Instance;
}
else if (type is INamedTypeSymbol namedType)
{
if (namedType.Arity != nativeType.Arity)
{
_diagnostics.ReportConfigurationNotSupported(attrData, "Native Type", nativeType.ToDisplayString());
_diagnostics.ReportInvalidMarshallingAttributeInfo(attrData, nameof(Resources.NativeGenericTypeMustBeClosedOrMatchArityMessage), nativeType.ToDisplayString());
return NoMarshallingInfo.Instance;
}
else
Expand All @@ -588,7 +592,7 @@ MarshallingInfo CreateNativeMarshallingInfo(
}
else
{
_diagnostics.ReportConfigurationNotSupported(attrData, "Native Type", nativeType.ToDisplayString());
_diagnostics.ReportInvalidMarshallingAttributeInfo(attrData, nameof(Resources.NativeGenericTypeMustBeClosedOrMatchArityMessage), nativeType.ToDisplayString());
return NoMarshallingInfo.Instance;
}
}
Expand Down Expand Up @@ -632,21 +636,26 @@ MarshallingInfo CreateNativeMarshallingInfo(

if (methods == SupportedMarshallingMethods.None)
{
_diagnostics.ReportConfigurationNotSupported(attrData, "Native Type", nativeType.ToDisplayString());
_diagnostics.ReportInvalidMarshallingAttributeInfo(
attrData,
isContiguousCollectionMarshaller
? nameof(Resources.CollectionNativeTypeMustHaveRequiredShapeMessage)
: nameof(Resources.NativeTypeMustHaveRequiredShapeMessage),
nativeType.ToDisplayString());
return NoMarshallingInfo.Instance;
}

if (isContiguousCollectionMarshaller)
{
if (!ManualTypeMarshallingHelper.HasNativeValueStorageProperty(nativeType, spanOfByte))
{
_diagnostics.ReportConfigurationNotSupported(attrData, "Native Type", nativeType.ToDisplayString());
_diagnostics.ReportInvalidMarshallingAttributeInfo(attrData, nameof(Resources.CollectionNativeTypeMustHaveRequiredShapeMessage), nativeType.ToDisplayString());
return NoMarshallingInfo.Instance;
}

if (!ManualTypeMarshallingHelper.TryGetElementTypeFromContiguousCollectionMarshaller(nativeType, out ITypeSymbol elementType))
{
_diagnostics.ReportConfigurationNotSupported(attrData, "Native Type", nativeType.ToDisplayString());
_diagnostics.ReportInvalidMarshallingAttributeInfo(attrData, nameof(Resources.CollectionNativeTypeMustHaveRequiredShapeMessage), nativeType.ToDisplayString());
return NoMarshallingInfo.Instance;
}

Expand Down
54 changes: 54 additions & 0 deletions DllImportGenerator/DllImportGenerator/Resources.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 19 additions & 1 deletion DllImportGenerator/DllImportGenerator/Resources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,9 @@
<data name="ConfigurationNotSupportedMessage" xml:space="preserve">
<value>The '{0}' configuration is not supported by source-generated P/Invokes. If the specified configuration is required, use a regular `DllImport` instead.</value>
</data>
<data name="ConfigurationNotSupportedMessageMarshallingInfo" xml:space="preserve">
<value>The specified marshalling configuration is not supported by source-generated P/Invokes. {0}.</value>
</data>
<data name="ConfigurationNotSupportedMessageParameter" xml:space="preserve">
<value>The specified '{0}' configuration for parameter '{1}' is not supported by source-generated P/Invokes. If the specified configuration is required, use a regular `DllImport` instead.</value>
</data>
Expand All @@ -153,6 +156,9 @@
<data name="ConfigurationNotSupportedTitle" xml:space="preserve">
<value>Specified configuration is not supported by source-generated P/Invokes.</value>
</data>
<data name="ConstantAndElementCountInfoDisallowed" xml:space="preserve">
<value>Only one of 'ConstantElementCount' or 'ElementCountInfo' may be used in a 'MarshalUsingAttribute' for a given 'ElementIndirectionLevel'</value>
</data>
<data name="ConvertToGeneratedDllImportDescription" xml:space="preserve">
<value>Use 'GeneratedDllImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time</value>
</data>
Expand All @@ -178,6 +184,18 @@
<data name="CustomTypeMarshallingNativeToManagedUnsupported" xml:space="preserve">
<value>The specified parameter needs to be marshalled from native to managed, but the native type '{0}' does not support it.</value>
</data>
<data name="CyclicalCountInfo" xml:space="preserve">
<value>This element cannot depend on '{0}' for collection size information without creating a dependency cycle</value>
</data>
<data name="DuplicateCountInfo" xml:space="preserve">
<value>Count information for a given element at a given indirection level can only be specified once</value>
</data>
<data name="DuplicateMarshallingInfo" xml:space="preserve">
<value>Multiple marshalling attributes per element per indirection level is unsupported, but duplicate information was provided for indirection level {0}</value>
</data>
<data name="ExtraneousMarshallingInfo" xml:space="preserve">
<value>Marshalling info was specified for 'ElementIndirectionLevel' {0}, but marshalling info was only needed for {1} level(s) of indirection</value>
</data>
<data name="GeneratedDllImportContainingTypeMissingModifiersDescription" xml:space="preserve">
<value>Types that contain methods marked with 'GeneratedDllImportAttribute' must be 'partial'. P/Invoke source generation will ignore methods contained within non-partial types.</value>
</data>
Expand Down Expand Up @@ -334,4 +352,4 @@
<data name="ValuePropertyMustHaveSetterMessage" xml:space="preserve">
<value>The 'Value' property on the native type '{0}' must have a setter</value>
</data>
</root>
</root>