From 27a586df04cba8134d95b8bc28047e8322f7e3eb Mon Sep 17 00:00:00 2001 From: Brian Fiete Date: Fri, 25 Dec 2020 05:22:02 -0800 Subject: [PATCH] Fixed atom deletion issue --- IDE/Tests/BugW006/BeefProj.toml | 5 ++++ IDE/Tests/BugW006/BeefSpace.toml | 6 +++++ IDE/Tests/BugW006/scripts/Test.txt | 25 ++++++++++++++++++++ IDE/Tests/BugW006/src/Program.bf | 25 ++++++++++++++++++++ IDE/src/ScriptManager.bf | 10 ++++++++ IDEHelper/Compiler/BfContext.cpp | 37 ++++++++++++++++++------------ IDEHelper/Compiler/BfSystem.cpp | 32 +++++++++++++++++++++++++- IDEHelper/Compiler/BfSystem.h | 7 ++++-- bin/test_ide.bat | 4 ++++ 9 files changed, 133 insertions(+), 18 deletions(-) create mode 100644 IDE/Tests/BugW006/BeefProj.toml create mode 100644 IDE/Tests/BugW006/BeefSpace.toml create mode 100644 IDE/Tests/BugW006/scripts/Test.txt create mode 100644 IDE/Tests/BugW006/src/Program.bf diff --git a/IDE/Tests/BugW006/BeefProj.toml b/IDE/Tests/BugW006/BeefProj.toml new file mode 100644 index 00000000..f0c46edc --- /dev/null +++ b/IDE/Tests/BugW006/BeefProj.toml @@ -0,0 +1,5 @@ +FileVersion = 1 + +[Project] +Name = "Bug" +StartupObject = "Bug.Program" diff --git a/IDE/Tests/BugW006/BeefSpace.toml b/IDE/Tests/BugW006/BeefSpace.toml new file mode 100644 index 00000000..c389207f --- /dev/null +++ b/IDE/Tests/BugW006/BeefSpace.toml @@ -0,0 +1,6 @@ +FileVersion = 1 +Projects = {Bug = {Path = "."}} + +[Workspace] +StartupProject = "Bug" + diff --git a/IDE/Tests/BugW006/scripts/Test.txt b/IDE/Tests/BugW006/scripts/Test.txt new file mode 100644 index 00000000..9a30b558 --- /dev/null +++ b/IDE/Tests/BugW006/scripts/Test.txt @@ -0,0 +1,25 @@ +# This tests that types that fail generic tests don't create types referenced in methods +# and also that they get deleted immediately when they are dereferenced. + +ShowFile("src/Program.bf") + +GotoText("//Test_Y") +AdjustCursor(16, 1) +InsertText(" abc") +Sleep(500) +InsertText(", in") +Sleep(500) +InsertText("t def") + +Compile() + +DeleteTextBackward(5) +Sleep(500) +DeleteTextBackward(4) +Sleep(500) +DeleteTextBackward(4) +Sleep(500) +ToggleCommentAt("Test_Definition") + +Sleep(500) +Compile() diff --git a/IDE/Tests/BugW006/src/Program.bf b/IDE/Tests/BugW006/src/Program.bf new file mode 100644 index 00000000..aaf8e8e2 --- /dev/null +++ b/IDE/Tests/BugW006/src/Program.bf @@ -0,0 +1,25 @@ +#pragma warning disable 168 + +using System; +using System.Collections; + +namespace Bug +{ + class Program + { + //*Test_Definition + public class Test where K: enum + { + typealias x = function void (T t); + //Test_Y + typealias y = (K); + } + /*@*/ + + + public static int Main(String[] args) + { + return 0; + } + } +} diff --git a/IDE/src/ScriptManager.bf b/IDE/src/ScriptManager.bf index 23838d45..4d897471 100644 --- a/IDE/src/ScriptManager.bf +++ b/IDE/src/ScriptManager.bf @@ -2172,6 +2172,16 @@ namespace IDE textPanel.EditWidget.mEditWidgetContent.DeleteSelection(); } + [IDECommand] + public void DeleteTextBackward(int count) + { + var textPanel = GetActiveTextPanel(); + if (textPanel == null) + return; + for (int i < count) + textPanel.EditWidget.mEditWidgetContent.Backspace(); + } + [IDECommand] public void MarkPosition() { diff --git a/IDEHelper/Compiler/BfContext.cpp b/IDEHelper/Compiler/BfContext.cpp index 2827f30c..3154eab9 100644 --- a/IDEHelper/Compiler/BfContext.cpp +++ b/IDEHelper/Compiler/BfContext.cpp @@ -2159,23 +2159,30 @@ void BfContext::VerifyTypeLookups(BfTypeInstance* typeInst) // If any atoms have been placed in the graveyard, typesHash will be zero and thus cause a rebuild uint32 atomUpdateIdx = lookupEntry.mName.GetAtomUpdateIdx(); - // Sanity check, mostly checking that useTypeDef wasn't deleted - BF_ASSERT((lookupEntry.mUseTypeDef->mName->mAtomUpdateIdx >= 1) && (lookupEntry.mUseTypeDef->mName->mAtomUpdateIdx <= mSystem->mAtomUpdateIdx)); - - // Only do the actual lookup if types were added or removed whose name is contained in one of the name parts referenced - if (atomUpdateIdx != lookupEntry.mAtomUpdateIdx) + if (atomUpdateIdx == 0) { - // NOTE: we purposely don't use mNextRevision here. If the the was NOT rebuilt then that means we didn't actually rebuild - // so the mNextRevision will be ignored - auto useTypeDef = lookupEntry.mUseTypeDef; - BfTypeDef* ambiguousTypeDef = NULL; - BfTypeDef* result = mSystem->FindTypeDef(lookupEntry.mName, lookupEntry.mNumGenericParams, useTypeDef->mProject, useTypeDef->mNamespaceSearch, &ambiguousTypeDef); - if (result != lookupEntryPair.mValue.mTypeDef) - { - isDirty = true; + isDirty = true; + } + else + { + // Sanity check, mostly checking that useTypeDef wasn't deleted + BF_ASSERT((lookupEntry.mUseTypeDef->mName->mAtomUpdateIdx >= 1) && (lookupEntry.mUseTypeDef->mName->mAtomUpdateIdx <= mSystem->mAtomUpdateIdx)); + + // Only do the actual lookup if types were added or removed whose name is contained in one of the name parts referenced + if (atomUpdateIdx != lookupEntry.mAtomUpdateIdx) + { + // NOTE: we purposely don't use mNextRevision here. If the the was NOT rebuilt then that means we didn't actually rebuild + // so the mNextRevision will be ignored + auto useTypeDef = lookupEntry.mUseTypeDef; + BfTypeDef* ambiguousTypeDef = NULL; + BfTypeDef* result = mSystem->FindTypeDef(lookupEntry.mName, lookupEntry.mNumGenericParams, useTypeDef->mProject, useTypeDef->mNamespaceSearch, &ambiguousTypeDef); + if (result != lookupEntryPair.mValue.mTypeDef) + { + isDirty = true; + } + else + lookupEntry.mAtomUpdateIdx = atomUpdateIdx; } - else - lookupEntry.mAtomUpdateIdx = atomUpdateIdx; } } diff --git a/IDEHelper/Compiler/BfSystem.cpp b/IDEHelper/Compiler/BfSystem.cpp index 5804572b..09782468 100644 --- a/IDEHelper/Compiler/BfSystem.cpp +++ b/IDEHelper/Compiler/BfSystem.cpp @@ -103,6 +103,8 @@ void Beefy::DoBfLog(int fileIdx, const char* fmt ...) BfAtom::~BfAtom() { BF_ASSERT(mPrevNamesMap.IsEmpty()); + BF_ASSERT(mRefCount == 0); + BF_ASSERT(mPendingDerefCount == 0); } void BfAtom::Ref() @@ -360,7 +362,7 @@ uint32 BfAtomComposite::GetAtomUpdateIdx() for (int i = 0; i < mSize; i++) { auto atom = mParts[i]; - if (atom->mRefCount == 0) + if ((atom->mRefCount - atom->mPendingDerefCount) == 0) return 0; // 0 is our "error condition" when we're looking at a graveyard'ed atom updateIdx = BF_MAX(updateIdx, atom->mAtomUpdateIdx); } @@ -618,6 +620,12 @@ void BfTypeDef::FreeMembers() { if (!mIsNextRevision) mSystem->UntrackName(this); + if (mInDeleteQueue) + { + BF_ASSERT(mName->mPendingDerefCount > 0); + if (mName->mPendingDerefCount > 0) + mName->mPendingDerefCount--; + } mSystem->ReleaseAtom(mName); } mName = NULL; @@ -625,6 +633,12 @@ void BfTypeDef::FreeMembers() if (mNameEx != NULL) { + if (mInDeleteQueue) + { + BF_ASSERT(mNameEx->mPendingDerefCount > 0); + if (mNameEx->mPendingDerefCount > 0) + mNameEx->mPendingDerefCount--; + } mSystem->ReleaseAtom(mNameEx); mNameEx = NULL; } @@ -1854,6 +1868,7 @@ BfAtom* BfSystem::GetAtom(const StringImpl& string) atom->mAtomUpdateIdx = ++mAtomUpdateIdx; atom->mString = *stringPtr; atom->mRefCount = 1; + atom->mPendingDerefCount = 0; atom->mHash = 0; for (char c : string) @@ -2496,6 +2511,21 @@ void BfSystem::RemoveTypeDef(BfTypeDef* typeDef) // mTypeDef is already locked by the system lock mTypeDefs.Remove(typeDef); AutoCrit autoCrit(mDataLock); + + // This will get properly handled in UntrackName when we process the mTypeDefDeleteQueue, but this + // mAtomUpdateIdx increment will trigger lookup changes in BfContext::VerifyTypeLookups + if (typeDef->mName != mEmptyAtom) + { + typeDef->mName->mAtomUpdateIdx = ++mAtomUpdateIdx; + typeDef->mName->mPendingDerefCount++; + } + if (typeDef->mNameEx != mEmptyAtom) + { + typeDef->mNameEx->mAtomUpdateIdx = ++mAtomUpdateIdx; + typeDef->mNameEx->mPendingDerefCount++; + } + typeDef->mInDeleteQueue = true; + mTypeDefDeleteQueue.push_back(typeDef); mTypeMapVersion++; } diff --git a/IDEHelper/Compiler/BfSystem.h b/IDEHelper/Compiler/BfSystem.h index d82235f3..a53698fd 100644 --- a/IDEHelper/Compiler/BfSystem.h +++ b/IDEHelper/Compiler/BfSystem.h @@ -50,7 +50,8 @@ class BfAtom { public: StringView mString; - int mRefCount; + int mRefCount; + int mPendingDerefCount; int mHash; uint32 mAtomUpdateIdx; bool mIsSystemType; @@ -929,7 +930,8 @@ public: bool mHasExtensionMethods; bool mHasOverrideMethods; bool mIsOpaque; - bool mIsNextRevision; + bool mIsNextRevision; + bool mInDeleteQueue; public: BfTypeDef() @@ -969,6 +971,7 @@ public: mIsOpaque = false; mPartialUsed = false; mIsNextRevision = false; + mInDeleteQueue = false; mDupDetectedRevision = -1; mNestDepth = 0; mOuterType = NULL; diff --git a/bin/test_ide.bat b/bin/test_ide.bat index f4a115cb..dc0552d7 100644 --- a/bin/test_ide.bat +++ b/bin/test_ide.bat @@ -59,6 +59,10 @@ PUSHD %~dp0..\ @CALL :TEST @IF !ERRORLEVEL! NEQ 0 GOTO HADERROR +@SET TESTPATH=IDE\Tests\BugW006 +@CALL :TEST +@IF !ERRORLEVEL! NEQ 0 GOTO HADERROR + @SET TESTPATH=IDE\Tests\IndentTest @CALL :TEST @IF !ERRORLEVEL! NEQ 0 GOTO HADERROR