Remove dead code due to 'if 0' #1297
Conversation
|
are you sure we need to be removing all of it? some may be useful... |
|
If there's a specific code you want to keep, let me know. Otherwise most of the code it quite old (5-10 years). Relatively new dead code were kept (e.g ext/gd/libgd/gd_crop.c due to rev a991360), same with dead code who seem useful for future reader (e.g. sapi/phpdbg/phpdbg_parser.c ). |
|
I would suggest to not to touch anything in the bundled libraries like libgd, libmagic, oniguruma, libmbfl and so on. Once they will need to be patched, upgraded, or etc. and that can bring some unnecessary conflicts. Thanks. |
kaplanlior
force-pushed
the
master-dead-code
branch
from
e3ebb47
to
4fca5cf
Compare
|
Thanks @weltling, I removed libpcre & oniguruma changes (both got updated recently). libgd as last updated in 2007 and libmbfl upstream is quite dead (last release 4 years ago). So I felt comfortable to touch that code. |
| @@ -4143,7 +4112,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FE_RESET_RW_SPEC_CONST_HANDLER | |||
|
|
|||
| static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_EXIT_SPEC_CONST_HANDLER(ZEND_OPCODE_HANDLER_ARGS) | |||
| { | |||
| #if 0 || (IS_CONST != IS_UNUSED) | |||
There was a problem hiding this comment.
This is generated code, it should not be changed (or only via zend_vm_def.h). Otherwise these 0 || will just reappear the next time someone regenerates the VM.
|
Some of these look like commented out debugging code - may it be valuable to retain it? Also seen some with a TODO in them. Those probably should stay. |
kaplanlior
force-pushed
the
master-dead-code
branch
from
4fca5cf
to
0683869
Compare
|
Thanks @nikic , I've excluded Zend/zend_vm_execute.h from this PR. Any help with further editing zend_vm_def.h to remove these "if 0 ||" section would be welcomed. Also, brought back one TODO section. Thanks for noticing. |
|
Please keep the Zend/zend_alloc.c, Zend/zend_execute_API.c,Zend/zend_strtod.c, Zend/zend_virtual_cwd.c, Zend/zend_vm_def.h, ext/opcache/Optimizer/* and /fastcgi.c sections. I'm not sure about ext/dom/, ext/bcmath/libbcmath/src/, ext/date/lib/, ext/gd/libgd/, ext/mbstring/libmbfl/, ext/mbstring/ucgendat/, ext/pdo/pdo_dbh.c, ext/zip/lib/, sapi/fpm/ |
kaplanlior
force-pushed
the
master-dead-code
branch
from
0683869
to
3cc2cf3
Compare
|
Thanks @dstogov , files excluded. |
kaplanlior
force-pushed
the
master-dead-code
branch
from
3cc2cf3
to
0524e13
Compare
|
The builds looks OK, although the status is failed. |
| @@ -453,26 +453,3 @@ void timelib_update_ts(timelib_time* time, timelib_tzinfo* tzi) | |||
| time->sse_uptodate = 1; | |||
| time->have_relative = time->relative.have_weekday_relative = time->relative.have_special_relative = 0; | |||
| } | |||
|
|
|||
| #if 0 | |||
| int main(void) | |||
There was a problem hiding this comment.
this looks like test code, may have sense to keep it.
|
Just keep it pls. Many codes are helper or ideas. Yes we could pit them in wiki and such but in lined is better and easier to find for contributors. |
|
In general, I'd avoid messing with any library that is merged into php source from other places. |
| @@ -72,43 +72,6 @@ struct rusage | |||
|
|
|||
| /* Page faults */ | |||
| zend_long ru_majflt; | |||
| #if 0 | |||
There was a problem hiding this comment.
Hm, this one is too much as as well. These are prepared things which are not yet supported on Windows. But please let them stay there.
kaplanlior
force-pushed
the
master-dead-code
branch
from
0524e13
to
88bb686
Compare
|
First, I really appreciate the reviews. I've done quite a few reverts to exclude the parts you've asked. |
|
There seems to be a strong consensus that we should not be doing this, I'm closing the PR. |
|
I think most of the non-library removals have also landed in the meantime (at least I remember doing a bunch of these) |
No description provided.