tinycc-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Tinycc-devel] mob broken; how to develop with mob and community


From: Roy Tam
Subject: Re: [Tinycc-devel] mob broken; how to develop with mob and community
Date: Sun, 4 May 2014 20:33:56 +0800

Hello,

2014-05-04 20:12 GMT+08:00 xpp <address@hidden>:
> Thank you for your criticism, I am no longer afraid of the future scrawl, but 
> I have benefited from your criticism, there are many issues not considered. I 
> have to commit to withdraw, libcrt rename ago. I'm a clown.

I appreciate your work on fixing things. But if your commits cannot
meet the coding format standard(for example, use 4 spaces instead of
tab char), IMO you should work in your branch/fork and send a pull
request to maillist so users can revivew and give you advise before
touching the mob branch (mob branch is a wiki-alike ecology, people
can just simply revert changes without notifying anyone. But we're now
doing that in a polite way, giving chances to original committer to
have time fixing the code without reverting changes totally)

Hope my 2 cents help. :)

>
> Roy Tam <address@hidden>编写:
>
>>Hello,
>>
>>2014-05-04 2:44 GMT+08:00 Michael Matz <address@hidden>:
>>> Hello,
>>>
>>> okay, are the last commits to mob from jiang meant as joke or vandalism?
>>>
>>> * 32bit code generation is hosed already in the testsuite, * gawk doesn't
>>> work anymore even for x86_64,
>>> * arm codegen is broken already in the testsuite (adding an internal
>>>   compiler error)
>>> * they contain ugly white-space changes making review exceedingly hard
>>> * despite the unnecessarily hard review I think there are numerous
>>>   problems in the actual implementation:
>>>   + the new parse_number uses inexact floating point directly (e.g. 1.0L/b
>>>     when b==10 isn't exactly representable, cumulating errors while
>>>     parsing)
>>>   + There's a new subtype VT_VLS meaning VLA plus STRUCT, which makes no
>>>     sense at all (VLA is VL _array_)
>>>   + TREG_MEM (also new) doesn't follow convention for type flags, and
>>>     seems like a layering violation
>>>   + It reverts a cleanup by Thomas (eda2c756edc4dca004ba217) without
>>>     discussion
>>>   + It renames libtcc1.a to libcrt.a, thereby trading a sensibly
>>>     tcc-specific name for something tcc-specific with something generic
>>>     (what if gcc had libcrt as well?)
>>>   + It increases VT_STRUCT_SHIFT to 20, breaking bitfields larger than 31
>>>     bits (we needs 12 bits to encode bitfield position and size, so the
>>>     maximum bit shift can be 19
>>>   + It changes gv2() so that VT_CMP/VT_JMP results aren't special-cased
>>>     anymore, without obvious compensation in all its users to avoid the
>>>     errors that the comment specifically mentioned
>>>   + It implements some strange non-standard preprocessor extension
>>>     push_macro/pop_macro (as pragmas) without discussion; it enlargens
>>>     some heavily used internal data structures for this.
>>>   + It adds some "fix x86-64 vla" commit, without testcase showing what's
>>>     actually broken, and for that shuffles the internal code generations
>>>     in large and unobvious ways (and removes the correct calls to alloca()
>>>     on x86-64 PE)
>>>
>>> And that's just what I saw on a cursory read of the commits.  Due to the
>>> white-space changes the more intricate parts are terrible to review and I've
>>> skipped them.
>>>
>>> When I wrote above "without discussion", then this was just for the most
>>> controversial parts.  It's true for all the patches.  I've seen no messages
>>> at all from jiang to this mailing list.  No discussion about implementation
>>> approaches, no discussions about bugs, no nothing.  The commit messages are
>>> mostly non-informative as well.
>>>
>>> All in all I think this approach is pretty unacceptable, but others here
>>> might differ.  If the patch series were a smaller then the problems in it
>>> could reasonably be fixed after the fact by others.  But as it stands we now
>>> have something in which every single one of the 22 topmost patches (ignoring
>>> the white-space fixup patch from grischka) has issues.
>>>
>>> If it were just my project I'd be tempted to revert the whole mob state to
>>> be before your (jiangs) patches, and expect you to work with the community
>>> to fix what you actually wanted to fix or improve.  (From the patch series I
>>> gather that one thing you wanted to fix was parameter passing on stack when
>>> memcpy is needed).  It the very minimum you have to subscribe to this
>>> mailing list (that's even listed in the mob rules), and of course also take
>>> part in discussions.  You also have to _review_ your patches before
>>> commiting (you would have seen the useless white-space changes) and write
>>> meaningful commit messages.
>>>
>>> Any opinions from others?
>>>
>>
>>IMO I'd urge jiang to create fork in github instead.
>>
>>>
>>> Ciao,
>>> Michael.
>>>


Regards,
Roy



reply via email to

[Prev in Thread] Current Thread [Next in Thread]