Re: [Tinycc-devel] mob broken; how to develop with mob and community
On 03/05/14 19:44, Michael Matz wrote: Hello, okay, are the last commits to mob from jiang meant as joke or vandalism? I would like to think it wasn't meant as vandalism, but it certainly looks like vandalism to me (at least to the i386 build, which currently doesn't even compile!). :( * 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? If it were my project, I would have reverted all of those commits some time ago! (NOTE: I haven't made significant contributions to this project, so I don't get a vote on this). ATB, Ramsay Jones ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] mob broken; how to develop with mob and community
On 04/05/14 10:30, Ramsay Jones wrote: On 03/05/14 19:44, Michael Matz wrote: Hello, okay, are the last commits to mob from jiang meant as joke or vandalism? I would like to think it wasn't meant as vandalism, but it certainly looks like vandalism to me (at least to the i386 build, which currently doesn't even compile!). :( Of course, I wrote that just before fetching from git ... :-P The i386 build compiles again. Thanks! ATB, Ramsay Jones ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] mob broken; how to develop with mob and community
Hello, 2014-05-04 20:12 GMT+08:00 xpp 30155...@qq.com: 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 roy...@gmail.com编写: Hello, 2014-05-04 2:44 GMT+08:00 Michael Matz matz@frakked.de: 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 Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel
[Tinycc-devel] mob broken; how to develop with mob and community
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. ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] mob broken; how to develop with mob and community
Hello, 2014-05-04 2:44 GMT+08:00 Michael Matz matz@frakked.de: 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. ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel