From 664078557f772dd269d99af58fe1cfb8813d57f3 Mon Sep 17 00:00:00 2001 From: Brian Fiete Date: Fri, 10 Apr 2020 07:53:56 -0700 Subject: [PATCH] Improving auto-implemented properties --- .../CompileFail001/scripts/CompileFail.txt | 13 +++- IDE/Tests/CompileFail001/src/Properties.bf | 35 +++++++++ IDEHelper/Compiler/BfAst.h | 13 ++++ IDEHelper/Compiler/BfExprEvaluator.cpp | 44 ++++++++++- IDEHelper/Compiler/BfExprEvaluator.h | 3 +- IDEHelper/Compiler/BfModule.cpp | 52 ++++++++----- IDEHelper/Tests/src/Properties.bf | 76 +++++++++++++++++++ 7 files changed, 212 insertions(+), 24 deletions(-) create mode 100644 IDE/Tests/CompileFail001/src/Properties.bf create mode 100644 IDEHelper/Tests/src/Properties.bf diff --git a/IDE/Tests/CompileFail001/scripts/CompileFail.txt b/IDE/Tests/CompileFail001/scripts/CompileFail.txt index 5bc2b664..7ca86a45 100644 --- a/IDE/Tests/CompileFail001/scripts/CompileFail.txt +++ b/IDE/Tests/CompileFail001/scripts/CompileFail.txt @@ -1,9 +1,9 @@ -ShowFile("src/Protection.bf") +ShowFile("src/Cases.bf") WaitForResolve() SleepTicks(20) AssertFileErrors() -ShowFile("src/Cases.bf") +ShowFile("src/Defer.bf") WaitForResolve() SleepTicks(20) AssertFileErrors() @@ -13,7 +13,12 @@ WaitForResolve() SleepTicks(20) AssertFileErrors() -ShowFile("src/Defer.bf") +ShowFile("src/Properties.bf") WaitForResolve() SleepTicks(20) -AssertFileErrors() \ No newline at end of file +AssertFileErrors() + +ShowFile("src/Protection.bf") +WaitForResolve() +SleepTicks(20) +AssertFileErrors() diff --git a/IDE/Tests/CompileFail001/src/Properties.bf b/IDE/Tests/CompileFail001/src/Properties.bf new file mode 100644 index 00000000..e0857b4a --- /dev/null +++ b/IDE/Tests/CompileFail001/src/Properties.bf @@ -0,0 +1,35 @@ +namespace IDETest +{ + class Properties + { + struct StructA + { + public int mA = 111; + + public this() + { + } + + public this(int a) + { + mA = a; + } + } + + struct StructB + { + public StructA B { get; } + + int mZ = 9; + + public this() //FAIL + { + } + + public void Yoop() mut + { + B = .(); //FAIL + } + } + } +} diff --git a/IDEHelper/Compiler/BfAst.h b/IDEHelper/Compiler/BfAst.h index 7c002c3d..4974c60e 100644 --- a/IDEHelper/Compiler/BfAst.h +++ b/IDEHelper/Compiler/BfAst.h @@ -714,6 +714,19 @@ public: //return (mKind != BfTypedValueKind_NoValue) && ((mValue) || ((mType != NULL) && (IsValuelessType()))); return (mKind != BfTypedValueKind_NoValue); } + + void MakeReadOnly() + { + switch (mKind) + { + case BfTypedValueKind_Addr: + mKind = BfTypedValueKind_ReadOnlyAddr; + break; + case BfTypedValueKind_TempAddr: + mKind = BfTypedValueKind_ReadOnlyTempAddr; + break; + } + } }; #define BF_AST_TYPE(name, TBase) \ diff --git a/IDEHelper/Compiler/BfExprEvaluator.cpp b/IDEHelper/Compiler/BfExprEvaluator.cpp index 53faed9b..30bcad66 100644 --- a/IDEHelper/Compiler/BfExprEvaluator.cpp +++ b/IDEHelper/Compiler/BfExprEvaluator.cpp @@ -3349,11 +3349,11 @@ BfTypedValue BfExprEvaluator::LookupField(BfAstNode* targetSrc, BfTypedValue tar auto field = nextField; nextField = nextField->mNextWithSameName; - if ((!isFailurePass) && (!mModule->CheckProtection(protectionCheckFlags, curCheckType, field->mDeclaringType->mProject, field->mProtection, startCheckType))) + if (((flags & BfLookupFieldFlag_IgnoreProtection) == 0) && (!isFailurePass) && + (!mModule->CheckProtection(protectionCheckFlags, curCheckType, field->mDeclaringType->mProject, field->mProtection, startCheckType))) { continue; } - bool isResolvingFields = curCheckType->mResolvingConstField || curCheckType->mResolvingVarField; @@ -3855,6 +3855,37 @@ BfTypedValue BfExprEvaluator::LookupField(BfAstNode* targetSrc, BfTypedValue tar } } + // Check for direct auto-property access + if (startCheckType == mModule->mCurTypeInstance) + { + if (auto propertyDeclaration = BfNodeDynCast(mPropDef->mFieldDeclaration)) + { + if (curCheckType->mTypeDef->HasAutoProperty(propertyDeclaration)) + { + bool hasSetter = GetPropertyMethodDef(mPropDef, BfMethodType_PropertySetter, BfCheckedKind_NotSet) != NULL; + auto autoFieldName = curCheckType->mTypeDef->GetAutoPropertyName(propertyDeclaration); + auto result = LookupField(targetSrc, target, autoFieldName, BfLookupFieldFlag_IgnoreProtection); + if (result) + { + if (!hasSetter) + { + if (((mModule->mCurMethodInstance->mMethodDef->mMethodType == BfMethodType_Ctor)) && + (startCheckType == mModule->mCurTypeInstance)) + { + // Allow writing inside ctor + } + else + result.MakeReadOnly(); + } + mPropDef = NULL; + mPropSrc = NULL; + mOrigPropTarget = NULL; + return result; + } + } + } + } + SetAndRestoreValue prevResult(mResult, target); CheckResultForReading(mResult); return BfTypedValue(); @@ -14502,6 +14533,15 @@ bool BfExprEvaluator::CheckModifyResult(BfTypedValue typedVal, BfAstNode* refNod mModule->TypeToString(mResultFieldInstance->mOwner).c_str(), mResultFieldInstance->GetFieldDef()->mName.c_str(), mModule->MethodToString(mModule->mCurMethodInstance).c_str()), refNode); } + else if (auto propertyDeclaration = BfNodeDynCast(mResultFieldInstance->GetFieldDef()->mFieldDeclaration)) + { + String propNam; + if (propertyDeclaration->mNameNode != NULL) + propertyDeclaration->mNameNode->ToString(propNam); + + error = mModule->Fail(StrFormat("Cannot %s auto-implemented property '%s.%s' without set accessor", modifyType, + mModule->TypeToString(mResultFieldInstance->mOwner).c_str(), propNam.c_str()), refNode); + } else { error = mModule->Fail(StrFormat("Cannot %s field '%s.%s' within struct method '%s'. Consider adding 'mut' specifier to this method.", modifyType, diff --git a/IDEHelper/Compiler/BfExprEvaluator.h b/IDEHelper/Compiler/BfExprEvaluator.h index 075c278b..6493db14 100644 --- a/IDEHelper/Compiler/BfExprEvaluator.h +++ b/IDEHelper/Compiler/BfExprEvaluator.h @@ -266,7 +266,8 @@ enum BfLookupFieldFlags { BfLookupFieldFlag_None = 0, BfLookupFieldFlag_IsImplicitThis = 1, - BfLookupFieldFlag_CheckingOuter = 2 + BfLookupFieldFlag_CheckingOuter = 2, + BfLookupFieldFlag_IgnoreProtection = 4 }; enum BfBinOpFlags diff --git a/IDEHelper/Compiler/BfModule.cpp b/IDEHelper/Compiler/BfModule.cpp index 31ea4a21..4ab51a86 100644 --- a/IDEHelper/Compiler/BfModule.cpp +++ b/IDEHelper/Compiler/BfModule.cpp @@ -2023,7 +2023,25 @@ void BfModule::LocalVariableDone(BfLocalVariable* localVar, bool isMethodExit) if ((localVar->mUnassignedFieldFlags & checkMask) != 0) { auto fieldDef = fieldInstance.GetFieldDef(); - if (fieldDef->mProtection != BfProtection_Hidden) + if (auto propertyDeclaration = BfNodeDynCast(fieldDef->mFieldDeclaration)) + { + String propName; + if (propertyDeclaration->mNameNode != NULL) + propertyDeclaration->mNameNode->ToString(propName); + + if (checkTypeInstance == mCurTypeInstance) + { + Fail(StrFormat("Auto-implemented property '%s' must be fully assigned before control is returned to the caller", + propName.c_str()), localNameNode, deferFullAnalysis); // 0171 + } + else + { + Fail(StrFormat("Auto-implemented property '%s.%s' must be fully assigned before control is returned to the caller", + TypeToString(checkTypeInstance).c_str(), + propName.c_str()), localNameNode, deferFullAnalysis); // 0171 + } + } + else { if (checkTypeInstance == mCurTypeInstance) { @@ -7128,11 +7146,6 @@ BfTypedValue BfModule::CreateValueFromExpression(BfExprEvaluator& exprEvaluator, typedVal = LoadValue(typedVal, 0, exprEvaluator.mIsVolatileReference); } -// if ((typedVal.IsSplat()) && ((flags & BfEvalExprFlags_AllowSplat) == 0)) -// typedVal = MakeAddressable(typedVal); - - //typedVal = AggregateSplat(typedVal); - if ((typedVal.mType->IsValueType()) && ((flags & BfEvalExprFlags_NoValueAddr) != 0)) typedVal = LoadValue(typedVal, 0, exprEvaluator.mIsVolatileReference); @@ -14358,10 +14371,12 @@ void BfModule::EmitCtorBody(bool& skipBody) if (fieldDef->mInitializer == NULL) { - if (fieldDef->mProtection != BfProtection_Hidden) - continue; - if (mCurTypeInstance->IsObject()) // Already zeroed out - continue; + continue; + +// if (fieldDef->mProtection != BfProtection_Hidden) +// continue; +// if (mCurTypeInstance->IsObject()) // Already zeroed out +// continue; } if (fieldInst->mResolvedType == NULL) @@ -17325,17 +17340,16 @@ void BfModule::ProcessMethod(BfMethodInstance* methodInstance, bool isInlineDup) else if (mCurTypeInstance->IsObject()) lookupValue = BfTypedValue(mBfIRBuilder->CreateInBoundsGEP(GetThis().mValue, 0, fieldInstance->mDataIdx), fieldInstance->mResolvedType, true); else - lookupValue = ExtractValue(GetThis(), fieldInstance, fieldInstance->mFieldIdx); - if (!methodInstance->mReturnType->IsRef()) - lookupValue = LoadValue(lookupValue); - mBfIRBuilder->CreateRet(lookupValue.mValue); + lookupValue = ExtractValue(GetThis(), fieldInstance, fieldInstance->mFieldIdx); + lookupValue = LoadOrAggregateValue(lookupValue); + CreateReturn(lookupValue.mValue); EmitLifetimeEnds(&mCurMethodState->mHeadScope); } else { // This can happen if we have two properties with the same name but different types - AssertErrorState(); - mBfIRBuilder->CreateRet(GetDefaultValue(methodInstance->mReturnType)); + CreateReturn(GetDefaultValue(mCurMethodInstance->mReturnType)); + EmitDefaultReturn(); } } else @@ -17370,7 +17384,11 @@ void BfModule::ProcessMethod(BfMethodInstance* methodInstance, bool isInlineDup) thisValue = LoadValue(thisValue); lookupAddr = mBfIRBuilder->CreateInBoundsGEP(thisValue.mValue, 0, fieldInstance->mDataIdx); } - mBfIRBuilder->CreateStore(lastParam->mValue, lookupAddr); + + BfExprEvaluator exprEvaluator(this); + auto localVal = exprEvaluator.LoadLocal(lastParam); + localVal = LoadOrAggregateValue(localVal); + mBfIRBuilder->CreateStore(localVal.mValue, lookupAddr); } else if (!fieldInstance->mResolvedType->IsValuelessType()) { diff --git a/IDEHelper/Tests/src/Properties.bf b/IDEHelper/Tests/src/Properties.bf new file mode 100644 index 00000000..a63c5876 --- /dev/null +++ b/IDEHelper/Tests/src/Properties.bf @@ -0,0 +1,76 @@ +#pragma warning disable 168 + +using System; + +namespace Tests +{ + class Properties + { + struct StructA + { + public int mA = 111; + + public this() + { + } + + public this(int a) + { + mA = a; + } + } + + struct StructB + { + public StructA B { get; set; } + + int mZ = 9; + + public this() + { + B = .(); + } + } + + struct StructC + { + public StructA B { get; } + + int mZ = 9; + + public this() + { + B = .(); + } + } + + class ClassB + { + public StructA B { get; set; } + + int mZ = 9; + + public this() + { + } + } + + [Test] + public static void TestBasics() + { + StructB sb = .(); + StructA sa = sb.B; + Test.Assert(sa.mA == 111); + sb.B = .(222); + sa = sb.B; + Test.Assert(sa.mA == 222); + + ClassB cb = scope .(); + sa = cb.B; + Test.Assert(sa.mA == 0); + cb.B = .(333); + sa = cb.B; + Test.Assert(sa.mA == 333); + } + } +}