Conversation
|
Can we treat these as errors so that they do not come back? Are there more outside corelib? |
d9b36df to
13c505d
Compare
Enabled rule and fixed existing sources. |
| [return: MarshalUsing(typeof(UniqueComInterfaceMarshaller<IErrorInfo>))] | ||
| IErrorInfo GetErrorInfo( | ||
| [PreserveSig] | ||
| System.Data.OleDb.OleDbHResult GetErrorInfo( |
There was a problem hiding this comment.
[In][Out] weren't enough for COM as it was issuing another warning, so I had to align its with other methods in this file; PreserveSig+outarg (it was an odd one out).
There was a problem hiding this comment.
What warning were you seeing before switching this to PreserveSig?
There was a problem hiding this comment.
It looked different from the others:
error SYSLIB1092: The return value in the managed definition will be converted to an 'out' parameter when calling the unmanaged COM method. If the return value is intended to be the HRESULT code returned by the unmanaged COM method, use '[PreserveSig]' on the method. (https://learn.microsoft.com/dotnet/fundamentals/syslib-diagnostics/syslib1092)
There was a problem hiding this comment.
I thought that one should only fire on return types of "int" or "int"-sized enums...
We shouldn't be forcing everyone to use PreserveSig.
There was a problem hiding this comment.
We shouldn't be forcing everyone to use PreserveSig.
Is this something this PR should be fixing or wait for it to get fixed in main then rebase?
There was a problem hiding this comment.
The problem is on just GetRecordCount, where the diagnostic is accurate (int represents an out argument, not the HResult reuturn). For that case, it is a lot clearer to use [PreserveSig] to remove ambiguity.
@am11 can we revert the changes to GetErrorInfo?
|
This pull request has been automatically marked |
Fix #122755.