From dacbcf4eb377dc48a6f794fd44451f0bffdb8581 Mon Sep 17 00:00:00 2001 From: Brian Fiete Date: Wed, 3 Jun 2020 12:07:58 -0700 Subject: [PATCH] Fixed some constraint and generic type lifetime issues --- IDEHelper/Compiler/BfCompiler.cpp | 53 ++++++++++++---- IDEHelper/Compiler/BfContext.cpp | 2 +- IDEHelper/Compiler/BfExprEvaluator.cpp | 20 ++++-- IDEHelper/Compiler/BfModule.cpp | 29 ++++++--- IDEHelper/Compiler/BfModule.h | 2 +- IDEHelper/Compiler/BfModuleTypeUtils.cpp | 74 +++++++++++++++++----- IDEHelper/Compiler/BfResolvedTypeUtils.cpp | 7 +- IDEHelper/Compiler/BfResolvedTypeUtils.h | 5 +- 8 files changed, 145 insertions(+), 47 deletions(-) diff --git a/IDEHelper/Compiler/BfCompiler.cpp b/IDEHelper/Compiler/BfCompiler.cpp index d2679ef2..31e17b4d 100644 --- a/IDEHelper/Compiler/BfCompiler.cpp +++ b/IDEHelper/Compiler/BfCompiler.cpp @@ -2051,7 +2051,14 @@ void BfCompiler::UpdateDependencyMap(bool deleteUnusued, bool& didWork) madeFullPass = false; if ((mResolvePassData != NULL) && (mResolvePassData->mParser != NULL)) madeFullPass = false; - + + if ((deleteUnusued) && (madeFullPass)) + { + // Work queues should be empty if we're not canceling + BF_ASSERT(mContext->mPopulateTypeWorkList.size() == 0); + BF_ASSERT(mContext->mMethodWorkList.size() == 0); + } + // Remove old data in dependency maps, and find types which don't have any references (direct or indirect) // to a non-generic type and remove them for (int pass = 0; true; pass++) @@ -2070,7 +2077,7 @@ void BfCompiler::UpdateDependencyMap(bool deleteUnusued, bool& didWork) auto typeInst = type->ToTypeInstance(); if (depType != NULL) - { + { extern BfModule* gLastCreatedModule; for (auto itr = depType->mDependencyMap.begin(); itr != depType->mDependencyMap.end(); ++itr) @@ -2090,7 +2097,7 @@ void BfCompiler::UpdateDependencyMap(bool deleteUnusued, bool& didWork) auto depTypeInst = dependentType->ToTypeInstance(); auto& depData = itr->mValue; - bool isInvalidVersion = (dependentType->mRevision > depData.mRevision) && (deleteUnusued) && (madeFullPass); + bool isInvalidVersion = (dependentType->mRevision > depData.mRevision);// && (deleteUnusued) && (madeFullPass); //TODO: Just to cause crash if dependentType is deleted bool isIncomplete = dependentType->IsIncomplete(); @@ -2109,11 +2116,16 @@ void BfCompiler::UpdateDependencyMap(bool deleteUnusued, bool& didWork) if ((dependentType->IsDeleting()) || (isInvalidVersion)) { - // If we're deleting the type, OR the dependency of the type has been removed. - // We detect a removed dependency by the dependent type changing but the dependency revision - // is older than the dependent type. - BfLogSysM("Removing old dependent %p from %p\n", dependentType, depType); - itr = depType->mDependencyMap.erase(itr); + if (dependentType->IsDeleting() || ((deleteUnusued) && (madeFullPass))) + { + // If we're deleting the type, OR the dependency of the type has been removed. + // We detect a removed dependency by the dependent type changing but the dependency revision + // is older than the dependent type. + BfLogSysM("Removing old dependent %p from %p\n", dependentType, depType); + itr = depType->mDependencyMap.erase(itr); + } + else + ++itr; } else { @@ -2121,20 +2133,30 @@ void BfCompiler::UpdateDependencyMap(bool deleteUnusued, bool& didWork) // Keep in mind that actually invoking a generic method creates a DependencyFlag_LocalUsage dependency. The // DependencyFlag_MethodGenericArg is just used by the owner during creation of the method specialization bool isDependentUsage = - (depData.mFlags != BfDependencyMap::DependencyFlag_UnspecializedType) && - (depData.mFlags != BfDependencyMap::DependencyFlag_MethodGenericArg); + (depData.mFlags & ~( + BfDependencyMap::DependencyFlag_UnspecializedType | + BfDependencyMap::DependencyFlag_MethodGenericArg | + BfDependencyMap::DependencyFlag_GenericArgRef)) != 0; // We need to consider specialized generic types separately, to remove unused specializations if (typeInst != NULL) { + bool isDirectReference = (depTypeInst != NULL) && (!depTypeInst->IsOnDemand()) && (!dependentType->IsGenericTypeInstance()); + if ((depTypeInst != NULL) && (typeInst->mLastNonGenericUsedRevision != mRevision) && (isDependentUsage) && - ((!dependentType->IsGenericTypeInstance()) || (dependentType->IsUnspecializedType()) || (depTypeInst->mLastNonGenericUsedRevision == mRevision))) - { - typeInst->mLastNonGenericUsedRevision = mRevision; - foundNew = true; + ((isDirectReference) || (dependentType->IsUnspecializedType()) || (depTypeInst->mLastNonGenericUsedRevision == mRevision))) + { + //if (deleteUnusued) + { + typeInst->mLastNonGenericUsedRevision = mRevision; + foundNew = true; + } if (!typeInst->HasBeenReferenced()) + { mContext->AddTypeToWorkList(typeInst); + foundNew = true; + } } } @@ -2299,6 +2321,9 @@ void BfCompiler::ProcessPurgatory(bool reifiedOnly) { auto type = mGenericInstancePurgatory[i]; if ((reifiedOnly) && (!type->IsReified())) + continue; + + if ((reifiedOnly) && ((type->mRebuildFlags & BfTypeRebuildFlag_AwaitingReference) != 0)) continue; if (!type->IsDeleting()) diff --git a/IDEHelper/Compiler/BfContext.cpp b/IDEHelper/Compiler/BfContext.cpp index 7ab7f53d..6265abe7 100644 --- a/IDEHelper/Compiler/BfContext.cpp +++ b/IDEHelper/Compiler/BfContext.cpp @@ -1527,7 +1527,7 @@ void BfContext::DeleteType(BfType* type, bool deferDepRebuilds) continue; } - if ((dependencyEntry.mFlags & ~(BfDependencyMap::DependencyFlag_UnspecializedType)) == 0) + if ((dependencyEntry.mFlags & ~(BfDependencyMap::DependencyFlag_UnspecializedType | BfDependencyMap::DependencyFlag_WeakReference)) == 0) continue; // Not a cause for rebuilding if ((deferDepRebuilds) && (dependentTypeInst != NULL)) diff --git a/IDEHelper/Compiler/BfExprEvaluator.cpp b/IDEHelper/Compiler/BfExprEvaluator.cpp index e0dcee4e..7457de39 100644 --- a/IDEHelper/Compiler/BfExprEvaluator.cpp +++ b/IDEHelper/Compiler/BfExprEvaluator.cpp @@ -415,11 +415,22 @@ bool BfGenericInferContext::InferGenericArgument(BfMethodInstance* methodInstanc return true; } + auto typeInstance = argType->ToTypeInstance(); + if (typeInstance == NULL) + return true; + if (wantGenericType->IsInterface()) + { + for (auto& ifaceEntry : typeInstance->mInterfaces) + InferGenericArgument(methodInstance, ifaceEntry.mInterfaceType, wantType, BfIRValue()); + } + else if (typeInstance->mBaseType != NULL) + InferGenericArgument(methodInstance, typeInstance->mBaseType, wantType, BfIRValue()); + if (!argType->IsGenericTypeInstance()) return true; auto argGenericType = (BfGenericTypeInstance*)argType; if (argGenericType->mTypeDef != wantGenericType->mTypeDef) - return false; + return true; for (int genericArgIdx = 0; genericArgIdx < (int)argGenericType->mTypeGenericArguments.size(); genericArgIdx++) { @@ -1546,7 +1557,7 @@ bool BfMethodMatcher::CheckMethod(BfTypeInstance* targetTypeInstance, BfTypeInst auto wantType = methodInstance->GetParamType(paramIdx); if ((genericArgumentsSubstitute != NULL) && (wantType->IsUnspecializedType())) { - auto resolvedType = mModule->ResolveGenericType(wantType, NULL, genericArgumentsSubstitute); + auto resolvedType = mModule->ResolveGenericType(wantType, NULL, genericArgumentsSubstitute, false); if (resolvedType == NULL) goto NoMatch; wantType = resolvedType; @@ -2164,7 +2175,7 @@ void BfMethodMatcher::TryDevirtualizeCall(BfTypedValue target, BfTypedValue* ori auto useModule = mModule->mContext->mUnreifiedModule; boxedType = useModule->CreateBoxedType(target.mType); useModule->PopulateType(boxedType, BfPopulateType_DataAndMethods); - useModule->AddDependency(boxedType, mModule->mCurTypeInstance, BfDependencyMap::DependencyFlag_Calls); + useModule->AddDependency(boxedType, mModule->mCurTypeInstance, BfDependencyMap::DependencyFlag_WeakReference); } else { @@ -12739,8 +12750,9 @@ BfModuleMethodInstance BfExprEvaluator::GetSelectedMethod(BfAstNode* targetSrc, else paramSrc = methodMatcher.mArguments[methodMatcher.mBestMethodGenericArgumentSrcs[checkGenericIdx]].mExpression; + // Note: don't pass methodMatcher.mBestMethodGenericArguments into here, this method is already specialized BfError* error = NULL; - if (!mModule->CheckGenericConstraints(BfGenericParamSource(methodInstance.mMethodInstance), genericArg, paramSrc, genericParams[checkGenericIdx], &methodMatcher.mBestMethodGenericArguments, + if (!mModule->CheckGenericConstraints(BfGenericParamSource(methodInstance.mMethodInstance), genericArg, paramSrc, genericParams[checkGenericIdx], NULL, failed ? NULL : &error)) { if (methodInstance.mMethodInstance->IsSpecializedGenericMethod()) diff --git a/IDEHelper/Compiler/BfModule.cpp b/IDEHelper/Compiler/BfModule.cpp index cc7240e8..6d4f02ce 100644 --- a/IDEHelper/Compiler/BfModule.cpp +++ b/IDEHelper/Compiler/BfModule.cpp @@ -1224,7 +1224,9 @@ void BfModule::StartNewRevision(RebuildKind rebuildKind, bool force) } } else - mContext->RebuildType(typeInst, false, false, false); + //TODO: Why weren't we placing specialzied in purgatory here originally? This caused failed types to stick around too long + mContext->RebuildType(typeInst, false, false, true); + //mContext->RebuildType(typeInst, false, false, false); } else { @@ -2978,9 +2980,9 @@ bool BfModule::CheckDefineMemberProtection(BfProtection protection, BfType* memb return true; } -void BfModule::AddDependency(BfType* usedType, BfType* usingType, BfDependencyMap::DependencyDependencyFlag flags) -{ - if (usedType == usingType) +void BfModule::AddDependency(BfType* usedType, BfType* userType, BfDependencyMap::DependencyDependencyFlag flags) +{ + if (usedType == userType) return; if ((mCurMethodInstance != NULL) && (mCurMethodInstance->mIsAutocompleteMethod)) @@ -3051,7 +3053,7 @@ void BfModule::AddDependency(BfType* usedType, BfType* usingType, BfDependencyMa if ((!mCompiler->mIsResolveOnly) && (mIsReified)) { - auto usingModule = usingType->GetModule(); + auto usingModule = userType->GetModule(); BfModule* usedModule; if (usedType->IsFunction()) { @@ -3080,7 +3082,7 @@ void BfModule::AddDependency(BfType* usedType, BfType* usingType, BfDependencyMa auto underlyingType = usedType->GetUnderlyingType(); if (underlyingType != NULL) // Not really a "GenericArg", but... same purpose. - AddDependency(underlyingType, usingType, BfDependencyMap::DependencyFlag_GenericArgRef); + AddDependency(underlyingType, userType, BfDependencyMap::DependencyFlag_GenericArgRef); BfDependedType* checkDType = usedType->ToDependedType(); if (checkDType == NULL) @@ -3089,15 +3091,24 @@ void BfModule::AddDependency(BfType* usedType, BfType* usingType, BfDependencyMa if ((usedType->mRebuildFlags & BfTypeRebuildFlag_AwaitingReference) != 0) mContext->MarkAsReferenced(checkDType); - checkDType->mDependencyMap.AddUsedBy(usingType, flags); + if (!checkDType->mDependencyMap.AddUsedBy(userType, flags)) + return; if (checkDType->IsGenericTypeInstance()) { auto genericTypeInstance = (BfGenericTypeInstance*) checkDType; for (auto genericArg : genericTypeInstance->mTypeGenericArguments) { - AddDependency(genericArg, usingType, BfDependencyMap::DependencyFlag_GenericArgRef); + AddDependency(genericArg, userType, BfDependencyMap::DependencyFlag_GenericArgRef); } - } + } + if (checkDType->IsTuple()) + { + for (auto& field : checkDType->ToTypeInstance()->mFieldInstances) + { + if (field.mDataIdx != -1) + AddDependency(field.mResolvedType, userType, BfDependencyMap::DependencyFlag_GenericArgRef); + } + } } void BfModule::AddCallDependency(BfMethodInstance* methodInstance, bool devirtualized) diff --git a/IDEHelper/Compiler/BfModule.h b/IDEHelper/Compiler/BfModule.h index 76defbad..2f61cc25 100644 --- a/IDEHelper/Compiler/BfModule.h +++ b/IDEHelper/Compiler/BfModule.h @@ -1603,7 +1603,7 @@ public: bool CheckAccessMemberProtection(BfProtection protection, BfType* memberType); bool CheckDefineMemberProtection(BfProtection protection, BfType* memberType); void CheckMemberNames(BfTypeInstance* typeInst); - void AddDependency(BfType* usedType, BfType* usingType, BfDependencyMap::DependencyDependencyFlag flags); + void AddDependency(BfType* usedType, BfType* userType, BfDependencyMap::DependencyDependencyFlag flags); void AddCallDependency(BfMethodInstance* methodInstance, bool devirtualized = false); void AddFieldDependency(BfTypeInstance* typeInstance, BfFieldInstance* fieldInstance, BfType* fieldType); void TypeFailed(BfTypeInstance* typeInstance); diff --git a/IDEHelper/Compiler/BfModuleTypeUtils.cpp b/IDEHelper/Compiler/BfModuleTypeUtils.cpp index bb103b23..29acc71f 100644 --- a/IDEHelper/Compiler/BfModuleTypeUtils.cpp +++ b/IDEHelper/Compiler/BfModuleTypeUtils.cpp @@ -103,11 +103,12 @@ bool BfModule::BuildGenericParams(BfType* resolvedTypeRef) BfTypeState typeState; typeState.mPrevState = mContext->mCurTypeState; typeState.mResolveKind = BfTypeState::ResolveKind_BuildingGenericParams; + typeState.mTypeInstance = resolvedTypeRef->ToTypeInstance(); SetAndRestoreValue prevTypeState(mContext->mCurTypeState, &typeState); BF_ASSERT(mCurMethodInstance == NULL); - auto genericTypeInst = (BfGenericTypeInstance*)resolvedTypeRef; + auto genericTypeInst = resolvedTypeRef->ToGenericTypeInstance(); if (genericTypeInst->mTypeGenericArguments[0]->IsGenericParam()) { @@ -195,7 +196,7 @@ bool BfModule::BuildGenericParams(BfType* resolvedTypeRef) } bool BfModule::ValidateGenericConstraints(BfTypeReference* typeRef, BfGenericTypeInstance* genericTypeInst, bool ignoreErrors) -{ + if ((mCurTypeInstance != NULL) && (mCurTypeInstance->IsTypeAlias())) { // Don't validate constraints during the population of a concrete generic type alias instance, we want to @@ -1470,7 +1471,7 @@ void BfModule::SetTypeOptions(BfTypeInstance* typeInstance) } bool BfModule::DoPopulateType(BfType* resolvedTypeRef, BfPopulateType populateType) -{ +{ auto typeInstance = resolvedTypeRef->ToTypeInstance(); auto typeDef = typeInstance->mTypeDef; @@ -2064,9 +2065,22 @@ bool BfModule::DoPopulateType(BfType* resolvedTypeRef, BfPopulateType populateTy BF_ASSERT(typeInstance->mBaseType == baseTypeInst); } - BfType* outerType = GetOuterType(typeInstance); - if (outerType != NULL) - AddDependency(outerType, typeInstance, BfDependencyMap::DependencyFlag_OuterType); + + if (auto genericTypeInst = typeInstance->ToGenericTypeInstance()) + { + if ((genericTypeInst->IsSpecializedType()) && (!genericTypeInst->mValidatedGenericConstraints) && (!typeInstance->IsBoxed())) + { + SetAndRestoreValue ignoreErrors(mIgnoreErrors, true); + ValidateGenericConstraints(NULL, genericTypeInst, false); + } + } + + if (!typeInstance->IsBoxed()) + { + BfType* outerType = GetOuterType(typeInstance); + if (outerType != NULL) + AddDependency(outerType, typeInstance, BfDependencyMap::DependencyFlag_OuterType); + } if ((baseTypeInst != NULL) && (typeInstance->mBaseType == NULL)) { @@ -5654,7 +5668,7 @@ BfTypeDef* BfModule::ResolveGenericInstanceDef(BfGenericInstanceTypeRef* generic } BfType* BfModule::ResolveGenericType(BfType* unspecializedType, BfTypeVector* typeGenericArguments, BfTypeVector* methodGenericArguments, bool allowFail) -{ +{ if (unspecializedType->IsGenericParam()) { auto genericParam = (BfGenericParamType*)unspecializedType; @@ -5966,7 +5980,17 @@ BfType* BfModule::ResolveGenericType(BfType* unspecializedType, BfTypeVector* ty genericArgs.push_back(genericArg); } - return ResolveTypeDef(genericTypeInst->mTypeDef, genericArgs); + auto resolvedType = ResolveTypeDef(genericTypeInst->mTypeDef, genericArgs, BfPopulateType_BaseType); + BfGenericTypeInstance* specializedType = NULL; + if (resolvedType != NULL) + specializedType = resolvedType->ToGenericTypeInstance(); + if (specializedType != NULL) + { + if (specializedType->mHadValidateErrors) + return NULL; + } + + return specializedType; } return unspecializedType; @@ -6285,11 +6309,24 @@ BfType* BfModule::ResolveTypeResult(BfTypeReference* typeRef, BfType* resolvedTy hadError = !populateModule->PopulateType(resolvedTypeRef, populateType); if ((genericTypeInstance != NULL) && (genericTypeInstance != mCurTypeInstance) && (populateType > BfPopulateType_Identity)) - { - if (((genericTypeInstance->mHadValidateErrors) || (!genericTypeInstance->mValidatedGenericConstraints) || (genericTypeInstance->mIsUnspecializedVariation)) && - ((mCurMethodInstance == NULL) || (!mCurMethodInstance->mIsUnspecializedVariation)) && - ((mCurTypeInstance == NULL) || (!mCurTypeInstance->IsUnspecializedTypeVariation())) && - ((mContext->mCurTypeState == NULL) || (mContext->mCurTypeState->mCurBaseTypeRef == NULL))) // We validate constraints for base types later + { + bool doValidate = (genericTypeInstance->mHadValidateErrors) || (!genericTypeInstance->mValidatedGenericConstraints) || (genericTypeInstance->mIsUnspecializedVariation); + if ((mCurMethodInstance != NULL) && (mCurMethodInstance->mIsUnspecializedVariation)) + doValidate = false; + if (mCurTypeInstance != NULL) + { + if (mCurTypeInstance->IsUnspecializedTypeVariation()) + doValidate = false; + if (auto curGenericTypeInstance = mCurTypeInstance->ToGenericTypeInstance()) + { + if (curGenericTypeInstance->mHadValidateErrors) + doValidate = false; + } + if ((mContext->mCurTypeState != NULL) && (mContext->mCurTypeState->mCurBaseTypeRef != NULL)) // We validate constraints for base types later + doValidate = false; + } + + if (doValidate) ValidateGenericConstraints(typeRef, genericTypeInstance, false); } @@ -6396,8 +6433,13 @@ BfTypeDef* BfModule::FindTypeDefRaw(const BfAtomComposite& findName, int numGene } BfTypeInstance* skipCheckBaseType = NULL; - if ((mContext->mCurTypeState != NULL) && (mContext->mCurTypeState->mCurBaseTypeRef != NULL)) - skipCheckBaseType = mContext->mCurTypeState->mTypeInstance; + if (mContext->mCurTypeState != NULL) + { + if (mContext->mCurTypeState->mCurBaseTypeRef != NULL) + skipCheckBaseType = mContext->mCurTypeState->mTypeInstance; + if (mContext->mCurTypeState->mResolveKind == BfTypeState::ResolveKind_BuildingGenericParams) + skipCheckBaseType = mContext->mCurTypeState->mTypeInstance; + } BfTypeDefLookupContext lookupCtx; bool allowPrivate = true; @@ -6407,7 +6449,9 @@ BfTypeDef* BfModule::FindTypeDefRaw(const BfAtomComposite& findName, int numGene BfTypeDef* protErrorTypeDef = NULL; BfTypeInstance* protErrorOuterType = NULL; + + if (!lookupCtx.HasValidMatch()) { std::function _CheckType = [&](BfTypeInstance* typeInstance) diff --git a/IDEHelper/Compiler/BfResolvedTypeUtils.cpp b/IDEHelper/Compiler/BfResolvedTypeUtils.cpp index 074d7021..c1ef340b 100644 --- a/IDEHelper/Compiler/BfResolvedTypeUtils.cpp +++ b/IDEHelper/Compiler/BfResolvedTypeUtils.cpp @@ -58,7 +58,7 @@ bool BfTypedValue::IsValuelessType() const ////////////////////////////////////////////////////////////////////////// -void BfDependencyMap::AddUsedBy(BfType* dependentType, BfDependencyMap::DependencyDependencyFlag flags) +bool BfDependencyMap::AddUsedBy(BfType* dependentType, BfDependencyMap::DependencyDependencyFlag flags) { BF_ASSERT(dependentType != NULL); BF_ASSERT(dependentType->mRevision != -1); @@ -71,6 +71,7 @@ void BfDependencyMap::AddUsedBy(BfType* dependentType, BfDependencyMap::Dependen { dependencyEntry->mRevision = dependentType->mRevision; dependencyEntry->mFlags = flags; + return true; } else { @@ -78,10 +79,14 @@ void BfDependencyMap::AddUsedBy(BfType* dependentType, BfDependencyMap::Dependen { dependencyEntry->mRevision = dependentType->mRevision; dependencyEntry->mFlags = flags; + return true; } else { + if ((dependencyEntry->mFlags & flags) == flags) + return false; dependencyEntry->mFlags = (BfDependencyMap::DependencyDependencyFlag)(dependencyEntry->mFlags | flags); + return true; } } } diff --git a/IDEHelper/Compiler/BfResolvedTypeUtils.h b/IDEHelper/Compiler/BfResolvedTypeUtils.h index 230702c0..4eb61ead 100644 --- a/IDEHelper/Compiler/BfResolvedTypeUtils.h +++ b/IDEHelper/Compiler/BfResolvedTypeUtils.h @@ -82,7 +82,8 @@ public: DependencyFlag_TypeReference = 0x100000, // Explicit type reference for things like tuples, so all referencing types get passed over on symbol reference DependencyFlag_Allocates = 0x200000, DependencyFlag_NameReference = 0x400000, - DependencyFlag_VirtualCall = 0x800000 + DependencyFlag_VirtualCall = 0x800000, + DependencyFlag_WeakReference = 0x1000000, // Keeps alive but won't rebuild }; struct DependencyEntry @@ -102,7 +103,7 @@ public: TypeMap mTypeSet; public: - void AddUsedBy(BfType* dependentType, DependencyDependencyFlag flags); + bool AddUsedBy(BfType* dependentType, DependencyDependencyFlag flags); bool IsEmpty(); TypeMap::iterator begin(); TypeMap::iterator end();