Community
Participate
Working Groups
Make the "effectively final" state of local variables available. Should be easy to do in IVariableBinding. Would also be very handy in ILocalVariable if possible. We could sneak this into getModifiers()/getFlags() with an internal flag, but I guess it's better to add a boolean isEffectivelyFinal() API instead.
Jay, this is blocking 2 UI work items. Thanks for taking a look. I wonder if we should change this snippet in Assignment.resolveType: if (localVariableBinding != null && localVariableBinding.isCatchParameter()) { to also include localVariableBinding.isParameter() Not strictly necessary for this ER, but ...
Created attachment 237384 [details] Patch with test Patch includes new API IVariableBinding with its implementation and new test. I haven't introduced the API in ILocalVariable since it's not straight forward in some of the cases, esp. when we recreate the element from the memento or binaries. Markus, let me know if you would like changes in ILocalVariable as well. Perhaps we can take that up via a new bug.
You probably got tricked by the abbreviated definition of "effectively final" in the non-normative "Summary" section of the jsr335 document, which is not complete enough for an API (where incomplete == wrong). Since the full definition is too long, the API should just refer to JLS8, e.g.: * Returns whether this binding corresponds to an effectively final local * variable (JLS8 4.12.4). Informally, an effectively final local variable is * not final and it is never assigned to after its initialization. The implementation needs to be adapted to the spec definition. Please check if it would make sense to fix the internal usage of TagBits.IsEffectivelyFinal as well. Example to test: void m() { final int finalVar = 1; // not effectively final! int effectivelyFinalVar = 2; int nonFinalVar = 3; nonFinalVar = 4; try { } catch (java.io.IOError | IllegalStateException implicitlyFinalExc) { // implicitlyFinalExc is not effectively final! } catch (Exception effectivelyFinalExc) { } } > Markus, let me know if you would like changes in ILocalVariable as well. > Perhaps we can take that up via a new bug. Yes. A separate bug is also fine.
(In reply to Markus Keller from comment #3) > The implementation needs to be adapted to the spec definition. Please check > if it would make sense to fix the internal usage of > TagBits.IsEffectivelyFinal as well. I think the compiler implementation sets this bits eagerly on any local variable under the presumed innocent until proven guilty model and the meaning is also a bit loose but "effectively correct" for our purposes :) It simply encompasses final, effectively final and implicitly final. This is perfect enough for the compiler and I would not change it. For the API, we could make other checks to discriminate. > Example to test: Jay, could you also add a resource variable to the test below ?
(In reply to Srikanth Sankaran from comment #4) > meaning is also a bit loose but "effectively correct" for our purposes :) "Effectively correct" is too strong perhaps, "effectively effective" could be a better description. > Jay, could you also add a resource variable to the test below ? You may also want to add a test for enhanced for loop. See https://bugs.eclipse.org/bugs/show_bug.cgi?id=382721#c7
Created attachment 237419 [details] Updated patch Patch includes updated doc - have added 'not final' to the doc. Note: I have not specified that 'final' includes both implicit and explicit as this this implied. Also added more test cases and removed one unnecessary piece of code in LocalVariableBinding.
> Patch includes updated doc - have added 'not final' to the doc. Note: I have > not specified that 'final' includes both implicit and explicit as this this > implied. In general, I agree that keywords in Javadocs should be enclosed in <code>, but as you said, the 'final' in this case also includes implicitly final variables, so it would be better to not mark 'final' as a keyword here. > "effectively effective" Sounds good ;-). Maybe add a comment like // includes explicitly and implicitly final to TagBits#IsEffectivelyFinal to avoid surprises in a few years?
(In reply to Markus Keller from comment #7) > Sounds good ;-). Maybe add a comment like > // includes explicitly and implicitly final > to TagBits#IsEffectivelyFinal to avoid surprises in a few years? Jay, please incorporate this suggestion before releasing, thanks.
(In reply to Srikanth Sankaran from comment #8) > (In reply to Markus Keller from comment #7) > > > Sounds good ;-). Maybe add a comment like > > // includes explicitly and implicitly final > > to TagBits#IsEffectivelyFinal to avoid surprises in a few years? > > Jay, please incorporate this suggestion before releasing, thanks. Alternately, we can rename TagBits#IsEffectivelyFinal to be TagBits.IsImmutable
(In reply to Srikanth Sankaran from comment #9) > Alternately, we can rename TagBits#IsEffectivelyFinal to be > TagBits.IsImmutable That sounds misleading to me. If it's not explicitly or implicitly final it's not really immutable, right? Since this is internal, I think we can just go with additional comment.
(In reply to Jayaprakash Arthanareeswaran from comment #10) > (In reply to Srikanth Sankaran from comment #9) > > Alternately, we can rename TagBits#IsEffectivelyFinal to be > > TagBits.IsImmutable > > That sounds misleading to me. If it's not explicitly or implicitly final > it's not really immutable, right? Correct, but why do you think it is misleading ? We start out with IsImmutable set to true as we do now with IsEffectivelyFinal set to true and clear it on the first assignment we see after a definite or potential assignment.
(In reply to Srikanth Sankaran from comment #11) > Correct, but why do you think it is misleading ? We start out with > IsImmutable > set to true as we do now with IsEffectivelyFinal set to true and clear it on > the first assignment we see after a definite or potential assignment. I always thought immutable meant 'something that can't be changed'. But here we need something like, 'something that has not been changed', right?
(In reply to Jayaprakash Arthanareeswaran from comment #12) > I always thought immutable meant 'something that can't be changed'. But here > we need something like, 'something that has not been changed', right? OK, I see what you mean. I was looking for a word that would not call for change in code (other than the rename propagation) by having to invert polarity. Does either of IsCaptureableByLambda or IsUnmutated sound good ? Otherwise we can leave it as is + a comment explaining it also signifies final variables of explicit/implicit finality.
All tests passed and pushed the changes (with just a comment for isEffectivelyFinal) : http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?h=BETA_JAVA8&id=6c7de59cc4e8b7d3ff952cb73eea6ba69e83b190
Marking as resolved.
