Skip to content

Commit

Permalink
Support feature settings on descriptor files (dotnet/linker#1277)
Browse files Browse the repository at this point in the history 
* Support feature settings on descriptor files

Fixes dotnet/linker#1268.

* Disable feature descriptors for mono

Since they conflict with the --exclude-feature "feature" attributes in
the descriptor XML.

* Revert "Disable feature descriptors for mono"

This reverts commit dotnet/linker@78c2f52.

* Support featuredefault="true"

To indicate that a substitution or descriptor should be applied in the
absence of a feature setting. Note that this is restricted to still
require the featurevalue attribute, to prevent the definition of
default substitutions that are never applied for any feature settings.

Also address PR feedback:
- Move the feature checks into a separate class
- #ifdef an implementation instead of callsites

* Fix signatures in tests

* Update data format docs

* Fix whitespace

* PR feedback

Add comments about order of checking assemblyname and feature attributes,
and make processing consistent in LinkAttributesStep.

* Clarify featuredefault behavior in documentation

Commit migrated from dotnet/linker@f87cfa2
  • Loading branch information
@sbomer
sbomer authored Jun 23, 2020
1 parent 280b50c commit d503ec8
Show file tree
Hide file tree
Showing 24 changed files with 426 additions and 175 deletions.
34 changes: 28 additions & 6 deletions src/tools/illink/docs/data-formats.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ The `required` attribute specifies that if the type is not marked, during the ma

<!-- Type with generics in the signature -->
<type fullname="Assembly.G`1" />
</assembly>
</linker>
```

Expand Down Expand Up @@ -87,6 +88,7 @@ The `required` attribute specifies that if the type is not marked, during the ma
<!-- Field with generics in the signature -->
<field signature="System.Collections.Generic.List`1&lt;System.Int32&gt; field3" />
<field signature="System.Collections.Generic.List`1&lt;T&gt; field4" />
</type>
</assembly>
</linker>
```
Expand All @@ -108,6 +110,7 @@ The `required` attribute specifies that if the type is not marked, during the ma

<!-- Preserve the method if the type is used. If the type is not used it will be removed -->
<method signature="System.Void Method4()" required="false" />
</type>
</assembly>
</linker>
```
Expand All @@ -134,6 +137,7 @@ The `required` attribute specifies that if the type is not marked, during the ma

<!-- Preserve the property if the type is used. If the type is not used it will be removed -->
<property signature="System.Int32 Property6" required="false" />
</type>
</assembly>
</linker>
```
Expand Down Expand Up @@ -214,19 +218,37 @@ The `initialize` attribute is optional and when not specified the code to set th
</linker>
```

### Conditional substitutions
### Conditional substitutions and descriptors

The `feature` and `featurevalue` attributes are optional, but must be used together when used.
They can be applied to any element to specify conditions under which the contained substitutions
are applied.
They can be applied to any element to specify conditions under which the contained substitutions or descriptors
are applied, based on feature settings passed via `--feature FeatureName bool`

```xml
<linker>
<assembly fullname="Assembly">
<!-- The substitution will apply only if "--feature EnableOptionalFeature false" are also used -->
<!-- This substitution will apply only if "EnableOptionalFeature" is set to "false" -->
<type fullname="Assembly.A" feature="EnableOptionalFeature" featurevalue="false">
<method signature="System.String TestMethod()" body="stub">
</method>
<method signature="System.String TestMethod()" body="stub" />
</type>
</assembly>
</linker>
```

`featuredefault="true"` can be used to indicate that this `featurevalue` is the default value for `feature`,
causing the contained substitutions or descriptors to be applied even when the feature setting is not passed to the linker.
Note that this will only have an effect where it is applied - the default value is not remembered or reused for other elements.

```xml
<linker>
<assembly fullname="Assembly">
<!-- This method will be preserved if "EnableDefaultFeature" is "true" or unspecified -->
<type fullname="Assembly.A" feature="EnableDefaultFeature" featurevalue="true" featuredefault="true">
<method signature="System.String TestMethod()" />
</type>
<!-- This method will only be preserved if "EnableDefaultFeature" is "true", not if it is unspecified-->
<type fullname="Assembly.A" feature="EnableDefaultFeature" featurevalue="true">
<method signature="System.String TestMethod2()" />
</type>
</assembly>
</linker>
Expand Down
12 changes: 8 additions & 4 deletions src/tools/illink/docs/error-codes.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ error and warning codes.

## Error Codes

#### `IL1001`: Failed to process XML substitution: 'XML document location'. Feature 'feature' does not specify a "featurevalue" attribute
#### `IL1001`: Failed to process 'XML document location'. Feature 'feature' does not specify a "featurevalue" attribute

- The substitution in 'XML document location' with feature value 'feature' does not use the `featurevalue` attribute. These attributes have to be used together.
- The substitution or descriptor in 'XML document location' with feature value 'feature' does not use the `featurevalue` attribute. These attributes have to be used together.

#### `IL1002`: Failed to process XML substitution: 'XML document location'. Unsupported non-boolean feature definition 'feature'
#### `IL1002`: Failed to process 'XML document location'. Unsupported non-boolean feature definition 'feature'

- The substitution in 'XML document location' with feature value 'feature' sets the attribute `featurevalue` to a non-boolean value. Only boolean values are supported for this attribute.
- The substitution or descriptor in 'XML document location' with feature value 'feature' sets the attribute `featurevalue` to a non-boolean value. Only boolean values are supported for this attribute.

#### `IL1003`: Error processing 'XML document name': 'XmlException'

Expand Down Expand Up @@ -55,6 +55,10 @@ error and warning codes.

- There was an error processing 'XML document location' xml file. The most likely reason for this is that the descriptor file has syntactical errors.

#### `IL1014`: Failed to process 'XML document location`. Unsupported value for featuredefault attribute

- The substitution or descriptor in 'XML document location' contains a 'featuredefault' attribute with an invalid value. This attribute only supports the true value, to indicate that this is the default behavior for a feature when a value is not given.

----
## Warning Codes

Expand Down
31 changes: 6 additions & 25 deletions src/tools/illink/src/linker/Linker.Steps/BodySubstituterStep.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Diagnostics;
using System.Linq;
using System.Globalization;
using System.Xml.XPath;
Expand Down Expand Up @@ -41,28 +42,7 @@ protected override void Process ()
ReadSubstitutions (_document);
}

bool ShouldProcessSubstitutions (XPathNavigator nav)
{
var feature = GetAttribute (nav, "feature");
if (string.IsNullOrEmpty (feature))
return true;

var value = GetAttribute (nav, "featurevalue");
if (string.IsNullOrEmpty (value)) {
Context.LogError ($"Failed to process XML substitution: '{_xmlDocumentLocation}'. Feature {feature} does not specify a 'featurevalue' attribute", 1001);
return false;
}

if (!bool.TryParse (value, out bool bValue)) {
Context.LogError ($"Failed to process XML substitution: '{_xmlDocumentLocation}'. Unsupported non-boolean feature definition {feature}", 1002);
return false;
}

if (Context.FeatureSettings == null || !Context.FeatureSettings.TryGetValue (feature, out bool featureSetting))
return false;

return bValue == featureSetting;
}
bool ShouldProcessSubstitutions (XPathNavigator nav) => FeatureSettings.ShouldProcessElement (nav, Context, _xmlDocumentLocation);

void ReadSubstitutions (XPathDocument document)
{
Expand All @@ -81,6 +61,8 @@ void ReadSubstitutions (XPathDocument document)
void ProcessAssemblies (XPathNodeIterator iterator)
{
while (iterator.MoveNext ()) {
// Errors for invalid assembly names should show up even if this element will be
// skipped due to feature conditions.
var name = GetAssemblyName (iterator.Current);

if (!ShouldProcessSubstitutions (iterator.Current))
Expand Down Expand Up @@ -126,10 +108,9 @@ void ProcessTypes (AssemblyDefinition assembly, XPathNodeIterator iterator)

void ProcessType (TypeDefinition type, XPathNavigator nav)
{
if (!nav.HasChildren)
return;
Debug.Assert (ShouldProcessSubstitutions (nav));

if (!ShouldProcessSubstitutions (nav))
if (!nav.HasChildren)
return;

XPathNodeIterator methods = nav.SelectChildren ("method", "");
Expand Down
61 changes: 22 additions & 39 deletions src/tools/illink/src/linker/Linker.Steps/LinkAttributesStep.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Text;
using System.Text.RegularExpressions;
Expand Down Expand Up @@ -153,15 +154,19 @@ protected override void Process ()
void ProcessAssemblies (LinkContext context, XPathNodeIterator iterator)
{
while (iterator.MoveNext ()) {
bool processAllAssemblies = GetFullName (iterator.Current) == "*";
// Errors for invalid assembly names should show up even if this element will be
// skipped due to feature conditions.
var name = processAllAssemblies ? null : GetAssemblyName (iterator.Current);

if (!ShouldProcessElement (iterator.Current))
return;
continue;

if (GetFullName (iterator.Current) == "*") {
foreach (AssemblyDefinition assemblyIterator in context.GetAssemblies ()) {
if (processAllAssemblies) {
foreach (AssemblyDefinition assemblyIterator in context.GetAssemblies ())
ProcessTypes (assemblyIterator, iterator, true);
}
} else {
AssemblyDefinition assembly = GetAssembly (context, GetAssemblyName (iterator.Current));
AssemblyDefinition assembly = GetAssembly (context, name);

if (assembly == null) {
Context.LogWarning ($"Could not resolve assembly {GetAssemblyName (iterator.Current).Name} specified in {_xmlDocumentLocation}", 2007, _xmlDocumentLocation);
Expand Down Expand Up @@ -249,6 +254,8 @@ void ProcessTypePattern (string fullname, AssemblyDefinition assembly, XPathNavi

void ProcessType (TypeDefinition type, XPathNavigator nav)
{
Debug.Assert (ShouldProcessElement (nav));

IEnumerable<CustomAttribute> attributes = ProcessAttributes (nav);
if (attributes.Count () > 0)
Context.CustomAttributes.AddCustomAttributes (type, attributes);
Expand Down Expand Up @@ -283,9 +290,9 @@ void ProcessSelectedFields (XPathNavigator nav, TypeDefinition type)
return;

while (fields.MoveNext ()) {
if (!ShouldProcessElement (fields.Current)) {
return;
}
if (!ShouldProcessElement (fields.Current))
continue;

string value = GetSignature (fields.Current);
if (!String.IsNullOrEmpty (value))
ProcessFieldSignature (type, value, fields);
Expand All @@ -303,9 +310,8 @@ void ProcessSelectedMethods (XPathNavigator nav, TypeDefinition type)
return;

while (methods.MoveNext ()) {
if (!ShouldProcessElement (methods.Current)) {
return;
}
if (!ShouldProcessElement (methods.Current))
continue;

string value = GetSignature (methods.Current);
if (!String.IsNullOrEmpty (value))
Expand All @@ -323,9 +329,8 @@ void ProcessSelectedProperties (XPathNavigator nav, TypeDefinition type)
if (properties.Count == 0)
return;
while (properties.MoveNext ()) {
if (!ShouldProcessElement (properties.Current)) {
return;
}
if (!ShouldProcessElement (properties.Current))
continue;

string value = GetSignature (properties.Current);
if (!String.IsNullOrEmpty (value))
Expand All @@ -343,9 +348,8 @@ void ProcessSelectedEvents (XPathNavigator nav, TypeDefinition type)
if (events.Count == 0)
return;
while (events.MoveNext ()) {
if (!ShouldProcessElement (events.Current)) {
return;
}
if (!ShouldProcessElement (events.Current))
continue;

string value = GetSignature (events.Current);
if (!String.IsNullOrEmpty (value))
Expand Down Expand Up @@ -600,27 +604,6 @@ static PropertyDefinition GetProperty (TypeDefinition type, string signature)
return null;
}

bool ShouldProcessElement (XPathNavigator nav)
{
var feature = GetAttribute (nav, "feature");
if (string.IsNullOrEmpty (feature))
return true;

var value = GetAttribute (nav, "featurevalue");
if (string.IsNullOrEmpty (value)) {
Context.LogError ($"Feature {feature} does not specify a \"featurevalue\" attribute", 1001);
return false;
}

if (!bool.TryParse (value, out bool bValue)) {
Context.LogError ($"Unsupported non-boolean feature definition {feature}", 1002);
return false;
}

if (Context.FeatureSettings == null || !Context.FeatureSettings.TryGetValue (feature, out bool featureSetting))
return false;

return bValue == featureSetting;
}
bool ShouldProcessElement (XPathNavigator nav) => FeatureSettings.ShouldProcessElement (nav, Context, _xmlDocumentLocation);
}
}
Loading

0 comments on commit d503ec8

Please sign in to comment.