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.

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

Restore assembly-level CLSCompliant true & remove method-level instances of false CLS Compliance #584

Merged
merged 3 commits into from Mar 15, 2019

Conversation

Projects
None yet
4 participants
@Shyam-Gupta
Copy link
Member

Shyam-Gupta commented Mar 13, 2019

  1. Changed ClsCompliant to true at assembly level for all 3 implementation assemblies
  2. (1) caused some compile time errors for instances in System.Windows.Forms where we have declared ClsCompliant to false at some methods and class levels. I removed those instances as all of them were in internal classes.

After this change there are no compile time errors and both unit tests and integration test passes.

I am not sure if there can be any runtime implications because of this change.

Shyam-Gupta added some commits Mar 13, 2019

1. Setting ClsCompliant to true for implementation assemblies
2. Removed ClsCompliant(false) at method levels wherever we were getting compilation error due to (1)

@Shyam-Gupta Shyam-Gupta requested a review from dotnet/dotnet-winforms as a code owner Mar 13, 2019

@zsd4yr zsd4yr changed the title Dev/shgu/clscompliant Restore assembly-level CLSCompliant true & explicitly remove other instances of false CLS Compliance Mar 13, 2019

@zsd4yr zsd4yr changed the title Restore assembly-level CLSCompliant true & explicitly remove other instances of false CLS Compliance Restore assembly-level CLSCompliant true & remove method-level instances of false CLS Compliance Mar 13, 2019

@zsd4yr

zsd4yr approved these changes Mar 13, 2019

@zsd4yr

This comment has been minimized.

@sharwell

This comment has been minimized.

Copy link
Member

sharwell commented Mar 14, 2019

Changed ClsCompliant to true at assembly level for all 3 implementation assemblies

Why is this needed? The most significant impact I'm aware of these days is it makes the build slower. 😄

(1) caused some compile time errors for instances in System.Windows.Forms where we have declared ClsCompliant to false at some methods and class levels. I removed those instances as all of them were in internal classes.

I'm really curious about this. I expected the use of CLSCompliant(true) at the assembly level to resolve these warnings, not cause them. Can you give an example of the type of warning you saw?

@zsd4yr

This comment has been minimized.

Copy link
Member

zsd4yr commented Mar 14, 2019

@Shyam-Gupta

This comment has been minimized.

Copy link
Member Author

Shyam-Gupta commented Mar 14, 2019

@sharwell

On removing CLSCompliant attribute from implementation assemblies, I get lots of errors like:

System\Windows\Forms\AxHost.cs(4657,32): error CS3021: 'AxHost.GetColorFromOleColor(uint)' does not need a CLSCompliant attribute because the assembly does not have a CLSCompliant attribute [C:\gnew\winforms\src\System.Windows.Forms\src\System.Windows.Forms.csproj]
System\Windows\Forms\AxHost.cs(4667,31): error CS3021: 'AxHost.GetOleColorFromColor(Color)' does not need a CLSCompliant attribute because the assembly does not have a CLSCompliant attribute [C:\gnew\winforms\src\System.Windows.Forms\src\System.Windows.Forms.csproj]
System\Windows\Forms\ComponentModel\COM2Interop_CTLBLDTYPE.cs(11,20): error CS3021: '_CTLBLDTYPE' does not need a CLSCompliant attribute because the assembly does not have a CLSCompliant attribute [C:\gnew\winforms\src\System.Windows.Forms\src\System.Windows.Forms.csproj]
C:\gnew\winforms\src\Common\src\UnsafeNativeMethods.cs(8347,28): error CS3021: 'UnsafeNativeMethods.IAccessibleEx' does not need a CLSCompliant attribute because the assembly does not have a CLSCompliant attribute [C:\gnew\winforms\src\System.Windows.Forms\src\System.Windows.Forms.csproj]
C:\gnew\winforms\src\Common\src\UnsafeNativeMethods.cs(8466,26): error CS3021: 'UnsafeNativeMethods.IRawElementProvider
Simple' does not need a CLSCompliant attribute because the assembly does not have a CLSCompliant attribute [C:\gnew\winforms\src\System.Windows.Forms\src\System.Windows.Forms.csproj]
C:\gnew\winforms\src\Common\src\UnsafeNativeMethods.cs(8546,26): error CS3021: 'UnsafeNativeMethods.IRawElementProviderFragment' does not need a CLSCompliant attribute because the assembly does not have a CLSCompliant attribute [C:\gnew\winforms\src\System.Windows.Forms\src\System.Windows.Forms.csproj]

- Merge from master.
- Resolved conflicts.
@sharwell

This comment has been minimized.

Copy link
Member

sharwell commented Mar 15, 2019

Error CS3021: 'AxHost.GetColorFromOleColor(uint)' does not need a CLSCompliant attribute because the assembly does not have a CLSCompliant attribute

@Shyam-Gupta This means this change is not adding [CLSCompliant(true)], it's simply removing [CLSCompliant(false)]. I need to look into the targets tomorrow and figure out why it's not working correctly.

@sharwell
Copy link
Member

sharwell left a comment

This change appears to be broken; I need to investigate in the morning.

@sharwell
Copy link
Member

sharwell left a comment

The change appears fine. The errors reported by @Shyam-Gupta are not the errors that are reported when [CLSCompliant(true)] is used, so it was a bit misleading. I'm not sure how that build ended up without a CLSCompliant attribute, but the code in this pull request should not result in that state. The errors that led to the removal of [CLSCompliant(false)] from types and/or members is this:

1>System\Windows\Forms\ComponentModel\COM2Interop\_CTLBLDTYPE.cs(11,20,11,31): warning CS3019: CLS compliance checking will not be performed on '_CTLBLDTYPE' because it is not visible from outside this assembly
1>System\Windows\Forms\SafeNativeMethods.cs(585,36,585,52): warning CS3019: CLS compliance checking will not be performed on 'SafeNativeMethods._TrackMouseEvent(NativeMethods.TRACKMOUSEEVENT)' because it is not visible from outside this assembly
1>System\Windows\Forms\NativeMethods.cs(2153,22,2153,28): warning CS3019: CLS compliance checking will not be performed on 'NativeMethods.OLECMD' because it is not visible from outside this assembly
1>System\Windows\Forms\NativeMethods.cs(2166,26,2166,43): warning CS3019: CLS compliance checking will not be performed on 'NativeMethods.IOleCommandTarget' because it is not visible from outside this assembly
1>System\Windows\Forms\NativeMethods.cs(3284,22,3284,32): warning CS3019: CLS compliance checking will not be performed on 'NativeMethods.CHOOSEFONT' because it is not visible from outside this assembly
1>System\Windows\Forms\NativeMethods.cs(3432,30,3432,37): warning CS3019: CLS compliance checking will not be performed on 'NativeMethods._POINTL' because it is not visible from outside this assembly
1>System\Windows\Forms\UnsafeNativeMethods.cs(8347,28,8347,41): warning CS3019: CLS compliance checking will not be performed on 'UnsafeNativeMethods.IAccessibleEx' because it is not visible from outside this assembly
1>System\Windows\Forms\UnsafeNativeMethods.cs(8466,26,8466,51): warning CS3019: CLS compliance checking will not be performed on 'UnsafeNativeMethods.IRawElementProviderSimple' because it is not visible from outside this assembly
1>System\Windows\Forms\UnsafeNativeMethods.cs(8546,26,8546,53): warning CS3019: CLS compliance checking will not be performed on 'UnsafeNativeMethods.IRawElementProviderFragment' because it is not visible from outside this assembly
1>System\Windows\Forms\UnsafeNativeMethods.cs(8612,26,8612,57): warning CS3019: CLS compliance checking will not be performed on 'UnsafeNativeMethods.IRawElementProviderFragmentRoot' because it is not visible from outside this assembly
1>System\Windows\Forms\UnsafeNativeMethods.cs(8648,26,8648,41): warning CS3019: CLS compliance checking will not be performed on 'UnsafeNativeMethods.IToggleProvider' because it is not visible from outside this assembly
1>System\Windows\Forms\UnsafeNativeMethods.cs(8669,26,8669,40): warning CS3019: CLS compliance checking will not be performed on 'UnsafeNativeMethods.ITableProvider' because it is not visible from outside this assembly
1>System\Windows\Forms\UnsafeNativeMethods.cs(8686,26,8686,44): warning CS3019: CLS compliance checking will not be performed on 'UnsafeNativeMethods.ITableItemProvider' because it is not visible from outside this assembly
1>System\Windows\Forms\UnsafeNativeMethods.cs(8699,26,8699,39): warning CS3019: CLS compliance checking will not be performed on 'UnsafeNativeMethods.IGridProvider' because it is not visible from outside this assembly
1>System\Windows\Forms\UnsafeNativeMethods.cs(8719,26,8719,43): warning CS3019: CLS compliance checking will not be performed on 'UnsafeNativeMethods.IGridItemProvider' because it is not visible from outside this assembly
1>System\Windows\Forms\UnsafeNativeMethods.cs(8761,26,8761,41): warning CS3019: CLS compliance checking will not be performed on 'UnsafeNativeMethods.IInvokeProvider' because it is not visible from outside this assembly
1>System\Windows\Forms\UnsafeNativeMethods.cs(8780,26,8780,45): warning CS3019: CLS compliance checking will not be performed on 'UnsafeNativeMethods.IScrollItemProvider' because it is not visible from outside this assembly

@Shyam-Gupta Shyam-Gupta merged commit 78ae911 into master Mar 15, 2019

1 check passed

license/cla All CLA requirements met.
Details

@Shyam-Gupta Shyam-Gupta deleted the dev/shgu/clscompliant branch Mar 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.