Fixed devirtualized methods with extension overrides

Author: bfiete

Build_BeefySysLib.bat

@@ -11,9 +11,11 @@ CALL bin\msbuild.bat BeefySysLib\BeefySysLib.vcxproj /p:Configuration=Release /p
 :SUCCESS
 @ECHO SUCCESS!
 @POPD
+pause
 @EXIT /b 0
 
 :HADERROR
 @ECHO =================FAILED=================
 @POPD
-@EXIT /b %ERRORLEVEL%
+pause
+@EXIT /b %ERRORLEVEL%

IDE/Tests/Test1/scripts/HotSwap_ExtensionOverride.txt

@@ -0,0 +1,25 @@
+# Tests:
+#  Adding a type extension that overrides a base virtual method during a hot compile
+#  Devirtualized 'base' calls picking up an override introduced by that extension
+
+ShowFile("src/HotSwap_ExtensionOverride.bf")
+GotoText("//HotSwap_ExtensionOverride_Start")
+ToggleBreakpoint()
+RunWithCompiling()
+
+# Before the extension exists
+StepOver()
+StepOver()
+AssertEvalEquals("a", "10")
+AssertEvalEquals("b", "11")
+
+# Hot compile, adding an extension that overrides ClassA.MethodA
+ToggleCommentAt("ExtClassA_MethodA")
+Compile()
+
+# The virtual call picks up the override, and so does the 'base' call inside ClassB.MethodA
+Continue()
+StepOver()
+StepOver()
+AssertEvalEquals("a", "100")
+AssertEvalEquals("b", "101")

IDE/Tests/Test1/src/HotSwap_ExtensionOverride.bf

@@ -0,0 +1,51 @@
+#pragma warning disable 168
+
+namespace IDETest
+{
+	class HotSwap_ExtensionOverride
+	{
+		public class ClassA
+		{
+			public virtual int MethodA()
+			{
+				return 10;
+			}
+		}
+
+		public class ClassB : ClassA
+		{
+			public override int MethodA()
+			{
+				// This is a devirtualized 'base' call. Once the extension below adds an override for
+				//  ClassA.MethodA it has to start resolving to that override instead.
+				return base.MethodA() + 1;
+			}
+		}
+
+		/*ExtClassA_MethodA
+		public extension ClassA
+		{
+			public override int MethodA()
+			{
+				return 100;
+			}
+		}
+		*/
+
+		static void DoTest()
+		{
+			ClassA ca = scope ClassA();
+			ClassB cb = scope ClassB();
+
+			//HotSwap_ExtensionOverride_Start
+			int a = ca.MethodA();
+			int b = cb.MethodA();
+		}
+
+		public static void Test()
+		{
+			DoTest();
+			DoTest();
+		}
+	}
+}

IDE/Tests/Test1/src/Program.bf

@@ -13,6 +13,7 @@ namespace IDETest
 			HotTester.Test();
 			HotSwap_BaseChange.Test();
 			HotSwap_Data.Test();
+			HotSwap_ExtensionOverride.Test();
 			HotSwap_GetUnusued.Test();
 			HotSwap_Interfaces2.Test();
 			HotSwap_Lambdas01.Test();

IDEHelper/Compiler/BfCompiler.cpp

@@ -1554,6 +1554,18 @@ void BfCompiler::CreateVData(BfVDataModule* bfModule)
 		}
 	}
 
+	// Devirtualized calls (ie: 'base.Method()') route through a thunk when a type extension may
+	//  supply a different override - each executable's vdata resolves that thunk for itself.
+	for (auto type : vdataTypeList)
+	{
+		if ((!type->IsReified()) || (type->IsUnspecializedType()) || (type->IsTypeAlias()))
+			continue;
+		auto typeInst = type->ToTypeInstance();
+		if (typeInst == NULL)
+			continue;
+		bfModule->CreateDevirtThunks(typeInst);
+	}
+
 	for (int typeId = 0; typeId < (int)typeDataVector.size(); typeId++)
 	{
 		if (!typeDataVector[typeId])
@@ -6054,7 +6066,8 @@ void BfCompiler::PopulateReified()
 										{
 											if ((methodDef->mIsOverride) && (methodDef->mParams.mSize == declaringMethod->mMethodDef->mParams.mSize))
 											{
-												auto implMethod = typeInst->mModule->GetRawMethodInstance(typeInst, methodDef);
+												// 'methodDef' is from checkTypeInst's method set, so mIdx indexes that type's methods
+												auto implMethod = typeInst->mModule->GetRawMethodInstance(checkTypeInst, methodDef);
 												if ((implMethod != NULL) && (typeInst->mModule->CompareMethodSignatures(declaringMethod, implMethod)))
 												{
 													if ((implMethod != NULL) && ((!implMethod->mMethodInstanceGroup->IsImplemented()) || (!implMethod->mIsReified)))
@@ -7437,11 +7450,7 @@ bool BfCompiler::DoCompile(const StringImpl& outputDirectory)
 				mPassInstance->Fail(StrFormat("Project '%s' must reference core library '%s'", bfProject->mName.c_str(), mBfObjectTypeDef->mProject->mName.c_str()));
 			}
 
-			if ((bfProject->mTargetType != BfTargetType_BeefConsoleApplication) && (bfProject->mTargetType != BfTargetType_BeefWindowsApplication) &&
-				(bfProject->mTargetType != BfTargetType_BeefLib_DynamicLib) && (bfProject->mTargetType != BfTargetType_BeefLib_StaticLib) &&
-				(bfProject->mTargetType != BfTargetType_C_ConsoleApplication) && (bfProject->mTargetType != BfTargetType_C_WindowsApplication) &&
-				(bfProject->mTargetType != BfTargetType_BeefTest) &&
-				(bfProject->mTargetType != BfTargetType_BeefApplication_StaticLib) && (bfProject->mTargetType != BfTargetType_BeefApplication_DynamicLib))
+			if (!bfProject->IsFinalProgramTarget())
 				continue;
 
 			if (bfProject->mTargetType == BfTargetType_BeefTest)

IDEHelper/Compiler/BfExprEvaluator.cpp

@@ -6660,6 +6660,28 @@ BfTypedValue BfExprEvaluator::CreateCall(BfAstNode* targetSrc, BfMethodInstance*
 	auto methodDef = methodInstance->mMethodDef;
 	BfIRValue funcCallInst = func;
 
+	if ((bypassVirtual) && (funcCallInst) && (!isDelegateThunk) && (origTargetType != NULL) && (mFunctionBindResult == NULL))
+	{
+		// We resolved this against only the extensions our own project can see, but this module is
+		//  shared by every executable that links it - route through a per-executable thunk instead.
+		auto origTargetTypeInst = origTargetType->ToTypeInstance();
+		auto thunkFunc = mModule->TryGetDevirtThunk(origTargetTypeInst, methodInstance);
+		if (thunkFunc)
+		{
+			// Thunk is typed from the declaring method, which may be owned further up the chain
+			auto& declaringMethodRef = origTargetTypeInst->mVirtualMethodTable[methodInstance->mVirtualTableIdx].mDeclaringMethod;
+			BfMethodInstance* declaringMethodInstance = (BfMethodInstance*)declaringMethodRef;
+			if ((!methodDef->mIsStatic) && (!irArgs.IsEmpty()) && (declaringMethodInstance->GetOwner() != methodInstance->GetOwner()))
+			{
+				// GetStructRetIdx is the sret param index, so 'this' shifts only when sret comes first
+				int thisIdx = (methodInstance->GetStructRetIdx() == 0) ? 1 : 0;
+				if (thisIdx < (int)irArgs.size())
+					irArgs[thisIdx] = mModule->mBfIRBuilder->CreateBitCast(irArgs[thisIdx], mModule->mBfIRBuilder->MapTypeInstPtr(declaringMethodInstance->GetOwner()));
+			}
+			funcCallInst = thunkFunc;
+		}
+	}
+
 	auto importCallKind = methodInstance->GetImportCallKind();
 	if ((funcCallInst) && (importCallKind != BfImportCallKind_None))
 	{