Conditional

erik-kallen commented

2013-03-09 07:19:51 -06:00

(Migrated from github.com)

Adds properties ConditionalSymbols to ITypeDefinition and IMethod and IsConditionallyRemoved to IAttribute and InvocationResolveResult

Adds properties `ConditionalSymbols` to `ITypeDefinition` and `IMethod` and `IsConditionallyRemoved` to `IAttribute` and `InvocationResolveResult`

dgrunwald commented

2013-03-09 07:21:01 -06:00

(Migrated from github.com)

On ITypeDefinition and IMethod? I don't see the ConditionalSymbols having any use there. Worse: with partial types/methods, a single type/method can have multiple different sets of conditional symbols.

On `ITypeDefinition` and `IMethod`? I don't see the `ConditionalSymbols` having any use there. Worse: with partial types/methods, a single type/method can have multiple different sets of conditional symbols.

dgrunwald commented

2013-03-09 07:23:52 -06:00

(Migrated from github.com)

Ah so it's not the conditional symbols at the position of the method, but those expected by the attribute? I don't think we need an extra interface member for those; there's few code that needs access to it and it can just look through the Attributes collection.

erik-kallen commented

2013-03-09 07:48:19 -06:00

(Migrated from github.com)

If they don't go there, I think the FindConditionalSymbols should go somewhere where it's useful (perhaps as an extension method). Isn't it an advantage to cache them, though? For the property on InvocationResolveResult, the collection will be examined for each invocation of the method (or it would need to be stored in a cache in the resolver).

If they don't go there, I think the `FindConditionalSymbols` should go somewhere where it's useful (perhaps as an extension method). Isn't it an advantage to cache them, though? For the property on `InvocationResolveResult`, the collection will be examined for each invocation of the method (or it would need to be stored in a cache in the resolver).

dgrunwald commented

2013-03-09 07:52:33 -06:00

(Migrated from github.com)

Types rarely have more than a couple of attributes. Each attribute already caches its resolved type and arguments; so there's little cost in running FindConditionalSymbols repeatedly.
Also, you shouldn't check the assembly name against mscorlib. The Microsoft C# compiler does no such checks; and I think the portable library reference assemblies have moved ConditionalAttribute to System.Runtime.dll.

Types rarely have more than a couple of attributes. Each attribute already caches its resolved type and arguments; so there's little cost in running `FindConditionalSymbols` repeatedly. Also, you shouldn't check the assembly name against `mscorlib`. The Microsoft C# compiler does no such checks; and I think the portable library reference assemblies have moved `ConditionalAttribute` to `System.Runtime.dll`.

erik-kallen commented

2013-03-09 07:55:52 -06:00

(Migrated from github.com)

So where do you think the method should go?

erik-kallen commented

2013-03-09 08:12:36 -06:00

(Migrated from github.com)

OK, rebased and created a new commit without those members.

erik-kallen commented

2013-08-19 14:14:41 -05:00

(Migrated from github.com)

Why don't you merge this? Is there a problem with it? The feature is quite useful (especially for attributes since that information is not available elsewhere)

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Rows
Columns

Conditional #149

Pull request closed