Semantic highlighting: Improve handling of macro expansions

core/org.eclipse.cdt.ui.tests/resources/semanticHighlighting/SHTest.cpp

@@ -1,4 +1,4 @@
-#define SIMPLE_MACRO
+#define INT      int
 #define FUNCTION_MACRO(arg) globalFunc(arg)
 
 enum Enumeration {
@@ -102,16 +102,16 @@ namespace ns {
     }
 }
 
-int ClassContainer::protMethod() {
+INT ClassContainer::protMethod() {
     return protField;
 }
 
-int ClassContainer::pubMethod() {
+INT ClassContainer::pubMethod() {
     int localVar = 0;
     return pubField + localVar;
 }
 
-int ClassContainer::staticPrivMethod() {
+INT ClassContainer::staticPrivMethod() {
     CppStruct* st= new CppStruct();
     st->structField= 1;
     CppUnion un;

core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/SemanticHighlightingTest.java

@@ -27,7 +27,7 @@ import org.eclipse.cdt.internal.ui.editor.SemanticHighlightings;
  */
 public class SemanticHighlightingTest extends AbstractSemanticHighlightingTest {
 	
-	private static final boolean PRINT_POSITIONS= false;
+	private static final boolean PRINT_POSITIONS= true;
 
 	private static final Class THIS= SemanticHighlightingTest.class;
 	
@@ -206,6 +206,7 @@ public class SemanticHighlightingTest extends AbstractSemanticHighlightingTest {
 		Position[] expected= new Position[] {
 				createPosition(11, 20, 1),
 				createPosition(28, 35, 3),
+				createPosition(29, 8, 19),
 				createPosition(30, 19, 3),
 				createPosition(77, 21, 4),
 				createPosition(77, 30, 4),
@@ -313,9 +314,11 @@ public class SemanticHighlightingTest extends AbstractSemanticHighlightingTest {
 				createPosition(11, 5, 10),
 				createPosition(12, 12, 16),
 				createPosition(19, 16, 10),
+				createPosition(29, 8, 19),
 				createPosition(30, 8, 10),
 				createPosition(98, 8, 13),
 				createPosition(99, 1, 16),
+				createPosition(120, 4, 17),
 			};
 		if (PRINT_POSITIONS) System.out.println(toString(actual));
 		assertEqualPositions(expected, actual);
@@ -342,6 +345,9 @@ public class SemanticHighlightingTest extends AbstractSemanticHighlightingTest {
 		Position[] actual= getSemanticHighlightingPositions();
 		Position[] expected= new Position[] {
 				createPosition(29, 8, 19),
+				createPosition(104, 0, 3),
+				createPosition(108, 0, 3),
+				createPosition(113, 0, 3),
 				createPosition(120, 4, 17),
 			};
 		if (PRINT_POSITIONS) System.out.println(toString(actual));

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/SemanticHighlightingReconciler.java

@@ -31,8 +31,10 @@ import org.eclipse.ui.IWorkbenchPartSite;
 
 import org.eclipse.cdt.core.IPositionConverter;
 import org.eclipse.cdt.core.dom.ast.ASTVisitor;
+import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier;
 import org.eclipse.cdt.core.dom.ast.IASTDeclaration;
 import org.eclipse.cdt.core.dom.ast.IASTExpression;
+import org.eclipse.cdt.core.dom.ast.IASTFileLocation;
 import org.eclipse.cdt.core.dom.ast.IASTMacroExpansion;
 import org.eclipse.cdt.core.dom.ast.IASTName;
 import org.eclipse.cdt.core.dom.ast.IASTNode;
@@ -63,6 +65,7 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener {
 			shouldVisitNames= true;
 			shouldVisitDeclarations= true;
 			shouldVisitExpressions= true;
+			shouldVisitDeclSpecifiers= true;
 		}
 		
 		/** The semantic token */
@@ -86,11 +89,17 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener {
 			if (!fFilePath.equals(declaration.getContainingFilename())) {
 				return PROCESS_SKIP;
 			}
-			IASTNodeLocation[] nodeLocations= declaration.getNodeLocations();
+			if (checkForMacro(declaration)) {
-			if (nodeLocations.length > 0 && nodeLocations[0] instanceof IASTMacroExpansion) {
-				if (visitNode(declaration)) {
 				return PROCESS_SKIP;
 			}
+			return PROCESS_CONTINUE;
+		}
+
+		/*
+		 * @see org.eclipse.cdt.core.dom.ast.ASTVisitor#visit(org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier)
+		 */
+		public int visit(IASTDeclSpecifier declSpec) {
+			if (checkForMacro(declSpec)) {
 				return PROCESS_SKIP;
 			}
 			return PROCESS_CONTINUE;
@@ -100,14 +109,7 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener {
 		 * @see org.eclipse.cdt.core.dom.ast.ASTVisitor#visit(org.eclipse.cdt.core.dom.ast.IASTExpression)
 		 */
 		public int visit(IASTExpression expression) {
-			if (!fFilePath.equals(expression.getContainingFilename())) {
+			if (checkForMacro(expression)) {
-				return PROCESS_SKIP;
-			}
-			IASTNodeLocation[] nodeLocations= expression.getNodeLocations();
-			if (nodeLocations.length > 0 && nodeLocations[0] instanceof IASTMacroExpansion) {
-				if (visitNode(expression)) {
-					return PROCESS_SKIP;
-				}
 				return PROCESS_SKIP;
 			}
 			return PROCESS_CONTINUE;
@@ -117,23 +119,25 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener {
 		 * @see org.eclipse.cdt.core.dom.ast.ASTVisitor#visit(org.eclipse.cdt.core.dom.ast.IASTName)
 		 */
 		public int visit(IASTName node) {
-			if (!fFilePath.equals(node.getContainingFilename())) {
-				return PROCESS_SKIP;
-			}
 			visitNode(node);
 			return PROCESS_CONTINUE;
 		}
 		
+		private boolean checkForMacro(IASTNode node) {
+			IASTNodeLocation[] nodeLocations= node.getNodeLocations();
+			if (nodeLocations.length == 1 && nodeLocations[0] instanceof IASTMacroExpansion) {
+				return visitNode(node);
+			}
+			return false;
+		}
+
 		private boolean visitNode(IASTNode node) {
 			boolean consumed= false;
 			fToken.update(node);
 			for (int i= 0, n= fJobSemanticHighlightings.length; i < n; ++i) {
 				SemanticHighlighting semanticHighlighting= fJobSemanticHighlightings[i];
 				if (fJobHighlightings[i].isEnabled() && semanticHighlighting.consumes(fToken)) {
-					IASTNodeLocation[] nodeLocations= node.getNodeLocations();
+					addNodeLocations(node.getNodeLocations(), fJobHighlightings[i]);
-					if (nodeLocations.length > 0) {
-						addNodeLocations(nodeLocations, fJobHighlightings[i]);
-					}
 					consumed= true;
 					break;
 				}
@@ -143,21 +147,22 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener {
 		}
 
 		/**
-		 * Add all node locations for the given highlighting.
+		 * Add the a location range for the given highlighting.
 		 * 
 		 * @param nodeLocations The node locations
 		 * @param highlighting The highlighting
 		 */
 		private void addNodeLocations(IASTNodeLocation[] nodeLocations, HighlightingStyle highlighting) {
-			for (int j = 0; j < nodeLocations.length; j++) {
+			final IASTFileLocation minLocation= getMinFileLocation(nodeLocations);
-				IASTNodeLocation nodeLocation = nodeLocations[j];
+			if (minLocation == null) {
-				if (nodeLocation instanceof IASTMacroExpansion) {
+				return;
-					IASTMacroExpansion macroExpansion= (IASTMacroExpansion)nodeLocation;
+			}
-					IASTNodeLocation[] expansionLocations = macroExpansion.getExpansionLocations();
+			final IASTFileLocation maxLocation= getMaxFileLocation(nodeLocations);
-					addNodeLocations(expansionLocations, highlighting);
+			if (maxLocation == null) {
-				} else {
+				return;
-					int offset= nodeLocation.getNodeOffset();
+			}
-					int length= nodeLocation.getNodeLength();
+			int offset= minLocation.getNodeOffset();
+			int length= maxLocation.getNodeOffset() + maxLocation.getNodeLength() - offset;
 			if (fPositionTracker != null) {
 				IRegion actualPos= fPositionTracker.historicToActual(new Region(offset, length));
 				offset= actualPos.getOffset();
@@ -167,7 +172,33 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener {
 				addPosition(offset, length, highlighting);
 			}
 		}
+
+		private IASTFileLocation getMaxFileLocation(IASTNodeLocation[] locations) {
+			if (locations == null || locations.length == 0) {
+				return null;
 			}
+			final IASTNodeLocation nodeLocation= locations[locations.length-1];
+			if (nodeLocation instanceof IASTFileLocation) {
+				return (IASTFileLocation)nodeLocation;
+			} else if (nodeLocation instanceof IASTMacroExpansion) {
+				IASTNodeLocation[] macroLocations= ((IASTMacroExpansion)nodeLocation).getExpansionLocations();
+				return getMaxFileLocation(macroLocations);
+			}
+			return null;
+		}
+
+		private IASTFileLocation getMinFileLocation(IASTNodeLocation[] locations) {
+			if (locations == null || locations.length == 0) {
+				return null;
+			}
+			final IASTNodeLocation nodeLocation= locations[0];
+			if (nodeLocation instanceof IASTFileLocation) {
+				return (IASTFileLocation)nodeLocation;
+			} else if (nodeLocation instanceof IASTMacroExpansion) {
+				IASTNodeLocation[] macroLocations= ((IASTMacroExpansion)nodeLocation).getExpansionLocations();
+				return getMinFileLocation(macroLocations);
+			}
+			return null;
 		}
 
 		/**

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/SemanticHighlightings.java

@@ -1492,12 +1492,10 @@ public class SemanticHighlightings {
 		 */
 		public boolean consumes(SemanticToken token) {
 			IASTNode node= token.getNode();
-			if (!(node instanceof IASTName)) {
 			IASTNodeLocation[] nodeLocations= node.getNodeLocations();
 			if (nodeLocations.length == 1 && nodeLocations[0] instanceof IASTMacroExpansion) {
 				return true;
 			}
-			}
 			return false;
 		}
 	}