Hello, 2014-05-04 20:12 GMT+08:00 xpp <[email protected]>: > 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 <[email protected]>编写： > >>Hello, >> >>2014-05-04 2:44 GMT+08:00 Michael Matz <[email protected]>: >>> 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 _______________________________________________ Tinycc-devel mailing list [email protected] https://lists.nongnu.org/mailman/listinfo/tinycc-devel
