Closed edgarfgp closed 4 months ago
[!WARNING] Author opted out of release notes, check is disabled for this pull request. cc @dotnet/fsharp-team-msft
AllowNullLiteralAttribute
needs updating
Interesting. VS Integration is using System.Runtime.InteropServices.ClassInterface
attribute docs which is meant to be used only on a class
/// <summary>Indicates the type of class interface to be generated for a class exposed to COM, if an interface is generated at all.</summary>
[AttributeUsage(AttributeTargets.Assembly | AttributeTargets.Class, Inherited = false)]
public sealed class ClassInterfaceAttribute : Attribute
{
/// <summary>Initializes a new instance of the <see cref="T:System.Runtime.InteropServices.ClassInterfaceAttribute" /> class with the specified <see cref="T:System.Runtime.InteropServices.ClassInterfaceType" /> enumeration member.</summary>
/// <param name="classInterfaceType">One of the <see cref="T:System.Runtime.InteropServices.ClassInterfaceType" /> values that describes the type of interface that is generated for a class.</param>
public ClassInterfaceAttribute(ClassInterfaceType classInterfaceType)
{
this.Value = classInterfaceType;
}
[<System.Runtime.InteropServices.ClassInterface(ClassInterfaceType.None)>] // This is allowed but should not
type public IVsMicrosoftInstalledProduct =
inherit IVsInstalledProduct
abstract IdBmpSplashM : byref<uint32> -> unit
[<System.Runtime.InteropServices.ClassInterface(ClassInterfaceType.None)>]
type Foo() = class end
using System.Runtime.InteropServices;
[ClassInterface(ClassInterfaceType.AutoDispatch)]
public class MyClass
{
}
[ClassInterface(ClassInterfaceType.AutoDispatch)] // This raises an error in C#
public interface IFoo {}
cc @vzarytovskii seems like System.Runtime.InteropServices.ClassInterface
has been wrongly used here ?
Not sure what to do with it, it'll probably need to be an abstract class. This is legacy project system, so won't be able to test it that easy, and we have to support it.
Not sure what to do with it, it'll probably need to be an abstract class. This is legacy project system, so won't be able to test it that easy, and we have to support it.
Yeah an AbstractClass
will be ok here but it won't be able to inherit from an interface IVsInstalledProduct
(seems to be a close source so I can't see the defined members)
Ohh Thanks my googling failed 😅
Yeah, we don't actually implement it, so it's not really used as the docs prescribe. This is some ancient COM trickery that worked long time ago but not necessarily still do.
In fact I just comment out that whole IVsMicrosoftInstalledProduct
interface, started VS debug instance and things seem to work the same without it:
Yeah, we don't actually implement it, so it's not really used as the docs prescribe. This is some ancient COM trickery that worked long time ago but not necessarily still do.
In fact I just comment out that whole
IVsMicrosoftInstalledProduct
interface, started VS debug instance and things seem to work the same without it
If it is not longer used/needed we could just comment out / remove this code.
ideally enforcing the right attribute targets in the compiler should not depend on the use of this interface
I'd really like this change and for the compiler to enforce attribute targets, but technically this would be a backward incompatible change, right? Do we have a lang suggestion or bug or issue report (I believe this has come up several times)?
Ah, wait, found it: #8547.
And related are all these: #16692, #16764, #16845, #16790. I assume my point above has already been discussed before then ;).
And there's mention of an RFC by you (@edgarfgp), can we link that from the original post above, or is that TODO? https://github.com/dotnet/fsharp/issues/8547#issuecomment-1952063131
I'd really like this change and for the compiler to enforce attribute targets, but technically this would be a backward incompatible change, right? Do we have a lang suggestion or bug or issue report (I believe this has come up several times)?
Ah, wait, found it: #8547.
And related are all these: #16692, #16764, #16845, #16790. I assume my point above has already been discussed before then ;).
And there's mention of an RFC by you (@edgarfgp), can we link that from the original post above, or is that TODO? #8547 (comment)
@abelbraaksma Once I have addressed all the AttributeTargets
inconsistencies I would be willing to write an RFC to properly spec this. This is behind a LangVersion preview.
Description
Enforce
AttributeTargets.Interface
AttributeTargets.Interface
should only work oninterfaces
AttributeTargets.Class
should only work onclasses
See C# version for reference sharplab
Checklist