[Mono-devel-list] mcs patch for precise location handling

[Marek Safar]

Hello,

>> I finally see where the point is ;-)
>> Unfortunately I have to say our hierarchy structure seems to be broken.
>>  From my point of view the MemberName should be introduced in 
>> DeclSpace class and not in MemberCore
>> where should be only your LocatedToken. But this change is not 
>> simple. Then we should avoid this for most of cases.
>
>
> Well, is it mandatory fix that should be done before introducing
> line number stuff? I think it's far beyond the scope of location
> stuff and you can make such changes later.

Indeed, this was just comment.

>
> Introducing more than two big changes at a time (and more
> importantly on my side, while I don't fully understand the code
> structures) would be rather harmful. Two weeks ago this patch was
> about 60KB and your first comment was "it's massive" ;-)
>
>> namespace_or_type_name
>>     : member_name
>>     | namespace_or_type_name DOT IDENTIFIER {
>> -        $$ = new MemberName ((MemberName) $1, (string) $3);
>> +        LocatedToken lt = (LocatedToken) $3;
>> +        $$ = new MemberName ((MemberName) $1, lt.Value, lt.Location);
>>       }
>>     ;
>>
>> member_name
>>     : IDENTIFIER {
>> -        $$ = new MemberName ((string) $1);
>> +        LocatedToken lt = (LocatedToken) $1;
>> +        $$ = new MemberName (lt.Value, lt.Location);
>>       }
>>     ;
>>
>>
>> Do you have the figures how many times are MemberName ctor and 
>> LocatedToken ctor called ?
>
>
> I could get them from profiler results. They are in my previous posts.
>
>> I think this is not correct. `lt.Location' is useless we should 
>> always print position where identifier begins.
>>
>> -        $$ = new MemberName ((MemberName) $1, (string) $3);
>> +        LocatedToken lt = (LocatedToken) $3;
>> +        $$ = new MemberName ((MemberName) $1, lt.Value, lt.Location)
>>
>
> Oh, sure. I fixed it in the latest patch here:
> http://monkey.workarea.jp/tmp/20050706/precise-location-20050706.diff
>
> It changes nothing other than this fix, plus merging the latest
> mcs changes.
>
> Martin was wondering when I commit the changes. (I have no idea ;-)

I don't see any other issue in this patch.

Marek