[Mono-devel-list] Re: System.XML warning

[Marek Safar]

Hello,

Comments in lined.

>>
>> System.Xml/XmlReader.cs(1051) warning CS0169: The private method 
>> 'System.Xml.XmlReader.CheckSupport()' is never used
>
>
> This is used in NET_2_0 part of code.
> Should we enclose ALL of such members in _nasty_ preprocessor
> directives? Does it ease developers read it?


Yes, it is easier to read and easier to maintain. If you don't enclose 
your members in #if NET_2_0 directive how can I know
that a field is not valid for NET_1_x ?. I have to search in your code 
and try to find out whether the field is valid or not.

>> System.Xml/XmlTextReader.cs(1672) warning CS0168: The variable 
>> 'dummyValue' is declared but never used
>
>
> It is _required_ even though it is actually not used (otherwise mcs/csc
> will reject this code). CSC never reports it as warning and I think
> that regarding it as a warning is bad design.
>
Is it some managed <-> unmanaged work ?

>> System.Xml.Xsl/XslTransform.cs(604) warning CS0169: The private 
>> method 'System.Xml.Xsl.UnmanagedXslTransform.xsltCleanupGlobals()' is 
>> never used
>> System.Xml.Xsl/XslTransform.cs(611) warning CS0169: The private 
>> method 'System.Xml.Xsl.UnmanagedXslTransform.xmlNewDoc( string)' is 
>> never used
>> System.Xml.Xsl/XslTransform.cs(614) warning CS0169: The private 
>> method 'System.Xml.Xsl.UnmanagedXslTransform.xmlSaveFile( string,  
>> System.IntPtr)' is never used
>> System.Xml.Xsl/XslTransform.cs(626) warning CS0169: The private 
>> method 'System.Xml.Xsl.UnmanagedXslTransform.xmlCleanupParser()' is 
>> never used
>> System.Xml.Xsl/XslTransform.cs(629) warning CS0169: The private 
>> method 'System.Xml.Xsl.UnmanagedXslTransform.xmlDocDumpMemory( 
>> System.IntPtr, ref System.IntPtr&, ref int&)' is never used
>> System.Xml.Xsl/XslTransform.cs(632) warning CS0169: The private 
>> method 'System.Xml.Xsl.UnmanagedXslTransform.xmlFree( System.IntPtr)' 
>> is never used
>> System.Xml.Xsl/XslTransform.cs(649) warning CS0169: The private 
>> method 'System.Xml.Xsl.UnmanagedXslTransform.valuePop( 
>> System.IntPtr)' is never used
>> System.Xml.Xsl/XslTransform.cs(652) warning CS0169: The private 
>> method 'System.Xml.Xsl.UnmanagedXslTransform.valuePush( 
>> System.IntPtr,  System.Xml.Xsl.UnmanagedXslTransform.xpathobject*)' 
>> is never used
>> System.Xml.Xsl/XslTransform.cs(655) warning CS0169: The private 
>> method 'System.Xml.Xsl.UnmanagedXslTransform.xmlXPathFreeObject( 
>> System.Xml.Xsl.UnmanagedXslTransform.xpathobject*)' is never used
>> System.Xml.Xsl/XslTransform.cs(658) warning CS0169: The private 
>> method 'System.Xml.Xsl.UnmanagedXslTransform.xmlXPathNewCString( 
>> string)' is never used
>> System.Xml.Xsl/XslTransform.cs(661) warning CS0169: The private 
>> method 'System.Xml.Xsl.UnmanagedXslTransform.xmlXPathNewFloat( 
>> double)' is never used
>> System.Xml.Xsl/XslTransform.cs(664) warning CS0169: The private 
>> method 'System.Xml.Xsl.UnmanagedXslTransform.xmlXPathNewBoolean( 
>> int)' is never used
>> System.Xml.Xsl/XslTransform.cs(667) warning CS0169: The private 
>> method 'System.Xml.Xsl.UnmanagedXslTransform.xmlXPathNewNodeSet( 
>> System.IntPtr)' is never used
>> System.Xml.Xsl/XslTransform.cs(670) warning CS0169: The private 
>> method 'System.Xml.Xsl.UnmanagedXslTransform.xmlXPathCastToBoolean( 
>> System.Xml.Xsl.UnmanagedXslTransform.xpathobject*)' is never used
>> System.Xml.Xsl/XslTransform.cs(673) warning CS0169: The private 
>> method 'System.Xml.Xsl.UnmanagedXslTransform.xmlXPathCastToNumber( 
>> System.Xml.Xsl.UnmanagedXslTransform.xpathobject*)' is never used
>> System.Xml.Xsl/XslTransform.cs(676) warning CS0169: The private 
>> method 'System.Xml.Xsl.UnmanagedXslTransform.xmlXPathCastToString( 
>> System.Xml.Xsl.UnmanagedXslTransform.xpathobject*)' is never used
>> System.Xml.Xsl/XslTransform.cs(679) warning CS0169: The private 
>> method 'System.Xml.Xsl.UnmanagedXslTransform.xmlXPathStringFunction( 
>> System.IntPtr,  int)' is never used
>
>
> Why are they reported as never used? They are actually in use.


Are you sure that they are used ?

I tried to remove 
'System.Xml.Xsl.UnmanagedXslTransform.xsltCleanupGlobals' and no 
compilation error.


>> System.Xml.Schema/XmlSchemaSet.cs(134) warning CS3019: CLS compliance 
>> checking will not be performed on 
>> 'System.Xml.Schema.XmlSchemaSet.XmlResolver' because it is private or 
>> internal
>
>
> This is reported just because this type is public only under NET_2_0.
> When there are such types, should we put NET_2_0 on every such
> attributes?

You can keep it. In this case Microsoft made custom property modifiers 
CLS-Compliant (similar case as generics). I am going to remove this 
warning soon.

>
> Those changes will be checked in after the upcoming release task.

Great.

>
> BTW how can we verify that those sources became clean after
> manual warning elimination?


I will send you mcs patch.

>
> Also, I think that some part of those warning report feature are
> problematic. For example,
>
>     - I think there is no (or little) need to report not-in-use
>       private methods. When we found such methods that we don't
>       know what it is, we usually grep (usually) single file
>       and notice that it is not in use. I believe that why MS
>       csc has such reporting feature for unused fields is that
>       they might be confused with local variables or parameters.
>       I don't think mcs should not let developers to be puritan
>       that believes ALL unused members MUST be eliminated.

 From my point of view, there are several reasons why report "unused" code.
- I like every feature which tells me that something is "wrong" with my 
code.
- Your code can be easily faster/smaller. When you remove class member 
you allocate less.
When you remove method sometimes it leads to next methods/fields clean up.

Maybe I am wrong but why should I keep unused code ?

>
>     - As I noted above, there are unexpected unused field check
>       that at least csc does not regard as should-be-warned.
>
If you know about any wrong warnings please let me know.
But csc is really not good in this area. They made some improvement for 
2.0. But even
simple cases are still not detected.

Marek