From a4a0c4e792fe52134be81f247989a707672ea085 Mon Sep 17 00:00:00 2001 From: Sarika Sinha Date: Wed, 22 Oct 2014 11:49:57 -0500 Subject: Bug 216098 - [breakpoints] Can't set a watchpoint on a final field Change-Id: I331d9ebd0102c1125194e61efe0ac2bafb75b1f0 Signed-off-by: Sarika Sinha --- .../testprograms/BreakpointsLocationBug344984.java | 17 -------- .../targets/BreakpointsLocationBug344984.java | 21 ++++++++++ .../eclipse/jdt/debug/tests/AbstractDebugTest.java | 3 +- .../BreakpointLocationVerificationTests.java | 29 ++----------- .../debug/tests/breakpoints/WatchpointTests.java | 37 ++++++++++++++++- .../internal/debug/ui/BreakpointMarkerUpdater.java | 5 +++ .../debug/ui/actions/ToggleBreakpointAdapter.java | 6 +++ .../ValidBreakpointLocationLocator.java | 48 ++++++++++++---------- 8 files changed, 101 insertions(+), 65 deletions(-) delete mode 100644 org.eclipse.jdt.debug.tests/testprograms/BreakpointsLocationBug344984.java create mode 100644 org.eclipse.jdt.debug.tests/testprograms/org/eclipse/debug/tests/targets/BreakpointsLocationBug344984.java diff --git a/org.eclipse.jdt.debug.tests/testprograms/BreakpointsLocationBug344984.java b/org.eclipse.jdt.debug.tests/testprograms/BreakpointsLocationBug344984.java deleted file mode 100644 index 4c702f09c..000000000 --- a/org.eclipse.jdt.debug.tests/testprograms/BreakpointsLocationBug344984.java +++ /dev/null @@ -1,17 +0,0 @@ -/******************************************************************************* - * Copyright (c) 2012 IBM Corporation and others. - * All rights reserved. This program and the accompanying materials - * are made available under the terms of the Eclipse Public License v1.0 - * which accompanies this distribution, and is available at - * http://www.eclipse.org/legal/epl-v10.html - * - * Contributors: - * IBM Corporation - initial API and implementation - *******************************************************************************/ -public class BreakpointsLocationBug344984 { - private final String fWorkingValues; // Breakpoint here - BreakpointsLocationBug344984() { - fWorkingValues= null; - System.out.println(fWorkingValues); - } -} \ No newline at end of file diff --git a/org.eclipse.jdt.debug.tests/testprograms/org/eclipse/debug/tests/targets/BreakpointsLocationBug344984.java b/org.eclipse.jdt.debug.tests/testprograms/org/eclipse/debug/tests/targets/BreakpointsLocationBug344984.java new file mode 100644 index 000000000..7b99c50a0 --- /dev/null +++ b/org.eclipse.jdt.debug.tests/testprograms/org/eclipse/debug/tests/targets/BreakpointsLocationBug344984.java @@ -0,0 +1,21 @@ +/******************************************************************************* + * Copyright (c) 2014 IBM Corporation and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * IBM Corporation - initial API and implementation + *******************************************************************************/ +package org.eclipse.debug.tests.targets; +public class BreakpointsLocationBug344984 { + private final String fWorkingValues; // Breakpoint here + BreakpointsLocationBug344984() { + fWorkingValues= null; + System.out.println(fWorkingValues); + } + public static void main(String[] args) { + new BreakpointsLocationBug344984(); + } +} \ No newline at end of file diff --git a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AbstractDebugTest.java b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AbstractDebugTest.java index 729d2e3a6..05f49d329 100644 --- a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AbstractDebugTest.java +++ b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AbstractDebugTest.java @@ -182,7 +182,8 @@ public abstract class AbstractDebugTest extends TestCase implements IEvaluation public static final String BOUND_JRE_PROJECT_NAME = "BoundJRE"; final String[] LAUNCH_CONFIG_NAMES_1_4 = {"LargeSourceFile", "LotsOfFields", "Breakpoints", "InstanceVariablesTests", "LocalVariablesTests", "LocalVariableTests2", "StaticVariablesTests", - "DropTests", "ThrowsNPE", "ThrowsException", "org.eclipse.debug.tests.targets.Watchpoint", "org.eclipse.debug.tests.targets.CallLoop", "A", + "DropTests", "ThrowsNPE", "ThrowsException", "org.eclipse.debug.tests.targets.Watchpoint", + "org.eclipse.debug.tests.targets.BreakpointsLocationBug344984", "org.eclipse.debug.tests.targets.CallLoop", "A", "HitCountLooper", "CompileError", "MultiThreadedLoop", "HitCountException", "MultiThreadedException", "MultiThreadedList", "MethodLoop", "StepFilterOne", "StepFilterFour", "EvalArrayTests", "EvalSimpleTests", "EvalTypeTests", "EvalNestedTypeTests", "EvalTypeHierarchyTests", "WorkingDirectoryTest", "OneToTen", "OneToTenPrint", "FloodConsole", "ConditionalStepReturn", "VariableChanges", "DefPkgReturnType", "InstanceFilterObject", "org.eclipse.debug.tests.targets.CallStack", diff --git a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/BreakpointLocationVerificationTests.java b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/BreakpointLocationVerificationTests.java index 1413bcc4e..6aa2be25a 100644 --- a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/BreakpointLocationVerificationTests.java +++ b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/BreakpointLocationVerificationTests.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2012 IBM Corporation and others. + * Copyright (c) 2000, 2014 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -109,27 +109,6 @@ public class BreakpointLocationVerificationTests extends AbstractDebugTest { testLocation(14, 14, "FinalBreakpointLocations", "FinalBreakpointLocations", true); } - /** - * Tests setting a line breakpoint on a final field with an Expression as an initializer - * @see https://bugs.eclipse.org/bugs/show_bug.cgi?id=376354 - * - * @throws Exception - */ - public void testFinalFieldWithTypeDecl2() throws Exception { - testLocation(15, 15, "FinalBreakpointLocations"); - } - - /** - * Tests setting a line breakpoint on a final field with an Expression as an initializer looking - * for best match - * @see https://bugs.eclipse.org/bugs/show_bug.cgi?id=376354 - * - * @throws Exception - */ - public void testFinalFieldWithTypeDecl2a() throws Exception { - testLocation(15, 15, "FinalBreakpointLocations", "FinalBreakpointLocations", true); - } - /** * Tests setting a line breakpoint on an inner type member for the initializer of * a final local field variable @@ -150,7 +129,7 @@ public class BreakpointLocationVerificationTests extends AbstractDebugTest { * @throws Exception */ public void testFinalFieldWithTypeDecl3a() throws Exception { - testLocation(17, 17, "FinalBreakpointLocations"); + testLocation(17, 17, "FinalBreakpointLocations", "FinalBreakpointLocations", true); } /** @@ -252,7 +231,7 @@ public class BreakpointLocationVerificationTests extends AbstractDebugTest { * @throws Exception */ public void testFieldLocationOnFinalField() throws Exception { - testLocation(12, 14, "BreakpointsLocationBug344984"); + testLocation(13, 13, "org.eclipse.debug.tests.targets.BreakpointsLocationBug344984"); } /** @@ -261,7 +240,7 @@ public class BreakpointLocationVerificationTests extends AbstractDebugTest { * @throws Exception */ public void testFieldLocationOnFinalFielda() throws Exception { - testLocation(12, 14, "BreakpointsLocationBug344984", "BreakpointsLocationBug344984", true); + testLocation(13, 13, "org.eclipse.debug.tests.targets.BreakpointsLocationBug344984", "org.eclipse.debug.tests.targets.BreakpointsLocationBug344984", true); } /** diff --git a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/WatchpointTests.java b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/WatchpointTests.java index 1f6ae2ebf..abea61a1f 100644 --- a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/WatchpointTests.java +++ b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/WatchpointTests.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2014 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -297,4 +297,39 @@ public class WatchpointTests extends AbstractDebugTest { getBreakpointManager().setEnabled(true); } } + + /** + * Tests that a watchpoint set to be skipped is indeed skipped + * + * @throws Exception + */ + public void testFinalWatchpoint() throws Exception { + String typeName = "org.eclipse.debug.tests.targets.BreakpointsLocationBug344984"; + + IJavaWatchpoint wp = createWatchpoint(typeName, "fWorkingValues", true, true); + + IJavaThread thread = null; + try { + thread = launchToBreakpoint(typeName); + assertNotNull("Breakpoint not hit within timeout period", thread); + + IBreakpoint hit = getBreakpoint(thread); + IStackFrame frame = thread.getTopStackFrame(); + assertNotNull("No watchpoint", hit); + + // should be modification + assertTrue("First hit should be modification", !wp.isAccessSuspend(thread.getDebugTarget())); + // line 27 + assertEquals("Should be on line 15", 15, frame.getLineNumber()); + + getBreakpointManager().setEnabled(false); + resumeAndExit(thread); + + } + finally { + terminateAndRemove(thread); + removeAllBreakpoints(); + getBreakpointManager().setEnabled(true); + } + } } diff --git a/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/BreakpointMarkerUpdater.java b/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/BreakpointMarkerUpdater.java index b5c09023f..b0277cecf 100644 --- a/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/BreakpointMarkerUpdater.java +++ b/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/BreakpointMarkerUpdater.java @@ -23,6 +23,7 @@ import org.eclipse.jdt.core.dom.CompilationUnit; import org.eclipse.jdt.debug.core.IJavaLineBreakpoint; import org.eclipse.jdt.debug.core.IJavaPatternBreakpoint; import org.eclipse.jdt.debug.core.IJavaStratumLineBreakpoint; +import org.eclipse.jdt.debug.core.IJavaWatchpoint; import org.eclipse.jdt.internal.debug.core.JDIDebugPlugin; import org.eclipse.jdt.internal.debug.core.JavaDebugUtils; import org.eclipse.jdt.internal.debug.core.breakpoints.JavaLineBreakpoint; @@ -111,6 +112,10 @@ public class BreakpointMarkerUpdater implements IMarkerUpdater { if(loc.getLocationType() == ValidBreakpointLocationLocator.LOCATION_NOT_FOUND) { return false; } + // Remove the watch point if it is not a valid watch point now + if (loc.getLocationType() != ValidBreakpointLocationLocator.LOCATION_FIELD && breakpoint instanceof IJavaWatchpoint) { + return false; + } int line = loc.getLineLocation(); //if the line number is already good, perform no marker updating if(MarkerUtilities.getLineNumber(marker) == line) { diff --git a/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/actions/ToggleBreakpointAdapter.java b/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/actions/ToggleBreakpointAdapter.java index 94288c85c..bb810b458 100644 --- a/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/actions/ToggleBreakpointAdapter.java +++ b/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/actions/ToggleBreakpointAdapter.java @@ -897,12 +897,18 @@ public class ToggleBreakpointAdapter implements IToggleBreakpointsTargetExtensio fieldName = javaField.getElementName(); int f = javaField.getFlags(); boolean fin = Flags.isFinal(f); + if (fin) { + fin = javaField.getConstant() != null; // watch point is allowed if no constant value + } allowed = !(fin) & !(Flags.isStatic(f) & fin); } else if (element instanceof IJavaFieldVariable) { IJavaFieldVariable var = (IJavaFieldVariable) element; typeName = var.getDeclaringType().getName(); fieldName = var.getName(); boolean fin = var.isFinal(); + if (fin) { + fin = javaField.getConstant() != null; // watch point is allowed if no constant value + } allowed = !(fin) & !(var.isStatic() & fin); } breakpoint = getWatchpoint(typeName, fieldName); diff --git a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/ValidBreakpointLocationLocator.java b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/ValidBreakpointLocationLocator.java index 3b74f5ccb..877f2ce3d 100644 --- a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/ValidBreakpointLocationLocator.java +++ b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/ValidBreakpointLocationLocator.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2003, 2012 IBM Corporation and others. + * Copyright (c) 2003, 2014 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -185,14 +185,13 @@ public class ValidBreakpointLocationLocator extends ASTVisitor { } /** - * Return the line number of the computed valid location, if the location - * type is LOCATION_LINE. + * Return the line number of the computed valid location */ public int getLineLocation() { - if (fLocationType == LOCATION_LINE) { - return fLineLocation; + if (fLocationType == LOCATION_NOT_FOUND) { + return -1; } - return -1; + return fLineLocation; } /** @@ -731,17 +730,28 @@ public class ValidBreakpointLocationLocator extends ASTVisitor { int line = lineNumber(offset); if(Flags.isFinal(node.getModifiers())) { if(init != null) { - if (line == fLineNumber && isReplacedByConstantValue(init)) { + if (line == fLineNumber) { + if (isReplacedByConstantValue(init)) { + fLocationType = LOCATION_LINE; + } else { + fLocationType = LOCATION_FIELD; + } fMemberOffset = offset; fLineLocation = line; - fLocationType = LOCATION_LINE; fLocationFound = true; fTypeName = computeTypeName(node); return false; } } else { - //if it is an uninitialized final field, try to find the next executable line + if (line == fLineNumber) { + fMemberOffset = offset; + fLineLocation = line; + fLocationType = LOCATION_FIELD; + fLocationFound = true; + fTypeName = computeTypeName(node); + return false; + } return false; } } @@ -750,6 +760,7 @@ public class ValidBreakpointLocationLocator extends ASTVisitor { // contains the name of the field if (line == fLineNumber) { fMemberOffset = offset; + fLineLocation = line; fLocationType = LOCATION_FIELD; fLocationFound = true; return false; @@ -1369,10 +1380,11 @@ public class ValidBreakpointLocationLocator extends ASTVisitor { public boolean visit(VariableDeclarationFragment node) { Expression initializer = node.getInitializer(); if (visit(node, false)) { - int startLine = lineNumber(node.getName().getStartPosition()); + int offset = node.getName().getStartPosition(); + int line = lineNumber(offset); if (initializer != null) { - if (fLineNumber == startLine) { - fLineLocation = startLine; + if (fLineNumber == line) { + fLineLocation = line; fLocationFound = true; fLocationType = LOCATION_LINE; fTypeName = computeTypeName(node); @@ -1381,19 +1393,13 @@ public class ValidBreakpointLocationLocator extends ASTVisitor { initializer.accept(this); } else { // the variable has no initializer - int offset = node.getName().getStartPosition(); // check if the breakpoint is to be set on the line which // contains the name of the field - ASTNode parent = node.getParent(); - if(parent.getNodeType() == ASTNode.FIELD_DECLARATION) { - //if the parent field is final and we are not initializing, find the next executable line - if(Flags.isFinal(((FieldDeclaration)parent).getModifiers())) { - return false; - } - } - if (lineNumber(offset) == fLineNumber) { + if (line == fLineNumber) { fMemberOffset = offset; + fLineLocation = line; fLocationType = LOCATION_FIELD; + fTypeName = computeTypeName(node); fLocationFound = true; return false; } -- cgit v1.2.3