From 177b5b72542eada876c4640d098124bd6a63514a Mon Sep 17 00:00:00 2001 From: Brian Fiete Date: Tue, 1 Sep 2020 15:57:08 -0700 Subject: [PATCH] Fixed lambda capture of shadows variables --- IDEHelper/Compiler/BfExprEvaluator.cpp | 185 +++++++-------------- IDEHelper/Compiler/BfExprEvaluator.h | 2 +- IDEHelper/Compiler/BfResolvedTypeUtils.cpp | 8 +- IDEHelper/Compiler/BfResolvedTypeUtils.h | 4 +- IDEHelper/Compiler/BfSourceClassifier.cpp | 3 + IDEHelper/Tests/src/Lambdas.bf | 19 +++ 6 files changed, 88 insertions(+), 133 deletions(-) diff --git a/IDEHelper/Compiler/BfExprEvaluator.cpp b/IDEHelper/Compiler/BfExprEvaluator.cpp index 752143b6..984d53d5 100644 --- a/IDEHelper/Compiler/BfExprEvaluator.cpp +++ b/IDEHelper/Compiler/BfExprEvaluator.cpp @@ -3323,15 +3323,6 @@ BfTypedValue BfExprEvaluator::LookupIdentifier(BfAstNode* refNode, const StringI int varSkipCount = 0; StringT<128> wantName; wantName.Reference(findName); - if (findName.StartsWith('@')) - { - wantName = findName; - while (wantName.StartsWith("@")) - { - varSkipCount++; - wantName.Remove(0); - } - } closureTypeInst->mTypeDef->PopulateMemberSets(); BfMemberSetEntry* memberSetEntry = NULL; @@ -9457,7 +9448,7 @@ bool BfExprEvaluator::CanBindDelegate(BfDelegateBindExpression* delegateBindExpr return IsExactMethodMatch(methodInstance, bindResult.mMethodInstance, true); } -BfTypedValue BfExprEvaluator::DoImplicitArgCapture(BfAstNode* refNode, BfIdentifierNode* identifierNode) +BfTypedValue BfExprEvaluator::DoImplicitArgCapture(BfAstNode* refNode, BfIdentifierNode* identifierNode, int shadowIdx) { String findName = identifierNode->ToString(); @@ -9468,6 +9459,8 @@ BfTypedValue BfExprEvaluator::DoImplicitArgCapture(BfAstNode* refNode, BfIdentif auto checkMethodState = mModule->mCurMethodState; bool isMixinOuterVariablePass = false; + int shadowSkip = shadowIdx; + while (checkMethodState != NULL) { BP_ZONE("LookupIdentifier:DoImplicitArgCapture"); @@ -9487,29 +9480,23 @@ BfTypedValue BfExprEvaluator::DoImplicitArgCapture(BfAstNode* refNode, BfIdentif while (varDecl != NULL) { -// if ((closureTypeInst != NULL) && (wantName == "this")) -// break; - if (varDecl->mNameNode == identifierNode) { - BfTypedValue localResult = LoadLocal(varDecl); -// auto autoComplete = GetAutoComplete(); -// if (identifierNode != NULL) -// { -// if (autoComplete != NULL) -// autoComplete->CheckLocalRef(identifierNode, varDecl); -// if (((mModule->mCurMethodState->mClosureState == NULL) || (mModule->mCurMethodState->mClosureState->mCapturing)) && -// (mModule->mCompiler->mResolvePassData != NULL) && (mModule->mCurMethodInstance != NULL)) -// mModule->mCompiler->mResolvePassData->HandleLocalReference(identifierNode, varDecl->mNameNode, mModule->mCurTypeInstance->mTypeDef, rootMethodState->mMethodInstance->mMethodDef, varDecl->mLocalVarId); -// } -// + if (shadowSkip > 0) + { + shadowSkip--; + } + else + { + BfTypedValue localResult = LoadLocal(varDecl); if (!isMixinOuterVariablePass) { mResultLocalVar = varDecl; mResultFieldInstance = NULL; mResultLocalVarRefNode = identifierNode; } - return localResult; + return localResult; + } } varDecl = varDecl->mShadowedLocal; @@ -9688,7 +9675,7 @@ BfTypedValue BfExprEvaluator::DoImplicitArgCapture(BfAstNode* refNode, BfMethodI if (identifierNode != NULL) { - lookupVal = DoImplicitArgCapture(refNode, identifierNode); + lookupVal = DoImplicitArgCapture(refNode, identifierNode, 0); } else { @@ -10940,11 +10927,45 @@ BfLambdaInstance* BfExprEvaluator::GetLambdaInstance(BfLambdaBindExpression* lam if (captureType == BfCaptureType_Reference) capturedEntry.mExplicitlyByReference = true; capturedEntry.mType = capturedType; + capturedEntry.mNameNode = outerLocal->mNameNode; if (outerLocal->mName == "this") capturedEntry.mName = "__this"; else + { capturedEntry.mName = outerLocal->mName; - capturedEntry.mNameNode = outerLocal->mNameNode; + + BfLocalVarEntry* entry = NULL; + if (varMethodState->mLocalVarSet.TryGetWith(capturedEntry.mName, &entry)) + { + auto startCheckVar = entry->mLocalVar; + + int shadowIdx = 0; + auto checkVar = startCheckVar; + while (checkVar != NULL) + { + if (checkVar == outerLocal) + { + // We only use mShadowIdx when we have duplicate name nodes (ie: in the case of for looks with iterator vs value) + auto shadowCheckVar = startCheckVar; + while (shadowCheckVar != checkVar) + { + if (shadowCheckVar->mNameNode == checkVar->mNameNode) + capturedEntry.mShadowIdx++; + + shadowCheckVar = shadowCheckVar->mShadowedLocal; + } + + for (int i = 0; i < shadowIdx; i++) + capturedEntry.mName.Insert(0, '@'); + + break; + } + + shadowIdx++; + checkVar = checkVar->mShadowedLocal; + } + } + } capturedEntries.Add(capturedEntry); } } @@ -11107,10 +11128,12 @@ BfLambdaInstance* BfExprEvaluator::GetLambdaInstance(BfLambdaBindExpression* lam SizedArray origParamTypes; BfIRType origReturnType; + bool forceStatic = false; if (invokeMethodInstance != NULL) { + forceStatic = methodDef->mIsStatic; auto invokeFunctionType = mModule->mBfIRBuilder->MapMethod(invokeMethodInstance); - invokeMethodInstance->GetIRFunctionInfo(mModule, origReturnType, origParamTypes, methodDef->mIsStatic); + invokeMethodInstance->GetIRFunctionInfo(mModule, origReturnType, origParamTypes, forceStatic); } else { @@ -11119,15 +11142,15 @@ BfLambdaInstance* BfExprEvaluator::GetLambdaInstance(BfLambdaBindExpression* lam SizedArray newTypes; - if ((invokeMethodInstance != NULL) && (invokeMethodInstance->GetStructRetIdx() == 0)) + if ((invokeMethodInstance != NULL) && (invokeMethodInstance->GetStructRetIdx(forceStatic) == 0)) newTypes.push_back(origParamTypes[0]); if (!methodDef->mIsStatic) newTypes.push_back(mModule->mBfIRBuilder->MapType(useTypeInstance)); - if ((invokeMethodInstance != NULL) && (invokeMethodInstance->GetStructRetIdx() == 1)) + if ((invokeMethodInstance != NULL) && (invokeMethodInstance->GetStructRetIdx(forceStatic) == 1)) newTypes.push_back(origParamTypes[1]); int paramStartIdx = 0; - if ((invokeMethodInstance != NULL) && (invokeMethodInstance->GetStructRetIdx() != -1)) + if ((invokeMethodInstance != NULL) && (invokeMethodInstance->GetStructRetIdx(forceStatic) != -1)) paramStartIdx++; if (!methodDef->mIsStatic) paramStartIdx++; @@ -11325,9 +11348,6 @@ BfLambdaInstance* BfExprEvaluator::GetLambdaInstance(BfLambdaBindExpression* lam lambdaCapture.mName = "this"; else lambdaCapture.mName = capturedEntry.mName; -// if (capturedEntry.mLocalVarDef != NULL) -// lambdaCapture.mLocalIdx = capturedEntry.mLocalVarDef->mLocalVarId; -// lambdaCapture.mCopyField = capturedEntry.mCopyField; lambdaInstance->mCaptures.Add(lambdaCapture); closureInstanceInfo->mCaptureEntries.Add(capturedEntry); @@ -11424,107 +11444,18 @@ void BfExprEvaluator::Visit(BfLambdaBindExpression* lambdaBindExpr) } } -// for (auto& capturedEntry : capturedEntries) -// { -// BfIRValue capturedValue; -// if (capturedEntry.mLocalVarDef != NULL) -// { -// BfTypedValue localResult = LoadLocal(capturedEntry.mLocalVarDef, true); -// -// if (!capturedEntry.mType->IsRef()) -// { -// if (localResult.IsSplat()) -// { -// auto aggResult = mModule->AggregateSplat(localResult); -// capturedValue = aggResult.mValue; -// } -// else -// { -// localResult = mModule->LoadValue(localResult); -// capturedValue = localResult.mValue; -// if ((capturedEntry.mLocalVarDef->mResolvedType->IsRef()) && (!capturedEntry.mType->IsRef())) -// capturedValue = mModule->mBfIRBuilder->CreateLoad(capturedValue); -// } -// } -// else -// { -// if (capturedEntry.mLocalVarDef->mResolvedType->IsRef()) -// localResult = mModule->LoadValue(localResult); -// capturedValue = localResult.mValue; -// } -// } -// else -// { -// // Read captured field from outer lambda ('this') -// auto localVar = mModule->mCurMethodState->mLocals[0]; -// capturedValue = mModule->mBfIRBuilder->CreateInBoundsGEP(localVar->mValue, 0, capturedEntry.mCopyField->mDataIdx); -// if ((capturedEntry.mCopyField->mResolvedType->IsRef()) || (!capturedEntry.mType->IsRef())) -// { -// capturedValue = mModule->mBfIRBuilder->CreateLoad(capturedValue); -// if ((capturedEntry.mCopyField->mResolvedType->IsRef()) && (!capturedEntry.mType->IsRef())) -// capturedValue = mModule->mBfIRBuilder->CreateLoad(capturedValue); -// } -// } -// if (capturedValue) -// { -// auto fieldPtr = mModule->mBfIRBuilder->CreateInBoundsGEP(mResult.mValue, 0, closureTypeInst->mFieldInstances[fieldIdx].mDataIdx); -// mModule->mBfIRBuilder->CreateStore(capturedValue, fieldPtr); -// } -// fieldIdx++; -// } - - int captureIdx = 0; - //for (auto& capturedEntry : lambdaInstance->mCaptures) + int captureIdx = 0; for (int captureIdx = 0; captureIdx < (int)lambdaInstance->mCaptures.size(); captureIdx++) { auto& capturedEntry = lambdaInstance->mCaptures[captureIdx]; - BfIdentifierNode* identifierNode = lambdaInstance->mMethodInstance->mMethodInfoEx->mClosureInstanceInfo->mCaptureEntries[captureIdx].mNameNode; - //if (captureIdx < lambdaInstance->mMethodInstance->mClosureInstanceInfo->mCaptureNodes.size) + auto& closureCaptureEntry = lambdaInstance->mMethodInstance->mMethodInfoEx->mClosureInstanceInfo->mCaptureEntries[captureIdx]; + BfIdentifierNode* identifierNode = closureCaptureEntry.mNameNode; BfIRValue capturedValue; -// if (capturedEntry.mLocalVarDef != NULL) -// { -// BfTypedValue localResult = LoadLocal(capturedEntry.mLocalVarDef, true); -// -// if (!capturedEntry.mType->IsRef()) -// { -// if (localResult.IsSplat()) -// { -// auto aggResult = mModule->AggregateSplat(localResult); -// capturedValue = aggResult.mValue; -// } -// else -// { -// localResult = mModule->LoadValue(localResult); -// capturedValue = localResult.mValue; -// if ((capturedEntry.mLocalVarDef->mResolvedType->IsRef()) && (!capturedEntry.mType->IsRef())) -// capturedValue = mModule->mBfIRBuilder->CreateLoad(capturedValue); -// } -// } -// else -// { -// if (capturedEntry.mLocalVarDef->mResolvedType->IsRef()) -// localResult = mModule->LoadValue(localResult); -// capturedValue = localResult.mValue; -// } -// } -// else -// { -// // Read captured field from outer lambda ('this') -// auto localVar = mModule->mCurMethodState->mLocals[0]; -// capturedValue = mModule->mBfIRBuilder->CreateInBoundsGEP(localVar->mValue, 0, capturedEntry.mCopyField->mDataIdx); -// if ((capturedEntry.mCopyField->mResolvedType->IsRef()) || (!capturedEntry.mType->IsRef())) -// { -// capturedValue = mModule->mBfIRBuilder->CreateLoad(capturedValue); -// if ((capturedEntry.mCopyField->mResolvedType->IsRef()) && (!capturedEntry.mType->IsRef())) -// capturedValue = mModule->mBfIRBuilder->CreateLoad(capturedValue); -// } -// } - auto fieldInstance = &closureTypeInst->mFieldInstances[fieldIdx]; BfTypedValue capturedTypedVal; if (identifierNode != NULL) - capturedTypedVal = DoImplicitArgCapture(NULL, identifierNode); + capturedTypedVal = DoImplicitArgCapture(NULL, identifierNode, closureCaptureEntry.mShadowIdx); else capturedTypedVal = LookupIdentifier(NULL, capturedEntry.mName); if (!fieldInstance->mResolvedType->IsRef()) diff --git a/IDEHelper/Compiler/BfExprEvaluator.h b/IDEHelper/Compiler/BfExprEvaluator.h index 29af3a4a..4fb73734 100644 --- a/IDEHelper/Compiler/BfExprEvaluator.h +++ b/IDEHelper/Compiler/BfExprEvaluator.h @@ -398,7 +398,7 @@ public: void CheckLocalMethods(BfAstNode* targetSrc, BfTypeInstance* typeInstance, const StringImpl& methodName, BfMethodMatcher& methodMatcher, BfMethodType methodType); void InjectMixin(BfAstNode* targetSrc, BfTypedValue target, bool allowImplicitThis, const StringImpl& name, const BfSizedArray& arguments, BfSizedArray* methodGenericArgs); void SetMethodElementType(BfAstNode* target); - BfTypedValue DoImplicitArgCapture(BfAstNode* refNode, BfIdentifierNode* identifierNode); + BfTypedValue DoImplicitArgCapture(BfAstNode* refNode, BfIdentifierNode* identifierNode, int shadowIdx); BfTypedValue DoImplicitArgCapture(BfAstNode* refNode, BfMethodInstance* methodInstance, int paramIdx, bool& failed, BfImplicitParamKind paramKind = BfImplicitParamKind_General, const BfTypedValue& methodRefTarget = BfTypedValue()); bool CanBindDelegate(BfDelegateBindExpression* delegateBindExpr, BfMethodInstance** boundMethod = NULL); bool IsExactMethodMatch(BfMethodInstance* methodA, BfMethodInstance* methodB, bool ignoreImplicitParams = false); diff --git a/IDEHelper/Compiler/BfResolvedTypeUtils.cpp b/IDEHelper/Compiler/BfResolvedTypeUtils.cpp index 370386e6..7361f738 100644 --- a/IDEHelper/Compiler/BfResolvedTypeUtils.cpp +++ b/IDEHelper/Compiler/BfResolvedTypeUtils.cpp @@ -662,7 +662,7 @@ bool BfMethodInstance::HasParamsArray() return GetParamKind((int)mParams.size() - 1) == BfParamKind_Params; } -int BfMethodInstance::GetStructRetIdx() +int BfMethodInstance::GetStructRetIdx(bool forceStatic) { if ((mReturnType->IsComposite()) && (!mReturnType->IsValuelessType()) && (!GetLoweredReturnType()) && (!mIsIntrinsic)) { @@ -674,7 +674,7 @@ int BfMethodInstance::GetStructRetIdx() if (owner->mModule->mCompiler->mOptions.mPlatformType != BfPlatformType_Windows) return 0; - if (!HasThis()) + if ((!HasThis()) || (forceStatic)) return 0; if (!owner->IsValueType()) return 1; @@ -1014,7 +1014,7 @@ void BfMethodInstance::GetIRFunctionInfo(BfModule* module, BfIRType& returnType, auto voidType = module->GetPrimitiveType(BfTypeCode_None); returnType = module->mBfIRBuilder->MapType(voidType); } - else if (GetStructRetIdx() != -1) + else if (GetStructRetIdx(forceStatic) != -1) { auto voidType = module->GetPrimitiveType(BfTypeCode_None); returnType = module->mBfIRBuilder->MapType(voidType); @@ -1161,7 +1161,7 @@ void BfMethodInstance::GetIRFunctionInfo(BfModule* module, BfIRType& returnType, _AddType(checkType2); } - if ((GetStructRetIdx() == 1) && (!forceStatic)) + if (GetStructRetIdx(forceStatic) == 1) { BF_SWAP(paramTypes[0], paramTypes[1]); } diff --git a/IDEHelper/Compiler/BfResolvedTypeUtils.h b/IDEHelper/Compiler/BfResolvedTypeUtils.h index 227c6013..563ba8ed 100644 --- a/IDEHelper/Compiler/BfResolvedTypeUtils.h +++ b/IDEHelper/Compiler/BfResolvedTypeUtils.h @@ -712,12 +712,14 @@ struct BfClosureCapturedEntry String mName; BfIdentifierNode* mNameNode; bool mExplicitlyByReference; + int mShadowIdx; // Only relative to matching nameNodes BfClosureCapturedEntry() { mNameNode = NULL; mType = NULL; mExplicitlyByReference = false; + mShadowIdx = 0; } bool operator<(const BfClosureCapturedEntry& other) const @@ -870,7 +872,7 @@ public: bool HasThis(); bool HasExplicitThis(); bool HasParamsArray(); - int GetStructRetIdx(); + int GetStructRetIdx(bool forceStatic = false); bool HasSelf(); bool GetLoweredReturnType(BfTypeCode* loweredTypeCode = NULL, BfTypeCode* loweredTypeCode2 = NULL); bool WantsStructsAttribByVal(); diff --git a/IDEHelper/Compiler/BfSourceClassifier.cpp b/IDEHelper/Compiler/BfSourceClassifier.cpp index d8ae2613..068ada0e 100644 --- a/IDEHelper/Compiler/BfSourceClassifier.cpp +++ b/IDEHelper/Compiler/BfSourceClassifier.cpp @@ -552,6 +552,9 @@ void BfSourceClassifier::Visit(BfPropertyDeclaration* propertyDeclaration) BfElementVisitor::Visit(propertyDeclaration); + if (auto expr = BfNodeDynCast(propertyDeclaration->mDefinitionBlock)) + return; + for (auto methodDeclaration : propertyDeclaration->mMethods) { if ((methodDeclaration != NULL) && (methodDeclaration->mNameNode != NULL)) diff --git a/IDEHelper/Tests/src/Lambdas.bf b/IDEHelper/Tests/src/Lambdas.bf index 78dd4a42..28ebd858 100644 --- a/IDEHelper/Tests/src/Lambdas.bf +++ b/IDEHelper/Tests/src/Lambdas.bf @@ -1,9 +1,16 @@ using System; +using System.Collections; namespace Tests { class Lambdas { + static void TestIt(List.Enumerator e, int idx, float val) + { + Test.Assert(e.Index == idx); + Test.Assert(e.Current == val); + } + [Test] static void TestBasics() { @@ -20,6 +27,18 @@ namespace Tests act(); Test.Assert(a == 101); + + List iList = scope List() { 10, 20, 30 }; + int idx = 0; + for (var val in iList) + { + delegate void() dlg = scope () => + { + TestIt(@val, idx, val); + }; + dlg(); + ++idx; + } } static int Add3(T func) where T : delegate int()