Community
Participate
Working Groups
Created attachment 276081 [details] Enum on which the problem happens I set the severity to "critical" because this may lead to data loss if the user has not saved before this happens (at it happened to me already >:-|). Create a Java Project and copy 'n' paste the attached Java enum. Go to line 61. The line is: s -> s.equalsIgnoreCase("sì") || s.equalsIgnoreCase("si") Then, go to column 18, insert a space before "s.equals", then move the cursor back so that the situation is this: s -> | s.equalsIgnoreCase("sì") || s.equalsIgnoreCase("si") (where "|" represents the cursor). Then, invoke code assist with Ctrl+Space at that position: Eclipse freezes completely and the CPU goes crazy. I took some thread dumps I'm going to attach. I don't see any deadlock, so I suspect it might be some sort of infinite loop. Maybe the problem is the presence of the Formats static nested class? The above steps reproduce the problem 100% of the times for me. But there are other paths that lead to the same result. At some other location, though, code assist works with no freeze.
Created attachment 276082 [details] Thread dump 1
Created attachment 276083 [details] Thread dump 2
Created attachment 276084 [details] Thread dump 3
Created attachment 276085 [details] Thread dump 4
Created attachment 276086 [details] Thread dump 5
Tried to reproduce with a plain "Eclipse IDE for Java Developers" 2018-09 (no Groovy Eclipse Plugin installed), just to check it's not a problem with Groovy: the first Code Assist invocation take a while, but completed. The second invocation (at the same point) shown the exact same behaviour. I'm going to attach some thread dumps taken in this scenario.
Created attachment 276089 [details] Thread dump 1 (no Groovy Eclipse Plugin)
Created attachment 276090 [details] Thread dump 2 (no Groovy Eclipse Plugin)
Created attachment 276092 [details] Thread dump 3 (no Groovy Eclipse Plugin)
Created attachment 276093 [details] Thread dump 4 (no Groovy Eclipse Plugin)
*** Bug 541297 has been marked as a duplicate of this bug. ***
Bulk move out of 4.13
*** Bug 546097 has been marked as a duplicate of this bug. ***
*** Bug 559197 has been marked as a duplicate of this bug. ***
(In reply to Stephan Herrmann from comment #14) > *** Bug 559197 has been marked as a duplicate of this bug. *** That bug has another convenient repro.
Bug 559197 brought back to the table the question, whether or not completion is stoppable by the timeout monitor. While Sam reported infinite freeze, both Andrey and myself observed the timeout dialog. The intended rescue should happen inside CompletionParser.resumeOnSyntaxError(), which after 100 retries should check the timeout monitor. See also bug 482663 comment 3 and similar. This should work since Eclipse 4.10. Sam, are you sure you are running a "pure JDT"? Or is anything like groovy, aspectj, object teams, scala, lombok involved?
(In reply to Stephan Herrmann from comment #16) > Sam, are you sure you are running a "pure JDT"? Or is anything like groovy, > aspectj, object teams, scala, lombok involved? None of them. I downloaded eclipse-java-2019-12-R-win32-x86_64.zip from https://www.eclipse.org/downloads/packages/, set the -vm parameter in eclipse.ini to the jdk-11.0.2/bin directory and created the project (see attachment in bug 559197). That's all...
One more thing: the fix for bug 535743 added a few asserts in the code, so it is interesting to run Eclipse with -ea during analysis of this issue (below -vmargs in eclipse.ini or such).
(In reply to Sam Peters from comment #17) > (In reply to Stephan Herrmann from comment #16) > > Sam, are you sure you are running a "pure JDT"? Or is anything like groovy, > > aspectj, object teams, scala, lombok involved? > > None of them. I downloaded eclipse-java-2019-12-R-win32-x86_64.zip from > https://www.eclipse.org/downloads/packages/, set the -vm parameter in > eclipse.ini to the jdk-11.0.2/bin directory and created the project (see > attachment in bug 559197). That's all... Thanks for confirming. Indeed I can reproduce in that configuration. I'll give it a quick shot at debugging that installation.
(In reply to Stephan Herrmann from comment #19) > Thanks for confirming. Indeed I can reproduce in that configuration. I'll > give it a quick shot at debugging that installation. Oh, that's nice to hear. Good luck :)
I found something, see bug 559259. Reducing the severity of this bug, assuming after bug 559259 we will no longer freeze.
Users affected by the freeze (without timeout dialog) may try to disable "Java Proposals (Task-Focused)" in Preferences > Java > Editor > Content Assist > Advanced. I'm not saying that Mylyn is to blame here, but I have a theory that it contributes to the call chain where we fail to time out.
I was experiencing loss of work due to a content assist freeze with an anonymous class inside a lambda. This bug seems to be the main one for the root cause of the freeze, which is an infinite loop. I also got the freeze due to bug 563158. I won't pretend to understand much at all of the content assist code, but I did some debugging and possibly found a fix. I confirmed the infinite loop occurs with the latest version of eclipse that I could easily install: Eclipse IDE for Java Developers (includes Incubating components) Version: 2021-03 M3 (4.19.0 M3) Build id: 20210225-1153 Here's the code I'm using to reproduce the infinite loop: import java.util.Objects; import java.util.function.Function; public class ReproduceHang { Function<String, Object> field = (value -> new Object() { private final int i = Objects.requireNonNull(1); // line 6 }); public static void main(String[] args) { Function<String, Object> localVar = (value -> new Object() { private final int i = Objects.requireNonNull(1); // line 11 }); } } Note that ReproduceHang.field and localVar in main() are the exact same code. I observed the following behaviors before making any changes: 1. Hitting Ctrl-Space immediately after "Objects." on **line 6** results in: - No completion proposals. - An error sound. 2. Hitting Ctrl-Space immediately after "Objects." on **line 11** results in: - "Computing proposals (0%) ..." - Ongoing 100% utilization on one CPU core on first attempt. - 50-100% additional ongoing CPU utilization on every subsequent attempt. The line 6 behavior looks like a separate bug. I didn't look to see if it's been reported. The line 11 behavior is the infinite loop. In debugging, I found that the switch (resumeOnSyntaxError()) statement on line 12758 of o.e.jdt.internal.compiler.parser.Parser.parse() always goes to case RESUME on line 12768. The root source of this RESUME value is line 2326 of o.e.jdt.internal.codeassist.impl.AssistParser.fallBackToSpringForward(). Let me repeat, I don't understand this code. Nevertheless, I simply tried returning HALT from AssistParser line 2326 while debugging eclipse. That immediately ended the high CPU utilization and allowed content assist to return completion proposals for line 11. The returned proposals looked correct. I later found that returning RESTART also yields expected behavior. I also confirmed that changing the return value back to RESUME immediately brought back the infinite loop. Because I don't understand the content assist code, I looked at the git history of that section of code to see what would be affected by changing this value. Looks like not much would be broken by this. The code in question was added in jdt.core commit cccafe00dbf29f156949d8f0d0aec370b02aa048 which mentions bug 423987, which is followup for bug 422468. Bug 423987, comment 4 also mentions bug 473654. The code is also impacted by commit 72cd044d46c3586e1d4fcaba5b2c3d87eb83b467 from bug 535743. I looked through those four bugs for code to reproduce them and copy/pasted the code I found into my "fixed" workspace. In all those cases, I got the expected behavior and not the reported errant behavior. Hopefully there are tests that are better than my manual testing. I don't know if returning HALT or RESTART from line 2326 is correct, but I hope this information will narrow the focus on where to look for fixing this completion proposal infinite loop.
(In reply to William Kilian from comment #23) Thanks for digging in so deep! > In debugging, I found that the switch (resumeOnSyntaxError()) statement on > line 12758 of o.e.jdt.internal.compiler.parser.Parser.parse() always goes to > case RESUME on line 12768. The root source of this RESUME value is line 2326 > of o.e.jdt.internal.codeassist.impl.AssistParser.fallBackToSpringForward(). Yes, that's one of the monsters in our implementation. See that I even tried to document some of these things in https://wiki.eclipse.org/JDT_Core_Programmer_Guide/Completion Several things are unfortunate: * The engineer who implemented the initial version of completion with lambdas is no longer in the team. * The parser (an LALR(1) parser) is way outside it's contract, since the grammar of today's Java has several ambiguities, so even during normal parsing, the generated parser has to be coerced and tweaked to understand this stuff. * During completion we by definition have to make sense of incomplete code which breaks several of the kludges/heuristics from the previous bullet. Anyway, your comment gives sufficient food for a fresh look. Thanks.
I tried a number of things but found no good fix. I.e., whatever I tried it created regression elsewhere in our test suite. (In reply to William Kilian from comment #23) > Let me repeat, I don't understand this code. Nevertheless, I simply tried > returning HALT from AssistParser line 2326 while debugging eclipse. To make sure we are looking at the same line, is it the return in this block, right before the bottom of the method? if (assistNodeNeedsStacking()) { this.currentToken = TokenNameSEMICOLON; return RESUME; } We do have a number of test cases that depend on RESUME right here - notably in https://git.eclipse.org/c/jdt/eclipse.jdt.core.git/tree/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/CompletionTests18.java I begin to understand that in some cases resuming from here does not work, because before we get there, too much state was already manipulated. Apparently we don't succeed to restore all that for a clean resume. In cases like comment 23 we are trying to compensate for a syntax error that isn't even there. It's just an artifact of asking the parser to stop at the cursor position. If a lambda is involved we need to peek a little further, to actually recognize the lambda. For this purpose we extend the range, but when we RESUME some data structures/pointers are already broken :( OTOH a premature HALT may cause that the assistNode (the AST node where completion happened) has not been integrated into resulting AST.
(In reply to Stephan Herrmann from comment #25) > I tried a number of things but found no good fix. I.e., whatever I tried it > created regression elsewhere in our test suite. That's unfortunate. Whack-a-mole is fun at the arcade; not so much with programming. > To make sure we are looking at the same line, is it the return in this > block, right before the bottom of the method? > > if (assistNodeNeedsStacking()) { > this.currentToken = TokenNameSEMICOLON; > return RESUME; > } Yep, that's the line. I read your updates at https://wiki.eclipse.org/JDT_Core_Programmer_Guide/Completion Thanks for looking more deeply into this.
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/178809
(In reply to Eclipse Genie from comment #27) > New Gerrit change created: > https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/178809 Pushing a proposed change to gerrit was a bold step: as can be seen on jenkins, MANY completion parser tests are unhappy (while the actual completion tests are green). From an initial scan of the failures, however, I think that in most cases the actual result is actually *better* then what the tests expect: the recovered AST has more context which is good for computing expected types. So here's a call for participation: since this is a substantial change of a complex machine, it would be great if people would join in for a thorough review. @Vikas, will you be able to spare some time? I will continue to put stuff I learned during the course into https://wiki.eclipse.org/JDT_Core_Programmer_Guide/Completion and also explain the key changes here. Ideally, we'd come to an agreement about the direction before potentially editing a hundred of test expectations. You might also ask if such a demanding change is worth it, but (a) eclipse freezing due to completion is very bad and (b) I do hope that the change will also cover a few other pending bugs, perhaps even make further bug fixes easier.
==== Solution Strategy ==== The key change is removing the override CompletionParser.bodyEnd() - D'uh! Previously, this method would define that completion parsing should see EOF immediately once it hits the cursorLocation. Prior to lambdas this was OK, but since 1.8, EOF in the middle of a lambda will leave the parser in an utterly unhelpful state, so Srikanth added logic to AssitParser.fallBackToSpringForward() that would opportunistically move EOF (Scanner.eofPosition) when we need more context (essentially controlled by requireExtendedRecovery()). My understanding of the root cause of trouble is: when fallBackToSpringForward moves the goal post, we have already reacted on the premature EOF - we have processed tokens at least to the point of encountering a syntax error. We don't have efficient means to return to a sane state. -------------------------------------------------------------------------------------- Hence the key idea: don't increase the source range being parsed in the middle of the game, but start with the maximal relevant source range, i.e., the entire body of the current method (if any). -------------------------------------------------------------------------------------- Next I learned, that EOF at cursorLocation is crucial for triggering some completion-specific tasks in methods like updateRecoveryState(), attachOrphanCompletionNode() and the buildMore*Completion*Context() family of methods. To limit the damage of the change of strategy, I invented a new hook fetchNextToken() where the completion parser can decide to move the goal post in the opposite direction: if when reaching the cursorLocation we are clear of any incomplete lambda scenarios, let's stop parsing just then to approximate the previous strategy of eager EOF. NB: Long term we could try to completely abandon the previous strategy (which would rid us of quite some code), but currently, the new strategy still misses lots of cases handled by existing code. All other code changes basically compensate for not triggering updateRecoveryState() and friends right when we hit the cursor location. I'll describe those changes in subsequent comments.
== Compensating for no EOF at cursorLocation == The most obvious change is looking at currentPosition vs. cursorLocation instead of testing for EOF. = background = For discussing other changes it is important to understand the purposes of updateRecoveryState() and friends: We may get here in a state where the completion node is an orphan, i.e., we have created a CompletionOn* ASTNode, assigned it to AssistParser#assistNode, but have not yet integrated this node into the actual AST (so when CompletionEngine invokes parsedUnit.resolve() we would never find this node). This is were attachOrphanCompletionNode() comes in: this methods looks left and right and performs one or more of these actions: * add the completion node to the currentElement (RecoveredElement), so when the final AST will be retrieved from the RecoveredElement tree, the completion node will be part of it. * wrap the completion node into a new complex ASTNode based on ingredients from parser stacks. * build more completion context, which essentially does more of the above * assign nodes to #assistNodeParent and #enclosingNode (which are later directly used by CompletionEngine) After staring at this code for some time, I seem to understand that this code area is not solid based on principles of the Java grammer, but essentially this is a big collection of heuristics, which information in which situation might allow the completion parser to provide useful information. The resulting AST may deliberately deviate from the actual source, if only the CompletionEngine finds it useful we're good. = changes = Without EOF, in some cases we stopped creating any CompletionOn* ASTNode in the first place. Here are some new overrides that will create such nodes (if context demands so): * typeElidedArgument * newMessageSend At some points I saw the completion node to be attached in several places of the AST (which then no longer was a tree!). One affected node was the EmptyStatement synthesized from a synthesized ';' token. consumeEmptyStatement() now detects if the node about to be create already exists. In one case (consumeBlockStatement()) I could even directly leverage the existing buildMoreCompletionEnclosingContext(). Much of the handling of #astNodeParent and #enclosingNode has been pushed to CompletionNodeDetector. This is triggered now from endParse(). More on this later. A special case of orphans is handled directly during endParse: a top-level statement not yet attached to the body of its method. Interestingly, some manual creation of ASTNodes, previously done in attachOrphanCompletionNode() and friends, is no longer necessary, because we end up with a more complete AST anyway. Only detecting which of these should be seen as assistNodeParent and enclosingNode requires a bit more work now, new purposes of CompletionNodeDetector.
For a very specific story see https://wiki.eclipse.org/JDT_Core_Programmer_Guide/Completion#Supporting_casted-receiver_completions
Completing nested diamonds? Some tests introduced via bug 346454 challenge situations like Test<String>.T2<String> t = new Test<>().new T2<>(|) Tests like this expect the constructor of T2 to be proposed. The problem here: the inner diamond can impossibly be inferred, because the outer diamond ("new Test<>()") is already illegal as it lacks a target type and has nothing else to help inference. At this point, what's the use of proposing constructors of an ill-defined type, in particular since we are already inside the argument list? My change currently detects the illegal / uninferable outer and forgoes any proposals in that case. Is that too drastic? The original bug was about preventing a runtime exception :) I added two tests with fixed outer allocation, to demonstrate that then we are able to perform normal completion for the empty argument. (Unfortunately, the compiler doesn't provide the parameterized outer type binding when resolving the inner type, so expected types computation cannot leverage the outer type argument).
Hint: several smaller changes may look unrelated at first. To see their relevance, simply disable one of these changes and run completion tests. The resulting failure should make inspecting that change fairly easy. Frankly, several of this look like I'm fixing dormant bugs.
*** Bug 558530 has been marked as a duplicate of this bug. ***
(In reply to Stephan Herrmann from comment #34) > *** Bug 558530 has been marked as a duplicate of this bug. *** Unmarked as duplicate again as it required a couple of small fixes on top of this bug. Please see that bug 558530 finally puts us in the position where completion inside an expression lambdas knows the expected type.
Forgot to mention one relevant detail: (In reply to Stephan Herrmann from comment #29) > To limit the damage of the change of strategy, I invented a new hook > fetchNextToken() where the completion parser can decide to move the goal > post in the opposite direction: if when reaching the cursorLocation we are > clear of any incomplete lambda scenarios, let's stop parsing just then to > approximate the previous strategy of eager EOF. fetchNextToken() only triggers EOF when * where not currently parsing a lambda expression, and * the expression stack is empty The second bullet is immensely useful in order to build sufficient context for type inference. Only so we are able to compute expected types in lots of use cases.
Test classes with failures as of now: org.eclipse.jdt.core.tests.compiler.parser.AnnotationCompletionParserTest org.eclipse.jdt.core.tests.compiler.parser.CompletionParserTest org.eclipse.jdt.core.tests.compiler.parser.CompletionParserTest2 org.eclipse.jdt.core.tests.compiler.parser.CompletionRecoveryTest org.eclipse.jdt.core.tests.compiler.parser.EnumCompletionParserTest org.eclipse.jdt.core.tests.compiler.parser.ExplicitConstructorInvocationCompletionTest org.eclipse.jdt.core.tests.compiler.parser.FieldAccessCompletionTest org.eclipse.jdt.core.tests.compiler.parser.GenericsCompletionParserTest org.eclipse.jdt.core.tests.compiler.parser.InnerTypeCompletionTest org.eclipse.jdt.core.tests.compiler.parser.LabelStatementCompletionTest org.eclipse.jdt.core.tests.compiler.parser.MethodInvocationCompletionTest org.eclipse.jdt.core.tests.compiler.parser.NameReferenceCompletionTest org.eclipse.jdt.core.tests.compiler.parser.ReferenceTypeCompletionTest (plus two intermittent, independent failures)
(In reply to Stephan Herrmann from comment #37) > Test classes with failures as of now: > > org.eclipse.jdt.core.tests.compiler.parser.AnnotationCompletionParserTest > org.eclipse.jdt.core.tests.compiler.parser.CompletionParserTest As a first batch, these two classes have been adjusted in patch set #4. In most cases I'm fairly certain that results are now equally suitable or better than before. Just in the case of CompletionParserTest.testBug346454b I see room for improvement. It's basically the same issue as mentioned in comment 32 (test case is from the same bug), viz. what completions do we expect in situations like Test<String>.T2<String> t = new Test<>().new T2<>() Two issues: * outer allocation diamond is already illegal, thus we have no target type for the inner * if the outer is fixed, where exactly should completion produce which results? new T2|<>() new T2<>|() new T2<>(|) I believe the first form is be far the most common form, and this form is not influenced by my changes. The second form currently has no proposals, because we don't create AST for the allocation (but still it doesn't crash like before bug 346454 :) Completing in the third form is useless in this case because there's no constructor in T2 that takes arguments. I dropped a TODO into a new testBug346454b2 with a hint where in CompletionParser we currently drop the ball. I don't think this is a new problem.
While focus currently is on adjusting test expectations (while validating that the new outcome is "equal" or "better"), I made a few more code changes after Vikas' +1 in gerrit: * special handling for ArrayInitializer in CompletionNodeDetector, to the effect that in specific situations we prefer a larger AST node as assistNodeParent. This allowed me to keep some existing test expectations. My rule here is: more context is generally preferable, at least this change should not reduce context vs previous behavior. * adjustment of source positions for CompletionOnMessageSendName. Without this change, in some cases the replacement range was too large, leading to removal of the message send receiver, when the proposal was applied. This adjustment must be made after super.consumeMethodInvocationName(), because that's where positions are initially assigned. For some issues regarding the application of completion proposals I will add a few tests in JDT/UI, after this change has been merged.
In addition to a flood of test adjustments, I made a few more functional changes. In patch set #7 some handling of MessageSend is applied to more variants (create CompletionOn.., and adjust source positions). Also endParse() now tries harder to attach the assist node (with optimal context) into the current method. Changed in patch set #8: I observed that dietParse & block-statements parse would create two different assist nodes: diet: CompletionOnArgumentName, block-statements: CompletionOnSingleNameReference. The former is fully sufficient, whereas the latter could only confuse the completion engine. For that reason I detect this situation before starting to parse block statements, possibly short circuiting the same. While adjusting tests I privately noted some tests where the expectedReplacedSource changes from "bar(" to "bar()". I'll check with JDT/UI tests if this has any impact on the actual behavior.
Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/178809 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=e3517ddc41f3c9536a29ef9be4e7dd3104993ab2
(In reply to Eclipse Genie from comment #41) > Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/178809 was > merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/ > ?id=e3517ddc41f3c9536a29ef9be4e7dd3104993ab2 I'll keep the bug open just until I added a few more tests in JDT/UI to ensure the observable behavior has not changed in some cases where test expectations in JDT/Core have changed (in particular completion parser tests with detailed expectations).
I would like just to say a big thank you to Stephan for the insight, the commitment and the effort spent on this issue. I think this is one of the areas where Eclipse needs some love lately, although being rather "under the cover". Really appreciated!
(In reply to Stephan Herrmann from comment #42) > (In reply to Eclipse Genie from comment #41) > > Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/178809 was > > merged to [master]. > > Commit: > > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/ > > ?id=e3517ddc41f3c9536a29ef9be4e7dd3104993ab2 > > I'll keep the bug open just until I added a few more tests in JDT/UI to > ensure the observable behavior has not changed in some cases where test > expectations in JDT/Core have changed (in particular completion parser tests > with detailed expectations). Unfortunately it *is* changed. org.eclipse.jdt.ui.tests.quickfix.AdvancedQuickAssistTest.testSurroundWithTemplate01 fails now, see https://download.eclipse.org/eclipse/downloads/drops4/I20210421-1800/testresults/html/org.eclipse.jdt.ui.tests_ep420I-unit-cen64-gtk3-java11_linux.gtk.x86_64_11.html
(In reply to Andrey Loskutov from comment #44) > (In reply to Stephan Herrmann from comment #42) > > (In reply to Eclipse Genie from comment #41) > > > Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/178809 was > > > merged to [master]. > > > Commit: > > > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/ > > > ?id=e3517ddc41f3c9536a29ef9be4e7dd3104993ab2 > > > > I'll keep the bug open just until I added a few more tests in JDT/UI to > > ensure the observable behavior has not changed in some cases where test > > expectations in JDT/Core have changed (in particular completion parser tests > > with detailed expectations). > > Unfortunately it *is* changed. > org.eclipse.jdt.ui.tests.quickfix.AdvancedQuickAssistTest. > testSurroundWithTemplate01 fails now, see > https://download.eclipse.org/eclipse/downloads/drops4/I20210421-1800/ > testresults/html/org.eclipse.jdt.ui.tests_ep420I-unit-cen64-gtk3- > java11_linux.gtk.x86_64_11.html Thanks for the hint. My bad, I had only run o.e.jdt.text.tests.ContentAssistTestSuite. I'm surprised to learn that even template proposals use codeComplete() - I'm on it now.
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/179673
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/179674
(In reply to Eclipse Genie from comment #46) > New Gerrit change created: > https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/179673 Fix in jdt.core (In reply to Eclipse Genie from comment #47) > New Gerrit change created: > https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/179674 Temporarily ignore the offending test (to unlock contributions) & add more integration tests.
(In reply to Mauro Molinari from comment #43) > I would like just to say a big thank you to Stephan for the insight, the > commitment and the effort spent on this issue. I think this is one of the > areas where Eclipse needs some love lately, although being rather "under the > cover". > > Really appreciated! Thanks for the kind words. They do make a difference for me :) Looking back, minimal support for completion in the context of lambdas was Srikanth's final nightmare before Java 8 GA. I'm afraid he may still have bad dreams because of it. We can't blame anybody for not continuing the necessary work here, because - as mentioned before - the whole completion implementation has become quite some beast. Working on it is not suitable for squeezing into a tight release schedule. Because of the complexity I'd like to extend a request to extensively test the latest changes in the field. It would be great if people pick up a recent I-build and closely observe code completion. If new problems are detected, please file new bugs and link them to this issue. I do expect new bugs to pop up, but I have reason to hope, that they can now be fixed with tolerable effort.
Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/179673 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=ab2d90d4e73153f0d027bfb01126547b18df36d8
Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/179674 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=55736e27df2636919fa4d09c8cc9ebf3d0ec3643
Last remaining TODO: revert the @Ignore in http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=55736e27df2636919fa4d09c8cc9ebf3d0ec3643
(In reply to Stephan Herrmann from comment #49) > Because of the complexity I'd like to extend a request to extensively test > the latest changes in the field. It would be great if people pick up a > recent I-build and closely observe code completion. If new problems are > detected, please file new bugs and link them to this issue. I do expect new > bugs to pop up, but I have reason to hope, that they can now be fixed with > tolerable effort. With the original patch version / build I20210421-1800 I'm unable to see completion on Thread.| <- here I've expected to see sleep() but got only "Thread.State" and Thread.UncaughtExceptionHandler Try to change this line (remove sleep() from Thread.sleep() and try to code completion after dot): https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/179683/2/org.eclipse.jdt.core.tests.builder/src/org/eclipse/jdt/core/tests/builder/AbstractLeakTest.java#175
(In reply to Andrey Loskutov from comment #53) > With the original patch version / build I20210421-1800 I'm unable to see > completion on Thread.| <- here I've expected to see sleep() but got only > "Thread.State" and Thread.UncaughtExceptionHandler > > Try to change this line (remove sleep() from Thread.sleep() and try to code > completion after dot): > https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/179683/2/org.eclipse.jdt. > core.tests.builder/src/org/eclipse/jdt/core/tests/builder/AbstractLeakTest. > java#175 I tried with both I20210421-1800 and current HEAD, but could not reproduce. In all cases I saw lots of methods proposed. I tried removing just "sleep" as well as the entire invocation "sleep(5000)", both worked. <shrug>
*** Bug 548779 has been marked as a duplicate of this bug. ***
The issue of allocation expressions discussed in comment #32 and comment 38 is being improved via bug 539617 comment 3. A questionable part of test adjustments made here, will be reverted via that bug.
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/179802
Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/179802 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=04fe4f6c20a414339454bf5a48bf894f0acfc39d
(In reply to Stephan Herrmann from comment #52) > Last remaining TODO: revert the @Ignore in > http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/ > ?id=55736e27df2636919fa4d09c8cc9ebf3d0ec3643 Done via: (In reply to Eclipse Genie from comment #58) > Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/179802 was > merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/ > ?id=04fe4f6c20a414339454bf5a48bf894f0acfc39d While still a few related fixes / tests may trickle in this main bug is resolved now.
(In reply to Stephan Herrmann from comment #28) > So here's a call for participation: since this is a substantial change of a > complex machine, it would be great if people would join in for a thorough > review. @Stephan: Since you asked for it, I put some extra testing during the verification time :) - reported a follow up bug 573621. Marking this as verified Verified for 4.20 M3 Version: 2021-06 (4.20)Build id: I20210518-0850 Thanks Stephan for a sustained herculean effort - And from the developer point of view, thanks for the detailed description which would help future engineers. - marking this as greatfix
Stephan, out of curiosity: does this also fixes bug #482663? It was closed by the stale bot, but your last comment in 2018 says that, even if the freeze didn't occur any more, code assist still failed to propose anything.
Remarks for any passers by: Yes, this fix caused some regressions (bug 573279, bug 573632, bug 573702, did I forget any?). No, I'm not surprised by that fact, considering that a fundamental design decision from JDT's day #1 had to be (partially) overthrown. Could we have fixed this bug with less disruption? Given that for many years nobody really found an idea how, I strongly doubt there is a free lunch hidden somewhere. I'm glad about any bug report that came in before 4.20 R, that's exactly what I asked for in comment 49. Thanks! I *am* slightly surprised that some of the basic regressions were not caught by our big test suite. Perhaps that test suite was biased towards challenging the old strategy, and had blind spots in other areas. We now have a few more tests, but sure a green test run will never guarantee absence of regressions. In my recollection this was the largest remaining technical debt from the introduction of Java 8 (anybody still remember? :) ). Let's hope that new versions of Java don't impose similar disruptions on JDT again.
*** Bug 482663 has been marked as a duplicate of this bug. ***
*** Bug 527263 has been marked as a duplicate of this bug. ***
*** Bug 504051 has been marked as a duplicate of this bug. ***
*** Bug 542574 has been marked as a duplicate of this bug. ***
*** Bug 540763 has been marked as a duplicate of this bug. ***
