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

[Michael Matz]

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?


Ciao,
Michael.