Bug 573764 - Ambiguous conversion on numeric type

Starting with C++11, a null pointer constant has to be specifically an
integer literal of 0 and not any constant expression. Before this
change, an expression like (0 & 1) would wrongly being considered a null
pointer constant. It also means it could implicitly convert to a pointer
type (like int *) and lead to problems during function resolution, like
ambiguity.

This change corrects the behavior for C++11 by tracking whether the
integer type (basic type) came from a literal expression so that we can
add this additional constraint when checking for a null pointer
constant. Because types are sometimes returned directly when evaluating
different kinds of non-literal expressions that contain literal
expressions, we have to be careful that we remove the flag that tracks
"from literal expression".

Unfortunately, the semantic code does not track the active C++ version
which means the behavior for pre-C++11 will be impacted. Tracking the
active C++ version would not be trivial and at least the new behavior is
more future-proof.

Change-Id: Ied625e96e70390872e36ab5bb4dc238d75809d2e
Signed-off-by: Marc-Andre Laperle <malaperle@gmail.com>

core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2CPPTests.java

@@ -13658,4 +13658,69 @@ public class AST2CPPTests extends AST2CPPTestBase {
 		parseAndCheckBindings(getAboveComment(), CPP, true);
 	}
 
+	//	void function(int * a) { }
+	//	void function(unsigned int b) { }
+	//
+	//	void functionPtr(decltype(nullptr) c) { }
+	//
+	//	int main()
+	//	{
+	//		function(0); // 0
+	//		function(nullptr); // 1
+	//		functionPtr(0); // 2
+	//		functionPtr(nullptr); // 3
+	//
+	//		function(0 & 1); // 4
+	//		functionPtr(0 & 1); // 5
+	//
+	//		function(0 << 1); // 6
+	//		functionPtr(0 << 1); // 7
+	//
+	//		function((0,0)); // 8
+	//		functionPtr((0,0)); // 9
+	//
+	//		function(int{0}); // 10
+	//		functionPtr(int{0}); // 11
+	//
+	//		function(true ? 0 : 0); // 12
+	//		functionPtr(true ? 0 : 0); // 13
+	//	}
+	public void testNullPointerConstantConversion_573764() throws Exception {
+		IASTTranslationUnit tu = parse(getAboveComment(), CPP);
+		NameCollector collector = new NameCollector();
+		tu.accept(collector);
+
+		IBinding funcDeclA = collector.getName(0).resolveBinding();
+		IBinding funcDeclB = collector.getName(2).resolveBinding();
+		IBinding funcDeclC = collector.getName(4).resolveBinding();
+
+		int callIndexStart = 7;
+		// Ambiguous
+		assertTrue(collector.getName(callIndexStart + 0).resolveBinding() instanceof IProblemBinding);
+		assertEquals(funcDeclA, collector.getName(callIndexStart + 1).resolveBinding());
+		assertEquals(funcDeclC, collector.getName(callIndexStart + 2).resolveBinding());
+		assertEquals(funcDeclC, collector.getName(callIndexStart + 3).resolveBinding());
+
+		assertEquals(funcDeclB, collector.getName(callIndexStart + 4).resolveBinding());
+		// Invalid argument (not null pointer constant)
+		assertTrue(collector.getName(callIndexStart + 5).resolveBinding() instanceof IProblemBinding);
+
+		assertEquals(funcDeclB, collector.getName(callIndexStart + 6).resolveBinding());
+		// Invalid argument (not null pointer constant)
+		assertTrue(collector.getName(callIndexStart + 7).resolveBinding() instanceof IProblemBinding);
+
+		// EvalComma
+		assertEquals(funcDeclB, collector.getName(callIndexStart + 8).resolveBinding());
+		// Invalid argument (not null pointer constant)
+		assertTrue(collector.getName(callIndexStart + 9).resolveBinding() instanceof IProblemBinding);
+
+		assertEquals(funcDeclB, collector.getName(callIndexStart + 10).resolveBinding());
+		// Invalid argument (not null pointer constant)
+		assertTrue(collector.getName(callIndexStart + 11).resolveBinding() instanceof IProblemBinding);
+
+		// EvalConditional
+		assertEquals(funcDeclB, collector.getName(callIndexStart + 12).resolveBinding());
+		// Invalid argument (not null pointer constant)
+		assertTrue(collector.getName(callIndexStart + 13).resolveBinding() instanceof IProblemBinding);
+	}
 }

core/org.eclipse.cdt.core/META-INF/MANIFEST.MF

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.cdt.core; singleton:=true
-Bundle-Version: 7.2.100.qualifier
+Bundle-Version: 7.2.200.qualifier
 Bundle-Activator: org.eclipse.cdt.core.CCorePlugin
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPBasicType.java

@@ -17,6 +17,9 @@ package org.eclipse.cdt.internal.core.dom.parser.cpp;
 
 import static org.eclipse.cdt.core.dom.ast.cpp.ICPPParameter.EMPTY_CPPPARAMETER_ARRAY;
 
+import java.text.MessageFormat;
+
+import org.eclipse.cdt.core.CCorePlugin;
 import org.eclipse.cdt.core.dom.ast.ASTTypeUtil;
 import org.eclipse.cdt.core.dom.ast.IASTExpression;
 import org.eclipse.cdt.core.dom.ast.IASTLiteralExpression;
@@ -58,10 +61,16 @@ public class CPPBasicType implements ICPPBasicType, ISerializableType {
 	public static final CPPBasicType CHAR = new CPPBasicType(Kind.eChar, 0);
 	public static final CPPBasicType VOID = new CPPBasicType(Kind.eVoid, 0);
 
-	private static final int FROM_STRING_LITERAL = 1 << 31;
+	public static final int FROM_LITERAL = 1 << 30;
+	public static final int FROM_STRING_LITERAL = 1 << 31;
+
+	private static final short TYPE_BUFFER_KIND_OFFSET = ITypeMarshalBuffer.FIRST_FLAG;
+	private static final short TYPE_BUFFER_FROM_LITERAL_FLAG = ITypeMarshalBuffer.SECOND_LAST_FLAG / 2;
+	private static final short TYPE_BUFFER_FIRST_FLAG_AFTER_KIND = TYPE_BUFFER_FROM_LITERAL_FLAG;
+	private static final int MAX_KIND_INT_VALUE = (TYPE_BUFFER_FIRST_FLAG_AFTER_KIND - 1) / TYPE_BUFFER_KIND_OFFSET;
 
 	private final Kind fKind;
-	private final int fModifiers;
+	private int fModifiers;
 	private Long fAssociatedValue;
 	private ICPPFunction fPseudoDestructor;
 
@@ -77,10 +86,12 @@ public class CPPBasicType implements ICPPBasicType, ISerializableType {
 		} else {
 			fKind = kind;
 		}
-		if (expression instanceof IASTLiteralExpression
+		if (expression instanceof IASTLiteralExpression) {
-				&& ((IASTLiteralExpression) expression).getKind() == IASTLiteralExpression.lk_string_literal) {
+			qualifiers |= FROM_LITERAL;
+			if (((IASTLiteralExpression) expression).getKind() == IASTLiteralExpression.lk_string_literal) {
 				qualifiers |= FROM_STRING_LITERAL;
 			}
+		}
 		fModifiers = qualifiers;
 		if (expression instanceof ICPPASTInitializerClause) {
 			Number num = ValueFactory.create(expression).numberValue();
@@ -209,9 +220,19 @@ public class CPPBasicType implements ICPPBasicType, ISerializableType {
 
 	@Override
 	public CPPBasicType clone() {
+		return clone(~0);
+	}
+
+	/**
+	 * Clone as normal but keep only requested flags.
+	 *
+	 * @param flagsMask The mask of flags to preserve during the clone.
+	 */
+	public CPPBasicType clone(int flagsMask) {
 		CPPBasicType t = null;
 		try {
 			t = (CPPBasicType) super.clone();
+			t.fModifiers &= flagsMask;
 		} catch (CloneNotSupportedException e) {
 			// Not going to happen.
 		}
@@ -235,15 +256,22 @@ public class CPPBasicType implements ICPPBasicType, ISerializableType {
 	}
 
 	/**
-	 * Returns {@code true} if the type was created for a string literal.
+	 * Returns {@code true} if the type was created from a string literal.
 	 */
 	public final boolean isFromStringLiteral() {
 		return (fModifiers & FROM_STRING_LITERAL) != 0;
 	}
 
+	/**
+	 * Returns {@code true} if the type was created from a literal.
+	 */
+	public final boolean isFromLiteral() {
+		return (fModifiers & FROM_LITERAL) != 0;
+	}
+
 	@Override
 	public final int getModifiers() {
-		return fModifiers & ~FROM_STRING_LITERAL;
+		return fModifiers & ~FROM_STRING_LITERAL & ~FROM_LITERAL;
 	}
 
 	@Override
@@ -254,27 +282,47 @@ public class CPPBasicType implements ICPPBasicType, ISerializableType {
 	@Override
 	public void marshal(ITypeMarshalBuffer buffer) throws CoreException {
 		final int kind = getKind().ordinal();
-		final int shiftedKind = kind * ITypeMarshalBuffer.FIRST_FLAG;
+		// 'kind' uses the space of the first few flags so make sure it doesn't overflow to the actual used flags further.
+		if (kind > MAX_KIND_INT_VALUE) {
+			throw new CoreException(CCorePlugin.createStatus(
+					MessageFormat.format("Cannot marshal a basic type, kind ''{0}'' would overflow following flags.", //$NON-NLS-1$
+							getKind().toString())));
+		}
+		final int shiftedKind = kind * TYPE_BUFFER_KIND_OFFSET;
 		final int modifiers = getModifiers();
 		short firstBytes = (short) (ITypeMarshalBuffer.BASIC_TYPE | shiftedKind);
-		if (modifiers != 0)
+		if (isFromLiteral())
-			firstBytes |= ITypeMarshalBuffer.LAST_FLAG;
+			firstBytes = setFirstBytesFlag(firstBytes, TYPE_BUFFER_FROM_LITERAL_FLAG);
 		if (fAssociatedValue != null)
-			firstBytes |= ITypeMarshalBuffer.SECOND_LAST_FLAG;
+			firstBytes = setFirstBytesFlag(firstBytes, ITypeMarshalBuffer.SECOND_LAST_FLAG);
+		if (modifiers != 0)
+			firstBytes = setFirstBytesFlag(firstBytes, ITypeMarshalBuffer.LAST_FLAG);
 		buffer.putShort(firstBytes);
 		if (modifiers != 0)
 			buffer.putByte((byte) modifiers);
 		if (fAssociatedValue != null)
 			buffer.putLong(getAssociatedNumericalValue());
+
+	}
+
+	private static short setFirstBytesFlag(short firstBytes, short flag) throws CoreException {
+		if (flag < TYPE_BUFFER_FIRST_FLAG_AFTER_KIND) {
+			throw new CoreException(CCorePlugin.createStatus(
+					MessageFormat.format("Cannot marshal a basic type, flag ''0x{0}'' overlaps ''kind'' bytes.", //$NON-NLS-1$
+							Integer.toHexString(flag))));
+		}
+		return (short) (firstBytes | flag);
 	}
 
 	public static IType unmarshal(short firstBytes, ITypeMarshalBuffer buffer) throws CoreException {
 		final boolean haveModifiers = (firstBytes & ITypeMarshalBuffer.LAST_FLAG) != 0;
 		final boolean haveAssociatedNumericalValue = (firstBytes & ITypeMarshalBuffer.SECOND_LAST_FLAG) != 0;
 		int modifiers = 0;
-		int kind = (firstBytes & (ITypeMarshalBuffer.SECOND_LAST_FLAG - 1)) / ITypeMarshalBuffer.FIRST_FLAG;
+		int kind = (firstBytes & (TYPE_BUFFER_FIRST_FLAG_AFTER_KIND - 1)) / TYPE_BUFFER_KIND_OFFSET;
 		if (haveModifiers)
 			modifiers = buffer.getByte();
+		if ((firstBytes & TYPE_BUFFER_FROM_LITERAL_FLAG) != 0)
+			modifiers |= FROM_LITERAL;
 		CPPBasicType result = new CPPBasicType(Kind.values()[kind], modifiers);
 		if (haveAssociatedNumericalValue)
 			result.setAssociatedNumericalValue(buffer.getLong());

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/Conversions.java

@@ -1168,11 +1168,15 @@ public class Conversions {
 			if (basicType.getKind() == Kind.eNullPtr)
 				return true;
 
+			// Starting from C++11, the value is required to be a literal. This means pre-C++11 parsing will not be correct here.
+			// But we don't currently have a way to check the C++ version here and in semantics code in general.
+			if (basicType.getKind() == Kind.eInt && basicType.isFromLiteral()) {
 				Long val = basicType.getAssociatedNumericalValue();
 				if (val != null && val == 0) {
 					return true;
 				}
 			}
+		}
 		return false;
 	}
 

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/EvalBinary.java

@@ -151,6 +151,11 @@ public class EvalBinary extends CPPDependentEvaluation {
 				}
 			}
 		}
+
+		if (fType instanceof CPPBasicType) {
+			fType = ((CPPBasicType) fType).clone(~CPPBasicType.FROM_LITERAL);
+		}
+
 		return fType;
 	}
 

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/EvalComma.java

@@ -28,6 +28,7 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPTemplateParameterMap;
 import org.eclipse.cdt.internal.core.dom.parser.DependentValue;
 import org.eclipse.cdt.internal.core.dom.parser.ITypeMarshalBuffer;
 import org.eclipse.cdt.internal.core.dom.parser.IntegralValue;
+import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPBasicType;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPEvaluation;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.InstantiationContext;
 import org.eclipse.core.runtime.CoreException;
@@ -145,6 +146,9 @@ public class EvalComma extends CPPDependentEvaluation {
 	public IType getType() {
 		if (fType == null) {
 			fType = computeType();
+			if (fType instanceof CPPBasicType) {
+				fType = ((CPPBasicType) fType).clone(~CPPBasicType.FROM_LITERAL);
+			}
 		}
 		return fType;
 	}

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/EvalConditional.java

@@ -39,6 +39,7 @@ import org.eclipse.cdt.internal.core.dom.parser.ITypeMarshalBuffer;
 import org.eclipse.cdt.internal.core.dom.parser.IntegralValue;
 import org.eclipse.cdt.internal.core.dom.parser.ProblemType;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPArithmeticConversion;
+import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPBasicType;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPReferenceType;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPEvaluation;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPUnknownType;
@@ -180,6 +181,14 @@ public class EvalConditional extends CPPDependentEvaluation {
 		if (fValueCategory != null)
 			return;
 
+		evaluateInternal();
+
+		if (fType instanceof CPPBasicType) {
+			fType = ((CPPBasicType) fType).clone(~CPPBasicType.FROM_LITERAL);
+		}
+	}
+
+	private void evaluateInternal() {
 		fValueCategory = PRVALUE;
 
 		final ICPPEvaluation positive = fPositive == null ? fCondition : fPositive;

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/PDOM.java

@@ -308,10 +308,13 @@ public class PDOM extends PlatformObject implements IPDOM {
 	 *  217.0 - Added nodiscard class/struct information, bug 534420
 	 *  218.0 - Added nodiscard enums information, bug 534420
 	 *  219.0 - Fix enums nodiscard information in the index from 8 byte to 1 byte, bug 534420
+	 *
+	 *  CDT 10.4 development
+	 *  220.0 - Changed marshalling of CPPBasicType to store new "from literal" flag, bug 573764
 	 */
-	private static final int MIN_SUPPORTED_VERSION = version(219, 0);
+	private static final int MIN_SUPPORTED_VERSION = version(220, 0);
-	private static final int MAX_SUPPORTED_VERSION = version(219, Short.MAX_VALUE);
+	private static final int MAX_SUPPORTED_VERSION = version(220, Short.MAX_VALUE);
-	private static final int DEFAULT_VERSION = version(219, 0);
+	private static final int DEFAULT_VERSION = version(220, 0);
 
 	private static int version(int major, int minor) {
 		return (major << 16) + minor;