Preserve class attribute identity and validate inheritance conflicts across C3 MRO

engine/oop.c

@@ -163,32 +163,67 @@ PH7_PRIVATE ph7_class_method *PH7_ClassExtractMethod(ph7_class *pClass, const ch
 	return (ph7_class_method *)pEntry->pUserData;
 }
 /*
- * Check if the given name is a class attribute.
+ * Check if the given name is an attribute declared directly by the given class.
+ * Names are compared exactly (length and bytes), inherited copies are ignored.
  * Return the desired attribute [i.e: ph7_class_attr instance] on success.NULL otherwise.
  */
 PH7_PRIVATE ph7_class_attr *PH7_ClassExtractAttribute(ph7_class *pClass, const char *zName, sxu32 nByte) {
 	SyHashEntry *pEntry;
 	ph7_class_attr *pAttr;
+	SyString sName;
+	/* Attribute names match exactly, including their length */
+	SyStringInitFromBuf(&sName, zName, nByte);
 	/* Perform a hash lookup */
 	SyHashResetLoopCursor(&pClass->hAttr);
 	while((pEntry = SyHashGetNextEntry(&pClass->hAttr)) != 0) {
 		/* Point to the desired method */
 		pAttr = (ph7_class_attr *)pEntry->pUserData;
-		if(pAttr->pClass == pClass && SyStrncmp(pAttr->sName.zString, zName, nByte) == 0) {
+		if(pAttr->pClass == pClass && SyStringCmp(&pAttr->sName, &sName, SyMemcmp) == 0) {
 			return pAttr;
 		}
 	}
 	/* No such entry */
 	return 0;
 }
+/*
+ * Format an attribute reference for diagnostics. Instance fields use
+ * 'Class->name', static fields 'Class::$name' and constants 'Class::NAME'.
+ */
+PH7_PRIVATE void PH7_ClassAttrDiagnosticName(ph7_class *pClass, sxi32 iFlags, const SyString *pName,
+		char *zBuf, sxu32 nBuf, SyString *pOut) {
+	const char *zSeparator;
+	if(iFlags & PH7_CLASS_ATTR_CONSTANT) {
+		zSeparator = "::";
+	} else if(iFlags & PH7_CLASS_ATTR_STATIC) {
+		zSeparator = "::$";
+	} else {
+		zSeparator = "->";
+	}
+	nBuf = SyBufferFormat(zBuf, nBuf, "%z%s%z", &pClass->sName, zSeparator, pName);
+	SyStringInitFromBuf(pOut, zBuf, nBuf);
+}
 /*
  * Install a class attribute in the corresponding container.
  * Return SXRET_OK on success. Any other return value indicates failure.
+ * All class attributes (instance fields, static fields and constants) share
+ * a single name space. Duplicates inside one class are rejected at compile
+ * time; conflicts across the inheritance chain are checked at class init.
  */
-PH7_PRIVATE sxi32 PH7_ClassInstallAttr(ph7_class *pClass, ph7_class_attr *pAttr) {
+PH7_PRIVATE sxi32 PH7_ClassInstallAttr(ph7_gen_state *pGen, ph7_class *pClass, ph7_class_attr *pAttr) {
+	SyHashEntry *pEntry;
 	SyString *pName = &pAttr->sName;
 	sxi32 rc;
 	pAttr->pClass = pClass;
+	SyHashResetLoopCursor(&pClass->hAttr);
+	while((pEntry = SyHashGetNextEntry(&pClass->hAttr)) != 0) {
+		ph7_class_attr *pOther = (ph7_class_attr *)pEntry->pUserData;
+		if(pOther->pClass == pClass && SyStringCmp(&pOther->sName, pName, SyMemcmp) == 0) {
+			char zAttr[256];
+			SyString sAttr;
+			PH7_ClassAttrDiagnosticName(pClass, pAttr->iFlags, &pAttr->sName, zAttr, sizeof(zAttr), &sAttr);
+			PH7_GenCompileError(pGen, E_ERROR, pAttr->nLine, "Duplicate class attribute '%z' in class '%z'", &sAttr, &pClass->sName);
+		}
+	}
 	rc = SyHashInsert(&pClass->hAttr, (const void *)pName->zString, pName->nByte, pAttr);
 	return rc;
 }
@@ -203,6 +238,71 @@ PH7_PRIVATE sxi32 PH7_ClassInstallMethod(ph7_class *pClass, ph7_class_method *pM
 	rc = SyHashInsert(&pClass->hMethod, (const void *)pName->zString, pName->nByte, pMeth);
 	return rc;
 }
+/*
+ * Return TRUE if the first class is a descendant of the second one according
+ * to its already built C3 linearization.
+ */
+static sxi32 ClassIsDescendant(ph7_class *pClass, ph7_class *pAncestor) {
+	sxu32 i;
+	for(i = 1; i < SySetUsed(&pClass->aMro); i++) {
+		if(*(ph7_class **)SySetAt(&pClass->aMro, i) == pAncestor) {
+			return TRUE;
+		}
+	}
+	return FALSE;
+}
+/*
+ * Validate attribute declarations across the C3 linearization. Two visible
+ * declarations with the same name conflict even when they are identical.
+ * Private declarations of different classes are independent, even along an
+ * inheritance chain, but a private declaration cannot narrow a visible member
+ * inherited from a base.
+ */
+static sxi32 ClassValidateAttributes(ph7_vm *pVm, ph7_class *pClass) {
+	SyHashEntry *pEntry, *pOtherEntry;
+	sxu32 i, j;
+	for(i = 0; i < SySetUsed(&pClass->aMro); i++) {
+		ph7_class *pOwner = *(ph7_class **)SySetAt(&pClass->aMro, i);
+		SyHashResetLoopCursor(&pOwner->hAttr);
+		while((pEntry = SyHashGetNextEntry(&pOwner->hAttr)) != 0) {
+			ph7_class_attr *pAttr = (ph7_class_attr *)pEntry->pUserData;
+			if(pAttr->pClass != pOwner) {
+				/* Inherited copy, not a declaration of this class */
+				continue;
+			}
+			for(j = i + 1; j < SySetUsed(&pClass->aMro); j++) {
+				ph7_class *pOtherOwner = *(ph7_class **)SySetAt(&pClass->aMro, j);
+				SyHashResetLoopCursor(&pOtherOwner->hAttr);
+				while((pOtherEntry = SyHashGetNextEntry(&pOtherOwner->hAttr)) != 0) {
+					ph7_class_attr *pOther = (ph7_class_attr *)pOtherEntry->pUserData;
+					if(pOther->pClass != pOtherOwner || SyStringCmp(&pAttr->sName, &pOther->sName, SyMemcmp) != 0) {
+						continue;
+					}
+					if(pAttr->iProtection == PH7_CLASS_PROT_PRIVATE && pOther->iProtection == PH7_CLASS_PROT_PRIVATE) {
+						/* Private declarations of different classes are independent */
+						continue;
+					}
+					if(pAttr->iProtection == PH7_CLASS_PROT_PRIVATE) {
+						if(!ClassIsDescendant(pOwner, pOtherOwner)) {
+							/* A private member of an unrelated base cannot hide a visible one */
+							continue;
+						}
+					} else if(pOther->iProtection == PH7_CLASS_PROT_PRIVATE && !ClassIsDescendant(pOtherOwner, pOwner)) {
+						/* An inherited private member does not hide a new visible declaration */
+						continue;
+					}
+					char zAttr[256], zOther[256];
+					SyString sAttr, sOther;
+					PH7_ClassAttrDiagnosticName(pAttr->pClass, pAttr->iFlags, &pAttr->sName, zAttr, sizeof(zAttr), &sAttr);
+					PH7_ClassAttrDiagnosticName(pOther->pClass, pOther->iFlags, &pOther->sName, zOther, sizeof(zOther), &sOther);
+					PH7_VmThrowError(pVm, PH7_CTX_ERR, "Class attribute '%z' conflicts with '%z' in class '%z'",
+									 &sAttr, &sOther, &pClass->sName);
+				}
+			}
+		}
+	}
+	return SXRET_OK;
+}
 /*
  * Validate actual declarations, not inherited hash entries. Forward-declared
  * bases need not have executed CLASS_INIT, and virtual methods are not mounted
@@ -423,7 +523,13 @@ done:
 		SySetRelease(&sMro);
 		pClass->iMroState = 0;
 	}
-	return rc == SXRET_OK ? ClassValidateMethods(pVm, pClass) : rc;
+	if(rc == SXRET_OK) {
+		rc = ClassValidateAttributes(pVm, pClass);
+	}
+	if(rc == SXRET_OK) {
+		rc = ClassValidateMethods(pVm, pClass);
+	}
+	return rc;
 }
 /*
  * Perform an inheritance operation.
@@ -472,20 +578,31 @@ PH7_PRIVATE sxi32 PH7_ClassInherit(ph7_vm *pVm, ph7_class *pSub, ph7_class *pBas
 	SyHashEntry *pEntry;
 	SyString *pName;
 	sxi32 rc;
+	SXUNUSED(pVm);
 	/* Install in the derived hashtable */
 	rc = SyHashInsert(&pSub->hDerived, (const void *)SyStringData(&pBase->sName), SyStringLength(&pBase->sName), pBase);
 	if(rc != SXRET_OK) {
 		return rc;
 	}
-	/* Copy all attributes from the base class */
+	/* Copy all attributes from the base class. A diamond shares the same
+	 * declaration pointer, which must yield a single inherited entry. */
 	SyHashResetLoopCursor(&pBase->hAttr);
 	while((pEntry = SyHashGetNextEntry(&pBase->hAttr)) != 0) {
-		/* Check if attributes are not being redeclared in the subclass and emit WARNING */
+		sxi32 bInstalled;
+		SyHashEntry *pSubEntry;
 		pAttr = (ph7_class_attr *)pEntry->pUserData;
 		pName = &pAttr->sName;
-		if((pEntry = SyHashGet(&pSub->hAttr, (const void *)pName->zString, pName->nByte)) != 0) {
-			ph7_class_attr *pSubAttr = (ph7_class_attr *)pEntry->pUserData;
-			PH7_VmThrowError(pVm, PH7_CTX_WARNING, "Attribute '%z::$%z' hides inherited member '%z::$%z'", &pSubAttr->pClass->sName, pName, &pBase->sName, pName);
+		bInstalled = FALSE;
+		SyHashResetLoopCursor(&pSub->hAttr);
+		while((pSubEntry = SyHashGetNextEntry(&pSub->hAttr)) != 0) {
+			if(pSubEntry->pUserData == pAttr) {
+				bInstalled = TRUE;
+				break;
+			}
+		}
+		if(bInstalled) {
+			/* Already inherited through another base */
+			continue;
 		}
 		/* Install the attribute */
 		rc = SyHashInsert(&pSub->hAttr, (const void *)pName->zString, pName->nByte, pAttr);
@@ -586,25 +703,10 @@ PH7_PRIVATE sxi32 PH7_ClassInterfaceInherit(ph7_class *pSub, ph7_class *pBase) {
  */
 PH7_PRIVATE sxi32 PH7_ClassImplement(ph7_vm *pVm, ph7_class *pMain, ph7_class *pInterface) {
 	ph7_class_method *pMeth;
-	ph7_class_attr *pAttr;
 	SyHashEntry *pEntry;
 	SyString *pName;
-	sxi32 rc;
-	/* First off,copy all constants declared inside the interface */
-	SyHashResetLoopCursor(&pInterface->hAttr);
-	while((pEntry = SyHashGetNextEntry(&pInterface->hAttr)) != 0) {
-		/* Point to the constant declaration */
-		pAttr = (ph7_class_attr *)pEntry->pUserData;
-		pName = &pAttr->sName;
-		/* Make sure the attribute is not redeclared in the main class */
-		if(SyHashGet(&pMain->hAttr, pName->zString, pName->nByte) == 0) {
-			/* Install the attribute */
-			rc = SyHashInsert(&pMain->hAttr, pName->zString, pName->nByte, pAttr);
-			if(rc != SXRET_OK) {
-				return rc;
-			}
-		}
-	}
+	/* Interface constants are not inherited by the implementing class.
+	 * They remain reachable through the interface name only. */
 	SyHashResetLoopCursor(&pInterface->hMethod);
 	while((pEntry = SyHashGetNextEntry(&pInterface->hMethod)) != 0) {
 		pMeth = (ph7_class_method *)pEntry->pUserData;