From e9c6ca09e842b2f2fbc3f91398eea1e57a7e21f0 Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Wed, 12 Oct 2016 16:10:17 -0400 Subject: [PATCH] Bug 486682 - Syntax coloring of macro arguments that occur in reverse order in the AST This patch also fixes a couple of other bugs related to syntax coloring of macro expansions, which are exposed by this change (bug 490415, bug 496696). Change-Id: I3c0030ff61e721e099dc50afc109dd44e37276a3 --- .../parser/scanner/ASTPreprocessorName.java | 14 ++++++-- .../tests/text/SemanticHighlightingTest.java | 33 +++++++++++++++++++ .../SemanticHighlightingReconciler.java | 31 ++++++++--------- 3 files changed, 61 insertions(+), 17 deletions(-) diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/parser/scanner/ASTPreprocessorName.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/parser/scanner/ASTPreprocessorName.java index 180bed6ab2f..40f98e0e2e0 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/parser/scanner/ASTPreprocessorName.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/parser/scanner/ASTPreprocessorName.java @@ -30,7 +30,7 @@ import org.eclipse.core.runtime.IAdaptable; * Models IASTNames as needed for the preprocessor statements and macro expansions. * @since 5.0 */ -class ASTPreprocessorName extends ASTPreprocessorNode implements IASTName { +public class ASTPreprocessorName extends ASTPreprocessorNode implements IASTName { private final char[] fName; private final IBinding fBinding; @@ -236,6 +236,16 @@ class ASTMacroReferenceName extends ASTPreprocessorName { } return null; } - return super.getImageLocation(); + // ASTNode.getImageLocation() computes an image location based on the node location. + // Macro reference names which are nested references rather than the name of the + // macro being expanded itself, have their node location set to the entire macro + // expansion (see LocationMap.pushMacroExpansion()), which doesn't produce a + // useful image location. + if (getParent() instanceof ASTMacroExpansion) { + if (((ASTMacroExpansion) getParent()).getContext().getMacroReference() == this) { + return super.getImageLocation(); + } + } + return null; } } diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/SemanticHighlightingTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/SemanticHighlightingTest.java index 136e20a3d3c..2d8b49c07f6 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/SemanticHighlightingTest.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/SemanticHighlightingTest.java @@ -569,4 +569,37 @@ public class SemanticHighlightingTest extends TestCase { public void testVariableTemplates_486672() throws Exception { makeAssertions(); } + + // #define MACRO(Name, Type) Type Name(); //$macroDefinition + // typedef int Int; //$typedef + // class S { //$class + // MACRO(foo, Int) //$macroSubstitution,methodDeclaration,typedef + // }; + public void testMethodNameInsideMacro_486682() throws Exception { + makeAssertions(); + } + + // #define IF_0(t, f) f //$macroDefinition + // #define IF(bit, t, f) IF_ ## bit(t, f) //$macroDefinition + // #define WALDO //$macroDefinition + // #define MAIN(...) int main() { __VA_ARGS__ } //$macroDefinition + // + // MAIN //$macroSubstitution + // ( + // int x; //$localVariableDeclaration + // IF(0, WALDO, WALDO) //$macroSubstitution,macroSubstitution,macroSubstitution + // ) + public void testLexicalColoringInsideMacroExpansion_490415() throws Exception { + makeAssertions(); + } + + // #define N1(x) x + // #define M1(x) N1(x) + + // int main() { //$functionDeclaration + // M1(0); //$macroSubstitution + // } + public void testLexicalColoringInsideMacroExpansion_496696() throws Exception { + makeAssertions(); + } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/SemanticHighlightingReconciler.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/SemanticHighlightingReconciler.java index 5d006d5caa5..653fb5dce8b 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/SemanticHighlightingReconciler.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/SemanticHighlightingReconciler.java @@ -49,6 +49,7 @@ import org.eclipse.cdt.ui.CUIPlugin; import org.eclipse.cdt.internal.core.dom.parser.ASTNode; import org.eclipse.cdt.internal.core.model.ASTCache; +import org.eclipse.cdt.internal.core.parser.scanner.ASTPreprocessorName; import org.eclipse.cdt.internal.ui.editor.SemanticHighlightingManager.HighlightedPosition; import org.eclipse.cdt.internal.ui.editor.SemanticHighlightingManager.HighlightingStyle; @@ -67,10 +68,8 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener { private class PositionCollector extends ASTVisitor { /** The semantic token */ private SemanticToken fToken= new SemanticToken(); - private int fMinLocation; public PositionCollector(boolean visitImplicitNames) { - fMinLocation= -1; shouldVisitTranslationUnit= true; shouldVisitNames= true; shouldVisitDeclarations= true; @@ -92,7 +91,6 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener { visitNode(macroDef.getName()); } } - fMinLocation= -1; // Visit macro expansions. IASTPreprocessorMacroExpansion[] macroExps= tu.getMacroExpansions(); @@ -106,7 +104,6 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener { } } } - fMinLocation= -1; // Visit ordinary code. return super.visit(tu); @@ -215,11 +212,18 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener { } } else { // Fallback in case no image location available. - IASTNodeLocation[] nodeLocations= node.getNodeLocations(); - if (nodeLocations.length == 1) { - IASTNodeLocation nodeLocation = nodeLocations[0]; - if (!(nodeLocation instanceof IASTMacroExpansionLocation)) { - return nodeLocation; + // Only use the fallback for nodes that are not preprocessor nodes, + // because in the case of nested macro expansions, a preprocessor node + // can have a node location that is not representative of its actual + // image; such nodes should have an image location (accessed via + // getImageLocation(), above) where appropriate. + if (!(node instanceof ASTPreprocessorName)) { + IASTNodeLocation[] nodeLocations= node.getNodeLocations(); + if (nodeLocations.length == 1) { + IASTNodeLocation nodeLocation = nodeLocations[0]; + if (!(nodeLocation instanceof IASTMacroExpansionLocation)) { + return nodeLocation; + } } } } @@ -234,12 +238,9 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener { */ private void highlightLocation(IASTNodeLocation nodeLocation, HighlightingStyle highlightingStyle) { int offset= nodeLocation.getNodeOffset(); - if (offset >= fMinLocation) { - int length= nodeLocation.getNodeLength(); - if (offset > -1 && length > 0) { - fMinLocation= offset + length; - addPosition(offset, length, highlightingStyle); - } + int length= nodeLocation.getNodeLength(); + if (offset > -1 && length > 0) { + addPosition(offset, length, highlightingStyle); } }