Hello,
Marek Safar wrote:
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.
IMO having ifdefs are much more nasty than just having unused fields.
If there are ifdefs in effect, in every block (not only field declarations) you must grep up to find whether it is valid block
or not.
If those warning output REALLY annoys developers, I'd just add /nowarn:169 as well as 162, 612 and 618.
Is it some managed <-> unmanaged work ?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.
If you tried to remove that assignment statement for the dummy value, csc will complain that:
System.Xml\XmlTextReader.cs(1674,23): error CS0201: Only
assignment, call, increment, decrement, and new object
expressions can be used as a statementand mcs saying that:
syntax error, got token `SEMICOLON'
System.Xml\XmlTextReader.cs(1674) error CS1002: Expecting `;'Well, aside this code, there is possibility that those extra fields might be used from other assemblies. For example, I used to have XQueryCommandImpl type and field in System.Xml.dll and referenced it from System.Data.SqlXml.dll via reflection.
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.
Mmm, sorry then I should have checked more in details. At least
>>> System.Xml.Xsl/XslTransform.cs(649) warning CS0169: The private >>> method 'System.Xml.Xsl.UnmanagedXslTransform.valuePop( >>> System.IntPtr)' is never used
this is really in use.
I'll check for each unmanaged methods and comment out if they are unused.
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.
Ok, cool.
BTW how can we verify that those sources became clean after manual warning elimination?
I will send you mcs patch.
Please don't send me but post it to the mail list so that everyone can have a chance to try.
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 ?
From my point of view, there are many reasons not to report "unused" code. - I like every feature which tells me that something is "wrong" with my code, only when it is REALLY wrong. - It is too trivial contribution to performance to remove unused code, unless there are heavy reflection work. - Those puritan warnings really discourages us at every point of incomplete development where we wonder if our refactory is fine. For example, suppose we were developing UnmanagedXslTransform and going to support all exposed functionality in unmanaged library, Checking every import is just a pain.
If you know about any wrong warnings please let me know.
- As I noted above, there are unexpected unused field check that at least csc does not regard as should-be-warned.
As for that unused field, I think I explaind understandably, though I don't know that is the merginal line between those code that is worthy of being warned and those code that is not.
But csc is really not good in this area. They made some improvement for 2.0. But even
simple cases are still not detected.
Yes, csc 2.0 seems added some additional code analysis. But I doubt if the remaining check is "better". It sounds like design principle difference.
Atsushi Eno _______________________________________________ Mono-devel-list mailing list [email protected] http://lists.ximian.com/mailman/listinfo/mono-devel-list
