This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Enable generic attributes #9189
Merged
Merged
Changes from 19 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
257a880
Merge pull request #1 from dotnet/master
AviAvni f531ffe
Merge pull request #2 from dotnet/master
AviAvni 0182671
enable generic attributes
AviAvni dc0e9c3
revert delete check for attribute as generic variable
AviAvni 9c08415
fix tab vs space issue
AviAvni 8d3e409
Merge pull request #3 from dotnet/master
AviAvni 3cda555
add tests for generic attribute
AviAvni 5730374
Merge pull request #4 from dotnet/master
AviAvni 66c981a
Merge branch 'master' into generic-attributes
AviAvni a22d6fc
seperate metadata and tests
AviAvni 9c4ed61
add project files
AviAvni 587fddd
fix il reference
AviAvni 91e19b1
fix test by return 100
AviAvni 4cb4eba
change resolve method
AviAvni 33becfe
add declaring type so MetaSig handle shared generic ctor
AviAvni 11c28a8
fix review issues
AviAvni 69603dc
throw NotSupportedException when reqesting open generic attribute
AviAvni 4305967
Update GenericAttributeTests.cs
AviAvni 0a4df91
Revert "throw NotSupportedException when reqesting open generic attri…
AviAvni 166860f
Merge branch 'master' into generic-attributes
AviAvni File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -670,7 +670,7 @@ FCIMPL2(Object*, RuntimeTypeHandle::CreateCaInstance, ReflectClassBaseObject* pC | |
CONTRACTL { | ||
FCALL_CHECK; | ||
PRECONDITION(CheckPointer(pCaTypeUNSAFE)); | ||
PRECONDITION(!pCaTypeUNSAFE->GetType().IsGenericVariable()); | ||
PRECONDITION(!pCaTypeUNSAFE->GetType().IsGenericVariable()); | ||
PRECONDITION(pCaTypeUNSAFE->GetType().IsValueType() || CheckPointer(pCtorUNSAFE)); | ||
} | ||
CONTRACTL_END; | ||
|
@@ -694,10 +694,6 @@ FCIMPL2(Object*, RuntimeTypeHandle::CreateCaInstance, ReflectClassBaseObject* pC | |
PRECONDITION( | ||
(!pCtor && gc.refCaType->GetType().IsValueType() && !gc.refCaType->GetType().GetMethodTable()->HasDefaultConstructor()) || | ||
(pCtor == gc.refCaType->GetType().GetMethodTable()->GetDefaultConstructor())); | ||
|
||
// If we relax this, we need to insure custom attributes construct properly for Nullable<T> | ||
if (gc.refCaType->GetType().HasInstantiation()) | ||
COMPlusThrow(kNotSupportedException, W("Argument_GenericsInvalid")); | ||
|
||
gc.o = pCaMT->Allocate(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does this work for Nullable types that this comment is talking about? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you mean that the generic argument is nullable so I tried with int? and it works great too |
||
|
||
|
@@ -731,16 +727,20 @@ FCIMPL2(Object*, RuntimeTypeHandle::CreateCaInstance, ReflectClassBaseObject* pC | |
} | ||
FCIMPLEND | ||
|
||
FCIMPL5(LPVOID, COMCustomAttribute::CreateCaObject, ReflectModuleBaseObject* pAttributedModuleUNSAFE, ReflectMethodObject *pMethodUNSAFE, BYTE** ppBlob, BYTE* pEndBlob, INT32* pcNamedArgs) | ||
FCIMPL6(LPVOID, COMCustomAttribute::CreateCaObject, ReflectModuleBaseObject* pAttributedModuleUNSAFE, ReflectClassBaseObject* pCaTypeUNSAFE, ReflectMethodObject *pMethodUNSAFE, BYTE** ppBlob, BYTE* pEndBlob, INT32* pcNamedArgs) | ||
{ | ||
FCALL_CONTRACT; | ||
|
||
struct | ||
{ | ||
REFLECTCLASSBASEREF refCaType; | ||
OBJECTREF ca; | ||
REFLECTMETHODREF refCtor; | ||
REFLECTMODULEBASEREF refAttributedModule; | ||
} gc; | ||
gc.refCaType = (REFLECTCLASSBASEREF)ObjectToOBJECTREF(pCaTypeUNSAFE); | ||
TypeHandle th = gc.refCaType->GetType(); | ||
|
||
gc.ca = NULL; | ||
gc.refCtor = (REFLECTMETHODREF)ObjectToOBJECTREF(pMethodUNSAFE); | ||
gc.refAttributedModule = (REFLECTMODULEBASEREF)ObjectToOBJECTREF(pAttributedModuleUNSAFE); | ||
|
@@ -749,10 +749,10 @@ FCIMPL5(LPVOID, COMCustomAttribute::CreateCaObject, ReflectModuleBaseObject* pAt | |
FCThrowRes(kArgumentNullException, W("Arg_InvalidHandle")); | ||
|
||
MethodDesc* pCtorMD = gc.refCtor->GetMethod(); | ||
|
||
HELPER_METHOD_FRAME_BEGIN_RET_PROTECT(gc); | ||
{ | ||
MethodDescCallSite ctorCallSite(pCtorMD); | ||
MethodDescCallSite ctorCallSite(pCtorMD, th); | ||
MetaSig* pSig = ctorCallSite.GetMetaSig(); | ||
BYTE* pBlob = *ppBlob; | ||
|
||
|
@@ -767,10 +767,6 @@ FCIMPL5(LPVOID, COMCustomAttribute::CreateCaObject, ReflectModuleBaseObject* pAt | |
OBJECTREF *argToProtect = (OBJECTREF*)_alloca(cArgs * sizeof(OBJECTREF)); | ||
memset((void*)argToProtect, 0, cArgs * sizeof(OBJECTREF)); | ||
|
||
// If we relax this, we need to insure custom attributes construct properly for Nullable<T> | ||
if (pCtorMD->GetMethodTable()->HasInstantiation()) | ||
COMPlusThrow(kNotSupportedException, W("Argument_GenericsInvalid")); | ||
|
||
// load the this pointer | ||
argToProtect[0] = pCtorMD->GetMethodTable()->Allocate(); // this is the value to return after the ctor invocation | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
78 changes: 78 additions & 0 deletions
78
tests/src/reflection/GenericAttribute/GenericAttributeMetadata.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
// | ||
|
||
using System; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please add license header? |
||
using System.Reflection; | ||
using System.Collections; | ||
using System.Runtime.CompilerServices; | ||
|
||
[assembly: SingleAttribute<int>()] | ||
[assembly: SingleAttribute<bool>()] | ||
|
||
[assembly: MultiAttribute<int>()] | ||
[assembly: MultiAttribute<int>(1)] | ||
[assembly: MultiAttribute<int>(Value = 2)] | ||
[assembly: MultiAttribute<bool>()] | ||
[assembly: MultiAttribute<bool>(true)] | ||
|
||
[AttributeUsage(AttributeTargets.Assembly | AttributeTargets.Class | AttributeTargets.Property, AllowMultiple = false)] | ||
public class SingleAttribute<T> : Attribute | ||
{ | ||
|
||
} | ||
|
||
[AttributeUsage(AttributeTargets.Assembly | AttributeTargets.Class | AttributeTargets.Property, AllowMultiple = true)] | ||
public class MultiAttribute<T> : Attribute | ||
{ | ||
public T Value { get; set; } | ||
|
||
public MultiAttribute() | ||
{ | ||
} | ||
|
||
public MultiAttribute(T value) | ||
{ | ||
Value = value; | ||
} | ||
} | ||
|
||
public enum MyEnum | ||
{ | ||
Ctor, | ||
Property | ||
} | ||
|
||
[SingleAttribute<int>()] | ||
[SingleAttribute<bool>()] | ||
[MultiAttribute<int>()] | ||
[MultiAttribute<int>(1)] | ||
[MultiAttribute<int>(Value = 2)] | ||
[MultiAttribute<bool>()] | ||
[MultiAttribute<bool>(true)] | ||
[MultiAttribute<bool>(Value = true)] | ||
[MultiAttribute<bool?>()] | ||
[MultiAttribute<string>("Ctor")] | ||
[MultiAttribute<string>(Value = "Property")] | ||
[MultiAttribute<Type>(typeof(Class))] | ||
[MultiAttribute<Type>(Value = typeof(Class.Derive))] | ||
[MultiAttribute<MyEnum>(MyEnum.Ctor)] | ||
[MultiAttribute<MyEnum>(Value = MyEnum.Property)] | ||
public class Class | ||
{ | ||
public class Derive : Class | ||
{ | ||
|
||
} | ||
|
||
[SingleAttribute<int>()] | ||
[SingleAttribute<bool>()] | ||
[MultiAttribute<int>()] | ||
[MultiAttribute<int>(1)] | ||
[MultiAttribute<int>(Value = 2)] | ||
[MultiAttribute<bool>()] | ||
[MultiAttribute<bool>(true)] | ||
public int Property { get; set; } | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this precondition is still valid. This is guarding against something like this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@atsushikan You right sorry I revert this delete and validated it's still work