[Mono-dev] Re: [Mono-patches] r56609 - trunk/mcs/errors

[Raja R Harinath]

Hi,

Atsushi Eno <atsushi at ximian.com> writes:

> Hi,
>
>>> 	  It reports XmlTextReader error message changes as build breakage.
>>>
>>> 	  That's why I opposed to compare error messages blindly.
>> While I understand your point of view, I've come to appreciate this
>> "feature" of the compiler testsuite.
>>
>>> Modified: trunk/mcs/errors/cs1570-10.cs
>>> ===================================================================
>>> --- trunk/mcs/errors/cs1570-10.cs	2006-02-07 06:54:20 UTC (rev 56608)
>>> +++ trunk/mcs/errors/cs1570-10.cs	2006-02-07 07:57:08 UTC (rev 56609)
>>> @@ -1,4 +1,4 @@
>>> -// cs1570-10.cs: XML comment on `F:Testing.Test.PrivateField2' has non-well-formed XML (unmatched closing element: expected summary but found incorrect  Line 3, position 12.)
>>> +// cs1570-10.cs: XML comment on `F:Testing.Test.PrivateField2' has non-well-formed XML ('summary' is expected  Line 3, position 4.)
>>>  // Line: 23
>>>  // Compiler options: -doc:dummy.xml -warn:1 -warnaserror
>> I think we can put this issue to rest once and for all by putting the
>> parts of the message not under the control of the compiler on a separate
>> line.
>> I would just have the CS1570 message be
>>   XML comment on `...' has non-well-formed XML
>> The actual error message from XmlTextReader could be passed to
>>   Report.ExtraInformation (loc, "XML error: " + xml_error);
>> on a line _before_ the Report.Error.  This would generate error
>> messages
>> that look like:
>>   foo.cs (23,10): CS1570: XML comment on
>> `F:Testing.Test.PrivateField2' has non-well-formed XML
>>   foo.cs (23,10): XML Error: 'summary' is expected  Line 3, position 4
>> which I think is acceptable.
>
> Hmm, you would have already thought but I don't think it is /doc
> specific matter,

True.

> and handling XML error as some special case does not sound so good (am
> not sure if there are similar cases that depend on external things to
> mcs itself). 

But, I'm talking about the general principle, as applied to your case.

 * The compiler has infrastructure to add extra information about an error

 * It is useful to have a reasonably invariant error message that can be
   quickly checked trivially

Combining both rationales gives us the above style of reporting
messages:

  * put things that the compiler can reasonably control on one line --
    generated with Report.Error or Report.Warning, and checked by the
    regression testsuite

  * anything else goes on subsequent lines with
    Report.ExtraInformation.  The calling convention is slightly wierd
    in that the call to Report.ExtraInformation should precede the
    associated Report.Error

Also, I don't see having extra information in a parenthetical remark
as being much better than having it on a new line.

- Hari