[Modernization] What do we want to do with all the 1.7- code ?

[srikanth-sankaran]

I started this ticket with the title: Remove CLDC support with the description:

org.eclipse.jdt.internal.compiler.impl.CompilerOptions.UNSUPPORTED_VERSIONS lists VERSION_CLDC1_1 as unsupported.
We still have various vestiges of this in ECJ: for instance STACK_MAP attribute ...

But larger question though is what do we want to do with all the < 1.8 code in the compiler ? I think like we did for $IDENTITY-COMPARISON$ annotation, we should perhaps come up with an instrumented compiler that will flag all unreachable blocks and then use quick fixes to prune them after eyeballing the changes.

I have seen some observations against aggressively purging old code - but would like a discussion that - Done early in a release in the first week of M1, it can be managed well methinks.

I personally would like see all the clutter gone!

[srikanth-sankaran]

@jarthana @mpalat @stephan-herrmann @jukzi @iloveeclipse - Any reason why we MUST not do this - as outlined in a very early stage of M1 of an upcoming release ? Mechanically flagged unreachable blocks, but manually deleted via quick fixes after inspection and reviewed before commit ?

[jukzi]

I second removing dead code.

[srikanth-sankaran]

https://bugs.eclipse.org/bugs/show_bug.cgi?id=417803

I think like we did for the IDENTITY-COMPARISON
annotation, we should perhaps come up with an instrumented compiler that will flag all unreachable blocks and then use quick fixes to prune them after eyeballing the changes.

https://bugs.eclipse.org/bugs/show_bug.cgi?id=417803 is the relevant ticket. There we had to also enhance JDT.ui with quick fix, here the quick fix is free (dead block removal)

[srikanth-sankaran]

(It may be tricky to catch all the dead blocks through this mechanical approach cf. null analysis & correlation related issue)

[stephan-herrmann]

Let me remind you, that we are not yet done with the previous step, i.e., disabling compliance levels below 8. Let's first resolve #2758 and #3297 before we start discussing the next step.

That said, I see no problem with removing a few individual code blocks in areas that we actively work on, preferably with a code review involved.

[iloveeclipse]

Any reason why we MUST not do this - as outlined in a very early stage of M1 of an upcoming release ?

• If something obsoleted is API we can't delete immediately but have to mark "deprecated for deletion" first.
• The non API but public/protected code could be removed before M3 to avoid last minute 3rd party breakages (Xtext uses lot of internal JDT code).
• Everything else that is obsoleted should be removed, sooner is better. JDT is complex enough, if we can reduce complexity by cleaning up unused bits, we shohld do it.
• Investigate tests disabled during transition to minimal 1.8 target #2758 and Some tests fail silently due to now unsupported Java Version <8 #3297 are for sure not nice, but so far we are talking here about ~40 tests out of ~100.000. So whoever has free time and understanding what the tests are testing and why they are failing after transition to 1.8+ JLS is welcome to help. But it is definitely not a prerequisite for the code cleanup proposed by Srikanth.

[stephan-herrmann]

Just so you have my opinion on the table, too:

• I'm much in favor of small step clean-up, well interleaved with feature work, rather than stopping the world until all is 100% neat.
• I am not in favor of calling a clean-up done before it is 100% complete, including 100% of tests
• I'm not in favor of starting another round of clean-up before the previous round has completed.
• I don't plan to spend much time in reviewing big-scale clean-ups.
• The case of $Identity-Comparison$ had high urgency, because we made a fundamental design change, where inappropriate == was a bug, not just an inconvenience.
• (And large changes always drain some of my time away from JDT to keep my fork alive)

This said so you don't forget I'm the old conservative PITA that I am ... but I'm neither the majority nor do I have the money to support my strategy.

Positively speaking, I, too, will be happy to kick out unused stuff, when and where it gets in my way.

[iloveeclipse]

• I am not in favor of calling a clean-up done before it is 100% complete, including 100% of tests
• I'm not in favor of starting another round of clean-up before the previous round has completed.

During transition to 1.8 JLS I've asked for help to fix remaining issues in #2758 but so far no one provided any. So either people aren't interested in this work or have same problem to understand what these tests are about and how 1.8+ transition is related to test failures, or the priorities are on different tasks.

I honestly don't have the time to debug that code especially I'm not familiar with most of that areas. I could do that, but then I wouldn't have time for any other work for a month, which I can't afford.

So if we want thishttps://github.com//issues/2758 to be fixed, please help! But if no one will do that - should we stop now any refactorings? Surely not!

The argumentation that we should only start new cleanup after previous one is 100% done is not correct. Replace "cleanup" with "bug fixing" and we will end in a requirement that we can only start new bug fix after all known bugs are fixed, which is obviously not satisfiable.

[srikanth-sankaran]

Just so you have my opinion on the table, too:

• I'm much in favor of small step clean-up, well interleaved with feature work, rather than stopping the world until all is 100% neat.

This point states a general goodness which is not presently violated. We have significant feature work going in for future features as well as past JEPs implementations being reworked resulting in numerous defects being uncovered and fixed.

• I am not in favor of calling a clean-up done before it is 100% complete, including 100% of tests
• I'm not in favor of starting another round of clean-up before the previous round has completed.

I think we can side step any philosophical questions as to whether this must precede, by stipulating this will precede more modernization work being taken up in the same topic - OK ? :)

• I don't plan to spend much time in reviewing big-scale clean-ups.

This can be organized. Perhaps you can just review the 20-30 or so lines of code that will generate the warnings, rather than the warnings themselves which can be taken up by others

• (And large changes always drain some of my time away from JDT to keep my fork alive)

This is true - but we can discuss how to manage this. Perhaps we can compartmentalize it a bundle at a time or package at at time and do it in multiple steps if deemed helpful.

This said so you don't forget I'm the old conservative PITA that I am ... but I'm neither the majority nor do I have the money to support my strategy.

You don't need either to have your points being taken extremely seriously.

Positively speaking, I, too, will be happy to kick out unused stuff, when and where it gets in my way.

It would not be my preference to combine this cleanup in a commit for feature work or bug fixes and pollute the delta's - not saying you are suggesting that.

[stephan-herrmann]

The argumentation that we should only start new cleanup after previous one is 100% done is not correct.

"correct" is a funny word in this sentence, but let me rephrase my position: a clean-up should leave us in a better state than before, which implies it should not cause any code decay of its own.

[srikanth-sankaran]

So if we want thishttps://github.com//issues/2758 to be fixed, please help! But if no one will do that - should we stop now any refactorings? Surely not!

I'll take over this one, although I will work on this in a very gradual fashion

[stephan-herrmann]

It would not be my preference to combine this cleanup in a commit for feature work or bug fixes and pollute the delta's - not saying you are suggesting that.

If the clean-up part of any given feature change is significant in size, it would surely be a good idea to keep it as a separate commit (and then refrain from squashing on merge).

[srikanth-sankaran]

It would not be my preference to combine this cleanup in a commit for feature work or bug fixes and pollute the delta's - not saying you are suggesting that.

If the clean-up part of any given feature change is significant in size, it would surely be a good idea to keep it as a separate commit (and then refrain from squashing on merge).

That would help but not entirely. Looking at the delta between two versions spaced apart could bring up these "noise" changes that went in their own non-squashed commits. They will anyway but more likely when it is done in installments,

[stephan-herrmann]

So if we want thishttps://github.com//issues/2758 to be fixed, please help!

Did you see that one of those tests was already ticked off? Guess by whom :)
And during the course we even got #2727 resolved, haven't you been waiting for that?