Bug 487764 - Add a highlighting for variables passed by non-const reference

The highlighting is disabled by default.

Change-Id: I0b9e587c3b9d7206614937893d535825a4be99e5

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

@@ -602,4 +602,28 @@ public class SemanticHighlightingTest extends TestCase {
     public void testLexicalColoringInsideMacroExpansion_496696() throws Exception {
     	makeAssertions();
     }
+    
+    //	void foo(int&);                                  //$functionDeclaration
+    //	struct S {                                       //$class
+    //		int x;                                       //$field
+    //  };                                     
+    //	void bar(int x) {                                //$functionDeclaration,parameterVariable
+    //		foo(x);                                      //$function,variablePassedByNonconstRef
+    //		S s;                                         //$class,localVariableDeclaration
+    //	    foo(s.x);                                    //$function,variablePassedByNonconstRef
+    //	}
+    public void testVariablePassedByNonconstRef_487764a() throws Exception {
+    	makeAssertions();
+    }
+    
+    //	template <typename... Args>                      //$templateParameter
+    //	void foo(Args&&... args);                        //$functionDeclaration,templateParameter,parameterVariable
+    //	void bar() {                                     //$functionDeclaration
+    //		const int x;                                 //$localVariableDeclaration
+    //		int y;                                       //$localVariableDeclaration
+    //		foo(x, y, "waldo");                          //$function,localVariable,variablePassedByNonconstRef
+    //	}
+    public void testVariablePassedByNonconstRef_487764b() throws Exception {
+    	makeAssertions();
+    }
 }

core/org.eclipse.cdt.ui/plugin.xml

@@ -887,6 +887,12 @@
             label="%Dummy.label"
             value="200, 100, 0">            
       </colorDefinition>
+      <colorDefinition
+            id="org.eclipse.cdt.ui.variablePassedByNonConstReferenceHighlighting"
+            isEditable="false"
+            label="%Dummy.label"
+            value="200, 100, 150">            
+      </colorDefinition>
       <theme
          id="org.eclipse.ui.ide.systemDefault">
          <colorOverride

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

@@ -107,6 +107,7 @@ public final class CEditorMessages extends NLS {
 	public static String SemanticHighlighting_label;
 	public static String SemanticHighlighting_problem;
 	public static String SemanticHighlighting_externalSDK;
+	public static String SemanticHighlighting_variablePassedByNonConstReference;
 	public static String CEditor_markOccurrences_job_name;
 	public static String CEditorActionContributor_ExpandSelectionMenu_label;
 	public static String IndexUpdateRequestor_job_name;

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/CEditorMessages.properties

@@ -104,6 +104,7 @@ SemanticHighlighting_namespace= Namespaces
 SemanticHighlighting_label= Labels
 SemanticHighlighting_problem= Problems
 SemanticHighlighting_externalSDK= External SDK calls
+SemanticHighlighting_variablePassedByNonConstReference= Variables passed by non-const reference
 
 CEditor_markOccurrences_job_name= Occurrences Marker
 CEditorActionContributor_ExpandSelectionMenu_label=E&xpand Selection To

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

@@ -39,6 +39,13 @@ public abstract class SemanticHighlighting {
 		return false;
 	}
 	
+	/**
+	 * Indicates that the highlighting needs to visit expressions.
+	 */
+	public boolean requiresExpressions() {
+		return false;
+	}
+
 	/**
 	 * Returns <code>true</code> iff the semantic highlighting consumes the semantic token.
 	 * <p>

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

@@ -31,6 +31,7 @@ import org.eclipse.ui.IWorkbenchPartSite;
 import org.eclipse.cdt.core.dom.ast.ASTVisitor;
 import org.eclipse.cdt.core.dom.ast.IASTDeclaration;
 import org.eclipse.cdt.core.dom.ast.IASTDeclarator;
+import org.eclipse.cdt.core.dom.ast.IASTExpression;
 import org.eclipse.cdt.core.dom.ast.IASTImageLocation;
 import org.eclipse.cdt.core.dom.ast.IASTMacroExpansionLocation;
 import org.eclipse.cdt.core.dom.ast.IASTName;
@@ -62,6 +63,12 @@ import org.eclipse.cdt.internal.ui.text.ICReconcilingListener;
  * @since 4.0
  */
 public class SemanticHighlightingReconciler implements ICReconcilingListener {
+	
+	private class PositionCollectorRequirements {
+		public boolean visitImplicitNames = false;
+		public boolean visitExpressions = false;
+	}
+	
 	/**
 	 * Collects positions from the AST.
 	 */
@@ -69,17 +76,17 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener {
 		/** The semantic token */
 		private SemanticToken fToken= new SemanticToken();
 		
-		public PositionCollector(boolean visitImplicitNames) {
+		public PositionCollector(PositionCollectorRequirements requirements) {
 			shouldVisitTranslationUnit= true;
 			shouldVisitNames= true;
 			shouldVisitDeclarations= true;
-			shouldVisitExpressions= true;
+			shouldVisitExpressions= requirements.visitExpressions;
 			shouldVisitStatements= true;
 			shouldVisitDeclarators= true;
 			shouldVisitNamespaces= true;
 			shouldVisitVirtSpecifiers= true;
-			shouldVisitImplicitNames = visitImplicitNames;
+			shouldVisitImplicitNames = requirements.visitImplicitNames;
-			shouldVisitImplicitNameAlternates = visitImplicitNames;
+			shouldVisitImplicitNameAlternates = requirements.visitImplicitNames;
 		}
 
 		@Override
@@ -175,11 +182,24 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener {
 			return PROCESS_CONTINUE;
 		}
 		
+		@Override
+		public int visit(IASTExpression expression) {
+			if (visitNode(expression)) {
+				return PROCESS_SKIP;
+			}
+			return PROCESS_CONTINUE;
+		}
+		
 		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 the semantic highlighting doesn't color expressions, don't bother
+				// passing it one to begin with.
+				if (node instanceof IASTExpression && !semanticHighlighting.requiresExpressions()) {
+					continue;
+				}
 				if (fJobHighlightings[i].isEnabled() && semanticHighlighting.consumes(fToken)) {
 					IASTNodeLocation location = getLocationToHighlight(node);
 					if (location != null) {
@@ -344,7 +364,7 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener {
 			if (ast == null || fJobPresenter.isCanceled())
 				return;
 			
-			PositionCollector collector= new PositionCollector(requiresImplicitNames());
+			PositionCollector collector= new PositionCollector(getRequirements());
 
 			startReconcilingPositions();
 			
@@ -369,14 +389,20 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener {
 		}
 	}
 
-	private boolean requiresImplicitNames() {
+	private PositionCollectorRequirements getRequirements() {
+		PositionCollectorRequirements result = new PositionCollectorRequirements();
 		for (int i = 0; i < fSemanticHighlightings.length; i++) {
 			SemanticHighlighting sh = fSemanticHighlightings[i];
-			if (sh.requiresImplicitNames() && fHighlightings[i].isEnabled()) {
+			if (fHighlightings[i].isEnabled()) {
-				return true;
+				if (sh.requiresImplicitNames()) {
+					result.visitImplicitNames = true;
+				}
+				if (sh.requiresExpressions()) {
+					result.visitExpressions = true;
 				}
 			}
-		return false;
+		}
+		return result;
 	}
 
 	/**

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

@@ -24,11 +24,15 @@ import org.eclipse.swt.graphics.RGB;
 
 import org.eclipse.cdt.core.dom.ast.DOMException;
 import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier;
+import org.eclipse.cdt.core.dom.ast.IASTExpression;
+import org.eclipse.cdt.core.dom.ast.IASTFunctionCallExpression;
 import org.eclipse.cdt.core.dom.ast.IASTFunctionDeclarator;
 import org.eclipse.cdt.core.dom.ast.IASTImplicitName;
+import org.eclipse.cdt.core.dom.ast.IASTInitializerClause;
 import org.eclipse.cdt.core.dom.ast.IASTName;
 import org.eclipse.cdt.core.dom.ast.IASTNode;
 import org.eclipse.cdt.core.dom.ast.IASTProblem;
+import org.eclipse.cdt.core.dom.ast.IArrayType;
 import org.eclipse.cdt.core.dom.ast.IBinding;
 import org.eclipse.cdt.core.dom.ast.ICompositeType;
 import org.eclipse.cdt.core.dom.ast.IEnumeration;
@@ -39,9 +43,12 @@ import org.eclipse.cdt.core.dom.ast.ILabel;
 import org.eclipse.cdt.core.dom.ast.IMacroBinding;
 import org.eclipse.cdt.core.dom.ast.IParameter;
 import org.eclipse.cdt.core.dom.ast.IProblemBinding;
+import org.eclipse.cdt.core.dom.ast.IQualifierType;
 import org.eclipse.cdt.core.dom.ast.IScope;
+import org.eclipse.cdt.core.dom.ast.IType;
 import org.eclipse.cdt.core.dom.ast.ITypedef;
 import org.eclipse.cdt.core.dom.ast.IVariable;
+import org.eclipse.cdt.core.dom.ast.IASTExpression.ValueCategory;
 import org.eclipse.cdt.core.dom.ast.c.ICExternalBinding;
 import org.eclipse.cdt.core.dom.ast.c.ICFunctionScope;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTClassVirtSpecifier;
@@ -59,8 +66,10 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPConstructor;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPDeferredFunction;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPFunction;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPFunctionScope;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPFunctionType;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPMethod;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPNamespace;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPReferenceType;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPTemplateNonTypeParameter;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPTemplateParameter;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPUsingDeclaration;
@@ -77,6 +86,10 @@ import org.eclipse.cdt.ui.text.ISemanticToken;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPUnknownBinding;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.OverloadableOperator;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.HeuristicResolver;
+import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil;
+
+import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil.REF;
+import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil.TDEF;
 
 /**
  * Semantic highlightings.
@@ -202,6 +215,11 @@ public class SemanticHighlightings {
 	 */
 	public static final String OVERLOADED_OPERATOR= "overloadedOperator"; //$NON-NLS-1$
 	
+	/**
+	 * A named preference part that controls the highlighting of variables passed by non-const reference.
+	 */
+	public static final String VARIABLE_PASSED_BY_NONCONST_REF= "variablePassedByNonconstRef";  //$NON-NLS-1$
+	
 	/** Init debugging mode */
 	private static final boolean DEBUG= Boolean.parseBoolean(Platform.getDebugOption("org.eclipse.cdt.ui/debug/SemanticHighlighting"));  //$NON-NLS-1$
 
@@ -1596,6 +1614,111 @@ public class SemanticHighlightings {
 		}
 	}
 	
+	/**
+	 * Semantic highlighting for variables passed by non-const reference.
+	 * 
+	 * The purpose of having a highlighting for this is that there's an important
+	 * semantic difference between passing a variable by non-const reference 
+	 * (where the called function can modify the original variable) and passing
+	 * a variable by const reference or value (where it cannot), but syntactically
+	 * these two forms of passing look the same at the call site.
+	 */
+	private static final class VariablePassedByNonconstRefHighlighting 
+			extends SemanticHighlightingWithOwnPreference {
+
+		@Override
+		public String getPreferenceKey() {
+			return VARIABLE_PASSED_BY_NONCONST_REF;
+		}
+		
+		@Override
+		public boolean requiresExpressions() {
+			return true;
+		}
+
+		@Override
+		public RGB getDefaultDefaultTextColor() {
+			return new RGB(200, 100, 150); // dark pink
+		}
+		
+		@Override
+		public boolean isBoldByDefault() {
+			return true;
+		}
+
+		@Override
+		public boolean isEnabledByDefault() {
+			return false;
+		}
+
+		@Override
+		public String getDisplayName() {
+			return CEditorMessages.SemanticHighlighting_variablePassedByNonConstReference;
+		}
+
+		@Override
+		public boolean consumes(ISemanticToken token) {
+			IASTNode node = token.getNode();
+			
+			// This highlighting only applies to function arguments.
+			boolean isFunctionArgument = (node instanceof IASTExpression) &&
+					                     (node.getParent() instanceof IASTFunctionCallExpression) &&
+					                     (node.getPropertyInParent() == IASTFunctionCallExpression.ARGUMENT);
+			if (!isFunctionArgument) {
+				return false;
+			}
+			
+			// If the argument expression is not an lvalue, we don't care if the function
+			// modifies it. 
+			IASTExpression expression = (IASTExpression) node;
+			if (expression.getValueCategory() != ValueCategory.LVALUE) {
+				return false;
+			}
+			
+			// Resolve the type of the function being called.
+			// Note that, in the case of a call to a template function or a set of overloaded functions,
+			// the function name expression will be an id-expression, and 
+			// CPPASTIdExpression.getExpressionType() will perform template instantiation and overload 
+			// resolution, and we'll get the instantiated function type.
+			IASTFunctionCallExpression functionCall = ((IASTFunctionCallExpression) node.getParent());
+			IType functionType = functionCall.getFunctionNameExpression().getExpressionType();
+			functionType = SemanticUtil.getNestedType(functionType, TDEF | REF);
+			if (!(functionType instanceof ICPPFunctionType)) {
+				return false;
+			}
+			
+			// Find the parameter type matching the argument.
+			IType[] parameterTypes = ((ICPPFunctionType) functionType).getParameterTypes();
+			int argIndex = -1;
+			IASTInitializerClause[] arguments = functionCall.getArguments();
+			for (int i = 0; i < arguments.length; ++i) {
+				if (arguments[i] == expression) {
+					argIndex = i;
+					break;
+				}
+			}
+			if (argIndex == -1 || argIndex >= parameterTypes.length) {
+				return false;
+			}
+			IType parameterType = parameterTypes[argIndex];
+			
+			// Consume the node if the parameter type is a non-const reference.
+			// In the case of an reference to an array type, it is the element type
+			// which must be non-const.
+			parameterType = SemanticUtil.getNestedType(parameterType, TDEF);
+			if (parameterType instanceof ICPPReferenceType) {
+				IType referredType = ((ICPPReferenceType) parameterType).getType();
+				if (referredType instanceof IArrayType) {
+					referredType = ((IArrayType) referredType).getType();
+				}
+				boolean isConstRef = (referredType instanceof IQualifierType) &&
+						             ((IQualifierType) referredType).isConst();
+				return !isConstRef;
+			}
+			return false;
+		}
+	}
+	
 	/**
 	 * Semantic highlighting for context-sensitive keywords.
 	 * 
@@ -1799,6 +1922,7 @@ public class SemanticHighlightings {
 		highlightings.put(new Key(220), new LabelHighlighting());
 		highlightings.put(new Key(230), new EnumeratorHighlighting());
 		highlightings.put(new Key(240), new ContextSensitiveKeywordHighlighting());
+		highlightings.put(new Key(250), new VariablePassedByNonconstRefHighlighting());
 	}
 
 	private static final String ExtensionPoint = "semanticHighlighting"; //$NON-NLS-1$

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/preferences/CEditorColoringConfigurationBlock.java

@@ -891,27 +891,30 @@ class CEditorColoringConfigurationBlock extends AbstractConfigurationBlock {
 			{ createHighlightedRange( 8,  2,  4, SemanticHighlightings.GLOBAL_VARIABLE) },
 			{ createHighlightedRange( 8,  7,  2, SemanticHighlightings.OVERLOADED_OPERATOR) },
 			{ createHighlightedRange( 9,  9,  9, SemanticHighlightings.PARAMETER_VARIABLE) },
-			{ createHighlightedRange(11,  6,  7, SemanticHighlightings.CLASS) },
+			{ createHighlightedRange(11,  5,  7, SemanticHighlightings.FUNCTION_DECLARATION), createHighlightedRange(11,  5,  7, SemanticHighlightings.FUNCTION) },
-			{ createHighlightedRange(13,  7,  6, SemanticHighlightings.ENUM) },
+			{ createHighlightedRange(12,  6,  7, SemanticHighlightings.CLASS) },
-			{ createHighlightedRange(13, 16,  4, SemanticHighlightings.ENUMERATOR) },
+			{ createHighlightedRange(14,  7,  6, SemanticHighlightings.ENUM) },
-			{ createHighlightedRange(13, 22,  3, SemanticHighlightings.ENUMERATOR) },
+			{ createHighlightedRange(14, 16,  4, SemanticHighlightings.ENUMERATOR) },
-			{ createHighlightedRange(13, 27,  3, SemanticHighlightings.ENUMERATOR) },
+			{ createHighlightedRange(14, 22,  3, SemanticHighlightings.ENUMERATOR) },
-			{ createHighlightedRange(14, 14, 11, SemanticHighlightings.STATIC_FIELD), createHighlightedRange(14, 14, 11, SemanticHighlightings.FIELD) },
+			{ createHighlightedRange(14, 27,  3, SemanticHighlightings.ENUMERATOR) },
-			{ createHighlightedRange(15,  6,  5, SemanticHighlightings.FIELD) },
+			{ createHighlightedRange(15, 14, 11, SemanticHighlightings.STATIC_FIELD), createHighlightedRange(14, 14, 11, SemanticHighlightings.FIELD) },
-			{ createHighlightedRange(16, 10,  6, SemanticHighlightings.ENUM) },
+			{ createHighlightedRange(16,  6,  5, SemanticHighlightings.FIELD) },
-			{ createHighlightedRange(16, 17,  7, SemanticHighlightings.METHOD_DECLARATION), createHighlightedRange(16, 17,  7, SemanticHighlightings.METHOD) },
+			{ createHighlightedRange(17, 10,  6, SemanticHighlightings.ENUM) },
-			{ createHighlightedRange(17,  7,  6, SemanticHighlightings.METHOD_DECLARATION), createHighlightedRange(17,  7,  6, SemanticHighlightings.METHOD) },
+			{ createHighlightedRange(17, 17,  7, SemanticHighlightings.METHOD_DECLARATION), createHighlightedRange(16, 17,  7, SemanticHighlightings.METHOD) },
-			{ createHighlightedRange(17, 14,  6, SemanticHighlightings.ENUM) },
+			{ createHighlightedRange(18,  7,  6, SemanticHighlightings.METHOD_DECLARATION), createHighlightedRange(17,  7,  6, SemanticHighlightings.METHOD) },
-			{ createHighlightedRange(17, 21,  1, SemanticHighlightings.PARAMETER_VARIABLE) },
+			{ createHighlightedRange(18, 14,  6, SemanticHighlightings.ENUM) },
-			{ createHighlightedRange(18,  8,  5, SemanticHighlightings.LOCAL_VARIABLE_DECLARATION) },
+			{ createHighlightedRange(18, 21,  1, SemanticHighlightings.PARAMETER_VARIABLE) },
-			{ createHighlightedRange(18, 20,  5, SemanticHighlightings.MACRO_REFERENCE) },
+			{ createHighlightedRange(19,  8,  5, SemanticHighlightings.LOCAL_VARIABLE_DECLARATION) },
-			{ createHighlightedRange(19,  0,  5, SemanticHighlightings.LABEL) },
+			{ createHighlightedRange(19, 20,  5, SemanticHighlightings.MACRO_REFERENCE) },
-			{ createHighlightedRange(19,  7,  6, SemanticHighlightings.FUNCTION) },
+			{ createHighlightedRange(20,  0,  5, SemanticHighlightings.LABEL) },
-			{ createHighlightedRange(19, 14,  5, SemanticHighlightings.LOCAL_VARIABLE) },
+			{ createHighlightedRange(20,  7,  6, SemanticHighlightings.FUNCTION) },
-			{ createHighlightedRange(20,  4,  7, SemanticHighlightings.METHOD) },
+			{ createHighlightedRange(20, 14,  5, SemanticHighlightings.LOCAL_VARIABLE) },
-			{ createHighlightedRange(21,  4, 12, SemanticHighlightings.STATIC_METHOD_INVOCATION), createHighlightedRange(21,  4, 12, SemanticHighlightings.METHOD) },
+			{ createHighlightedRange(21,  4,  7, SemanticHighlightings.METHOD) },
-			{ createHighlightedRange(22,  4,  7, SemanticHighlightings.PROBLEM) },
+			{ createHighlightedRange(22,  4, 12, SemanticHighlightings.STATIC_METHOD_INVOCATION), createHighlightedRange(21,  4, 12, SemanticHighlightings.METHOD) },
-			{ createHighlightedRange(24, 14, 12, SemanticHighlightings.METHOD_DECLARATION), createHighlightedRange(24, 14, 12, SemanticHighlightings.METHOD) },
+			{ createHighlightedRange(23,  4,  7, SemanticHighlightings.PROBLEM) },
+			{ createHighlightedRange(24,  4,  7, SemanticHighlightings.FUNCTION) },
+			{ createHighlightedRange(24, 12,  5, SemanticHighlightings.VARIABLE_PASSED_BY_NONCONST_REF), createHighlightedRange(24, 12,  5, SemanticHighlightings.LOCAL_VARIABLE) },
+			{ createHighlightedRange(26, 14, 12, SemanticHighlightings.METHOD_DECLARATION), createHighlightedRange(24, 14, 12, SemanticHighlightings.METHOD) },
 		};
 	}
 

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/preferences/ColorSettingPreviewCode.txt

@@ -9,6 +9,7 @@ int static myfunc(uint parameter) {
   cout << "hello\n";
   return parameter - 1;
 }
+void mutator(int&);
 class MyClass {
 public:
   enum Number { ZERO, ONE, TWO };
@@ -21,6 +22,7 @@ label: myfunc(local);
     vmethod();
     staticMethod();
     problem();
+    mutator(local);
   }
   static void staticMethod();
 };