[education/rkward] rkward: Basics of cache based modification detection functional

Thomas Friedrichsmeier null at kde.org
Thu May 26 10:06:45 BST 2022


Git commit f4d7d0965b263eb72d3886bd2da8a88e453f2d7d by Thomas Friedrichsmeier.
Committed on 19/05/2022 at 19:50.
Pushed by tfry into branch 'master'.

Basics of cache based modification detection functional

M  +11   -19   rkward/core/renvironmentobject.cpp
M  +1    -1    rkward/core/renvironmentobject.h
M  +1    -1    rkward/core/rkpseudoobjects.cpp
M  +2    -1    rkward/core/rkpseudoobjects.h
M  +1    -1    rkward/core/robjectlist.cpp
M  +19   -47   rkward/rbackend/rkrbackend.cpp
M  +1    -3    rkward/rbackend/rkrbackend.h
M  +40   -34   rkward/rbackend/rkrinterface.cpp
M  +1    -1    rkward/rbackend/rkrinterface.h
M  +29   -23   rkward/rbackend/rkrsupport.cpp
M  +9    -2    rkward/rbackend/rkrsupport.h
M  +1    -1    rkward/rbackend/rpackages/rkward/DESCRIPTION
M  +0    -3    rkward/rbackend/rpackages/rkward/NAMESPACE

https://invent.kde.org/education/rkward/commit/f4d7d0965b263eb72d3886bd2da8a88e453f2d7d

diff --git a/rkward/core/renvironmentobject.cpp b/rkward/core/renvironmentobject.cpp
index c308931f..62ca2ae6 100644
--- a/rkward/core/renvironmentobject.cpp
+++ b/rkward/core/renvironmentobject.cpp
@@ -109,31 +109,23 @@ void REnvironmentObject::updateFromR (RCommandChain *chain) {
 	type |= Updating;
 }
 
-void REnvironmentObject::updateFromR (RCommandChain *chain, const QStringList &current_symbols) {
+void REnvironmentObject::updateFromR(RCommandChain *chain, const QStringList &added_symbols, const QStringList &removed_symbols) {
 	RK_TRACE (OBJECTS);
 
-	// only needed for the assert at the end
-	int debug_baseline = 0;
-
-	// which children are missing?
-	for (int i = childmap.size () - 1; i >= 0; --i) {
-		RObject *object = childmap[i];
-		if (!current_symbols.contains (object->getShortName ())) {
-			if (object->isPending () || (!(RKModificationTracker::instance()->removeObject (object, 0, true)))) debug_baseline++;
-		}
+	for (int i = removed_symbols.size() -1; i >= 0; --i) {
+		RObject *removed_object = findChildByName(removed_symbols[i]);
+		RK_ASSERT(removed_object);
+		if (removed_object) RKModificationTracker::instance()->removeObject(removed_object, 0, true);
 	}
 
-	// which ones are new in the list?
-	for (int i = current_symbols.count () - 1; i >= 0; --i) {
-		RObject *child = findChildByName (current_symbols[i]);
-		if (!child) child = createPendingChild (current_symbols[i], i, false, false);
-		if (child->isPending ()) {
-			child->type -= RObject::Pending;	// HACK: Child is not actually pending: We've seen it!
-			child->updateFromR (chain);
+	for (int i = added_symbols.size() -1; i >= 0; --i) {
+		RObject *child = findChildByName(added_symbols[i]);
+		if (!child) child = createPendingChild(added_symbols[i], i, false, false);
+		if (child->isPending()) {
+			child->type -= RObject::Pending;  // HACK: Child is not actually pending: We've just seen it!
+			child->updateFromR(chain);
 		}
 	}
-
-	RK_ASSERT ((debug_baseline + current_symbols.count ()) == childmap.size ());
 }
 
 bool REnvironmentObject::updateStructure (RData *new_data) {
diff --git a/rkward/core/renvironmentobject.h b/rkward/core/renvironmentobject.h
index cd1437c3..ab9506dc 100644
--- a/rkward/core/renvironmentobject.h
+++ b/rkward/core/renvironmentobject.h
@@ -24,7 +24,7 @@ public:
 
 	void updateFromR (RCommandChain *chain) override;
 /** like updateFromR, but only update new / removed symbols from R. Theoretically this could be defined in RContainerObject, but the only use case is for environments. */
-	virtual void updateFromR (RCommandChain *chain, const QStringList &current_symbols);
+	virtual void updateFromR (RCommandChain *chain, const QStringList &added_symbols, const QStringList &removed_symbols);
 
 	QString getFullName (int) const override;
 	QString makeChildName (const QString &short_child_name, bool misplaced, int options) const override;
diff --git a/rkward/core/rkpseudoobjects.cpp b/rkward/core/rkpseudoobjects.cpp
index 4ea73136..cd07a90a 100644
--- a/rkward/core/rkpseudoobjects.cpp
+++ b/rkward/core/rkpseudoobjects.cpp
@@ -95,7 +95,7 @@ void RKOrphanNamespacesObject::updateFromR (RCommandChain* chain) {
 	RK_ASSERT (false);
 }
 
-void RKOrphanNamespacesObject::updateFromR (RCommandChain* chain, const QStringList& current_symbols) {
+void RKOrphanNamespacesObject::updateNamespacesFromR (RCommandChain* chain, const QStringList& current_symbols) {
 	RK_TRACE (OBJECTS);
 	Q_UNUSED (chain);	// because the namespace objects themselves are not updated, only added as incomplete objects
 
diff --git a/rkward/core/rkpseudoobjects.h b/rkward/core/rkpseudoobjects.h
index c7c102b9..01c75917 100644
--- a/rkward/core/rkpseudoobjects.h
+++ b/rkward/core/rkpseudoobjects.h
@@ -70,10 +70,11 @@ public:
 
 	RKNamespaceObject *findOrphanNamespace (const QString &name) const;
 
+	using REnvironmentObject::updateFromR;
 	/** should not be called on this object. Reimplemented to raise an assert, and do nothing else. */
 	void updateFromR (RCommandChain *chain) override;
 	/** reimplemented from REnvironmentObject */
-	void updateFromR (RCommandChain *chain, const QStringList &current_symbols) override;
+	void updateNamespacesFromR (RCommandChain *chain, const QStringList &current_symbols);
 };
 
 #endif
diff --git a/rkward/core/robjectlist.cpp b/rkward/core/robjectlist.cpp
index 59c6c91f..0e879e70 100644
--- a/rkward/core/robjectlist.cpp
+++ b/rkward/core/robjectlist.cpp
@@ -212,7 +212,7 @@ void RObjectList::updateNamespaces (const QStringList &namespace_names) {
 	for (int i = 0; i < namespace_names.size (); ++i) {
 		if (!findPackage (namespace_names[i])) orphan_namespace_names.append (namespace_names[i]);
 	}
-	orphan_namespaces->updateFromR (update_chain, orphan_namespace_names);
+	orphan_namespaces->updateNamespacesFromR (update_chain, orphan_namespace_names);
 }
 
 REnvironmentObject *RObjectList::createTopLevelEnvironment (const QString &name) {
diff --git a/rkward/rbackend/rkrbackend.cpp b/rkward/rbackend/rkrbackend.cpp
index 7cc7b7c2..33a4f648 100644
--- a/rkward/rbackend/rkrbackend.cpp
+++ b/rkward/rbackend/rkrbackend.cpp
@@ -931,8 +931,9 @@ SEXP doSubstackCall (SEXP call) {
 		return R_NilValue;
 	} */
 
-
-	auto ret = RKRBackend::this_pointer->handleRequestWithSubcommands(list);
+	QStringList args;
+	if (list.size() > 1) args = list.mid(1);
+	auto ret = RKRBackend::this_pointer->handleRequestWithSubcommands(list.value(0), args);
 	if (!ret.warning.isEmpty()) Rf_warning(RKRBackend::fromUtf8(ret.warning));  // print warnings, first, as errors will cause a stop
 	if (!ret.error.isEmpty()) Rf_error(RKRBackend::fromUtf8(ret.error));
 
@@ -1058,7 +1059,8 @@ SEXP RKD_AdjustSize (SEXP devnum, SEXP id);
 void doPendingPriorityCommands ();
 
 SEXP checkEnv(SEXP a) {
-	return RKRSupport::StringListToSEXP(RKRShadowEnvironment::environmentFor(a)->diffAndUpdate());
+	auto res = RKRShadowEnvironment::diffAndUpdate(a);
+	return RKRSupport::StringListToSEXP(res.added + res.changed + res.removed);
 }
 
 bool RKRBackend::startR () {
@@ -1630,11 +1632,12 @@ RCommandProxy* RKRBackend::fetchNextCommand () {
 	return (handleRequest (&req, false));
 }
 
-GenericRRequestResult RKRBackend::handleRequestWithSubcommands(const QStringList &list) {
+GenericRRequestResult RKRBackend::handleRequestWithSubcommands(const QString &call, const QVariant &params) {
 	RK_TRACE (RBACKEND);
 
 	RBackendRequest request(true, RBackendRequest::GenericRequestWithSubcommands);
-	request.params["call"] = list;
+	request.params["call"] = call;
+	if (!params.isNull()) request.params["args"] = params;
 	request.command = current_command;
 	request.subcommandrequest = new RBackendRequest(true, RBackendRequest::OtherRequest);
 	handleRequest(&request);
@@ -1732,11 +1735,7 @@ void RKRBackend::checkObjectUpdatesNeeded (bool check_list) {
 	RK_TRACE (RBACKEND);
 	if (killed) return;
 
-	/* NOTE: We're keeping separate lists of the items on the search path, and the toplevel symbols in .GlobalEnv here.
-	This info is also present in RObjectList (and it's children). However: a) in a less convenient form, b) in the other thread. To avoid locking, and other complexity, keeping separate lists seems an ok solution. Keep in mind that only the names of only the toplevel objects are kept, here, so the memory overhead should be minimal */
-
 	bool search_update_needed = false;
-	bool globalenv_update_needed = false;
 
 	if (check_list) {	
 	// TODO: avoid parsing this over and over again
@@ -1746,50 +1745,23 @@ void RKRBackend::checkObjectUpdatesNeeded (bool check_list) {
 		if (search_update_needed) toplevel_env_names = dummy->stringVector ();
 		delete dummy;
 	
-	// TODO: avoid parsing this over and over again
-		RK_DEBUG (RBACKEND, DL_TRACE, "checkObjectUpdatesNeeded: getting globalenv symbols");
-		dummy = runDirectCommand ("ls (globalenv (), all.names=TRUE)\n", RCommand::GetStringVector);
-		QStringList new_globalenv_toplevel_names = dummy->stringVector ();
-		if (new_globalenv_toplevel_names.count () != global_env_toplevel_names.count ()) {
-			globalenv_update_needed = true;
-		} else {
-			for (int i = 0; i < new_globalenv_toplevel_names.count (); ++i) {
-				// order is not important in the symbol list
-				if (!global_env_toplevel_names.contains (new_globalenv_toplevel_names[i])) {
-					globalenv_update_needed = true;
-					break;
-				}
-			}
-		}
-		if (globalenv_update_needed) global_env_toplevel_names = new_globalenv_toplevel_names;
-		delete dummy;
-	
 		if (search_update_needed) {	// this includes an update of the globalenv, even if not needed
-			QStringList call ("syncenvs");
-			call.append (QString::number (toplevel_env_names.size ()));
-			call.append (toplevel_env_names);
 			dummy = runDirectCommand ("loadedNamespaces ()\n", RCommand::GetStringVector);
-			call.append (dummy->stringVector ());
 			delete dummy;
-			handleRequestWithSubcommands (call);
+			QVariantList args;
+			args.append(QVariant(toplevel_env_names));
+			args.append(QVariant(dummy->stringVector()));
+			handleRequestWithSubcommands("syncenvs", args);
 		} 
-		if (globalenv_update_needed) {
-			QStringList call = global_env_toplevel_names;
-			call.prepend ("syncglobal");	// should be faster than the reverse
-			handleRequestWithSubcommands (call);
-		}
-	}
-
-	if (search_update_needed || globalenv_update_needed) {
-		RK_DEBUG (RBACKEND, DL_TRACE, "checkObjectUpdatesNeeded: updating watches");
-		runDirectCommand (".rk.watch.globalenv ()\n");
 	}
 
-	if (!changed_symbol_names.isEmpty ()) {
-		QStringList call = changed_symbol_names;
-		call.prepend (QString ("sync"));	// should be faster than reverse
-		handleRequestWithSubcommands (call);
-		changed_symbol_names.clear ();
+	auto changes = RKRShadowEnvironment::diffAndUpdate(R_GlobalEnv);
+	if (!changes.isEmpty()) {
+		QVariantList args;
+		args.append(changes.added);
+		args.append(changes.removed);
+		args.append(changes.changed);
+		handleRequestWithSubcommands("sync", args);
 	}
 }
 
diff --git a/rkward/rbackend/rkrbackend.h b/rkward/rbackend/rkrbackend.h
index d45e5008..c247d8d1 100644
--- a/rkward/rbackend/rkrbackend.h
+++ b/rkward/rbackend/rkrbackend.h
@@ -97,7 +97,7 @@ public:
 	void handleRequest(RBackendRequest *request) { handleRequest (request, true); };
 /** A relic of history. In contrast to handlePlainGenericRequest(), these requests support running sub-commands. However, the remaining requests which are currently handled this way
 should probably be converted to dedicated RKRBackendRequest's in the future. See also handlePlainGenericRequest(). */
-	GenericRRequestResult handleRequestWithSubcommands(const QStringList &list);
+	GenericRRequestResult handleRequestWithSubcommands(const QString &call, const QVariant &args=QVariant());
 /** Sends a request to the frontend and returns the result (empty in case of asynchronous requests). Note that this function has considerable overlap with
 handleHistoricalSubstackRequest(). Exactly which requests get handled by which function is somewhat arbitrary, ATM. However, request that do not need sub-commands to be run, should generally be converted to use handlePlainGenericRequest(). (And probably all historicalSubstackRequests should be replaced!) */
 	GenericRRequestResult handlePlainGenericRequest(const QStringList &parameters, bool synchronous);
@@ -209,8 +209,6 @@ private:
 	QString output_file;
 /** A copy of the names of the toplevel environments (as returned by "search ()"). */
 	QStringList toplevel_env_names;
-/** A copy of the names of the toplevel symbols in the .GlobalEnv. */
-	QStringList global_env_toplevel_names;
 /** check whether the object list / global environment / individual symbols have changed, and updates them, if needed */
 	void checkObjectUpdatesNeeded (bool check_list);
 friend void doPendingPriorityCommands ();
diff --git a/rkward/rbackend/rkrinterface.cpp b/rkward/rbackend/rkrinterface.cpp
index 322071de..4dea3b49 100644
--- a/rkward/rbackend/rkrinterface.cpp
+++ b/rkward/rbackend/rkrinterface.cpp
@@ -401,7 +401,7 @@ void RInterface::handleRequest (RBackendRequest* request) {
 		RK_ASSERT(request->subcommandrequest);
 		command_requests.append(request->subcommandrequest);
 		request->subcommandrequest = nullptr;  // it is now a separate request. Make sure we won't try to send it back as part of this one.
-		processHistoricalSubstackRequest(request->params["call"].toStringList(), parent, request);
+		processHistoricalSubstackRequest(request->params["call"].toString(), request->params.value("args"), parent, request);
 		RKRBackendProtocolFrontend::setRequestCompleted(request);
 	} else if (request->type == RBackendRequest::PlainGenericRequest) {
 		request->setResult(processPlainGenericRequest(request->params["call"].toStringList()));
@@ -672,7 +672,7 @@ GenericRRequestResult RInterface::processPlainGenericRequest(const QStringList &
 	return GenericRRequestResult();
 }
 
-void RInterface::processHistoricalSubstackRequest (const QStringList &calllist, RCommand *parent_command, RBackendRequest *request) {
+void RInterface::processHistoricalSubstackRequest (const QString &call, const QVariant &args, RCommand *parent_command, RBackendRequest *request) {
 	RK_TRACE (RBACKEND);
 
 	RCommandChain *in_chain;
@@ -686,62 +686,68 @@ void RInterface::processHistoricalSubstackRequest (const QStringList &calllist,
 	in_chain = openSubcommandChain (parent_command);
 	RK_DEBUG (RBACKEND, DL_DEBUG, "started sub-command chain (%p) for command %s", in_chain, qPrintable (parent_command->command ()));
 
-	QString call = calllist.value (0);
 	if (call == "sync") {
-		RK_ASSERT (calllist.count () >= 2);
-
-		for (int i = 1; i < calllist.count (); ++i) {
-			QString object_name = calllist[i];
+		QVariantList al = args.toList();  // added, removed, changed
+		RObjectList::getGlobalEnv()->updateFromR(in_chain, al.value(0).toStringList(), al.value(1).toStringList());
+		QStringList changed = al.value(2).toStringList();
+		RK_DEBUG(RBACKEND, DL_DEBUG, "symbols added %s, removed %s, changed %s", qPrintable(al.value(0).toStringList().join(",")), qPrintable(al.value(1).toStringList().join(",")), qPrintable(al.value(2).toStringList().join(",")));
+		for (int i = 0; i < changed.count (); ++i) {
+			QString object_name = changed[i];
 			RObject *obj = RObjectList::getObjectList ()->findObject (object_name);
 			if (obj) {
-				RK_DEBUG (RBACKEND, DL_DEBUG, "triggering update for symbol %s", object_name.toLatin1 ().data());
+				RK_DEBUG(RBACKEND, DL_DEBUG, "triggering update for symbol %s", qPrintable(object_name));
 				obj->markDataDirty ();
 				obj->updateFromR (in_chain);
 			} else {
-				RK_DEBUG (RBACKEND, DL_WARNING, "lookup failed for changed symbol %s", object_name.toLatin1 ().data());
+				RK_DEBUG (RBACKEND, DL_WARNING, "lookup failed for changed symbol %s", qPrintable(object_name));
 			}
 		}
-	} else if (call == "syncenvs") {
+		closeChain(in_chain);
+		return;
+	}
+	if (call == "syncenvs") {
+		QVariantList al = args.toList();  // envs, namespaces
 		RK_DEBUG (RBACKEND, DL_DEBUG, "triggering update of object list");
-		int search_len = calllist.value (1).toInt ();
-		RObjectList::getObjectList ()->updateFromR (in_chain, calllist.mid (2, search_len), calllist.mid (2 + search_len));
-	} else if (call == "syncglobal") {
-		RK_DEBUG (RBACKEND, DL_DEBUG, "triggering update of globalenv");
-		RObjectList::getGlobalEnv ()->updateFromR (in_chain, calllist.mid (1));
+		RObjectList::getObjectList ()->updateFromR (in_chain, al.value(0).toStringList(), al.value(1).toStringList());
+		closeChain(in_chain);
+		return;
+	}
+
+	// NOTE: all requests below are wrapped from R commands, and pass all arguments as stringlist (although that may worth changing
+	QStringList arglist = args.toStringList();
+	if (false) {  // syntax dummy
 #ifndef DISABLE_RKWINDOWCATCHER
 	// NOTE: WARNING: When converting these to PlainGenericRequests, the occasional "error, figure margins too large" starts coming up, again. Not sure, why.
  	} else if (call == "startOpenX11") {
-		RK_ASSERT (calllist.count () == 2);
-		RKWindowCatcher::instance ()->start (calllist.value (1).toInt ());
+		RK_ASSERT(arglist.count() == 1);
+		RKWindowCatcher::instance()->start(arglist.value(0).toInt());
  	} else if (call == "endOpenX11") {
-		RK_ASSERT (calllist.count () == 2);
-		RKWindowCatcher::instance ()->stop (calllist.value (1).toInt ());
+		RK_ASSERT(arglist.count() == 1);
+		RKWindowCatcher::instance()->stop(arglist.value (0).toInt());
 	} else if (call == "updateDeviceHistory") {
-		if (calllist.count () >= 2) {
-			RKWindowCatcher::instance ()->updateHistory (calllist.mid (1));
+		if (!arglist.isEmpty()) {
+			RKWindowCatcher::instance()->updateHistory(arglist);
 		}
 	} else if (call == "killDevice") {
-		RK_ASSERT (calllist.count () == 2);
-		RKWindowCatcher::instance ()->killDevice (calllist.value (1).toInt ());
+		RK_ASSERT(arglist.count() == 1);
+		RKWindowCatcher::instance()->killDevice(arglist.value(0).toInt());
 #endif // DISABLE_RKWINDOWCATCHER
 	} else if (call == "edit") {
-		RK_ASSERT (calllist.count () >= 2);
-
-		QStringList object_list = calllist.mid (1);
-		new RKEditObjectAgent (object_list, in_chain);
+		RK_ASSERT(!arglist.isEmpty());
+		new RKEditObjectAgent (arglist, in_chain);
 	} else if (call == "require") {
-		RK_ASSERT (calllist.count () == 2);
-		QString lib_name = calllist.value (1);
+		RK_ASSERT (!arglist.isEmpty());
+		QString lib_name = arglist.value(0);
 		KMessageBox::information (0, i18n ("The R-backend has indicated that in order to carry out the current task it needs the package '%1', which is not currently installed. We will open the package-management tool, and there you can try to locate and install the needed package.", lib_name), i18n ("Require package '%1'", lib_name));
 		RKLoadLibsDialog::showInstallPackagesModal (0, in_chain, QStringList(lib_name));
 	} else if (call == "doPlugin") {
-		if (calllist.count () >= 3) {
+		if (arglist.count () >= 2) {
 			QString message;
 			bool ok;
 			RKComponentMap::ComponentInvocationMode mode = RKComponentMap::ManualSubmit;
-			if (calllist[2] == "auto") mode = RKComponentMap::AutoSubmit;
-			else if (calllist[2] == "submit") mode = RKComponentMap::AutoSubmitOrFail;
-			ok = RKComponentMap::invokeComponent (calllist[1], calllist.mid (3), mode, &message, in_chain);
+			if (arglist[1] == "auto") mode = RKComponentMap::AutoSubmit;
+			else if (arglist[1] == "submit") mode = RKComponentMap::AutoSubmitOrFail;
+			ok = RKComponentMap::invokeComponent(arglist[0], arglist.mid(2), mode, &message, in_chain);
 
 			if (!message.isEmpty ()) {
 				request->setResult(GenericRRequestResult(QVariant(), ok ? message : QString(), !ok ? message : QString()));
@@ -750,7 +756,7 @@ void RInterface::processHistoricalSubstackRequest (const QStringList &calllist,
 			RK_ASSERT (false);
 		}
 	} else if (call == QStringLiteral ("output")) {
-		request->setResult(RKOutputDirectory::handleRCall(calllist.mid(1), in_chain));
+		request->setResult(RKOutputDirectory::handleRCall(arglist, in_chain));
 	} else {
 		request->setResult(GenericRRequestResult::makeError(i18n("Unrecognized call '%1'", call)));
 	}
diff --git a/rkward/rbackend/rkrinterface.h b/rkward/rbackend/rkrinterface.h
index 2f413cdd..72fb2a21 100644
--- a/rkward/rbackend/rkrinterface.h
+++ b/rkward/rbackend/rkrinterface.h
@@ -91,7 +91,7 @@ private:
 	} command_logfile_mode;
 
 /** helper function to handle backend requests that (may) involve running additional R-"sub"-commands. TODO; This should probably be merged with processRBackendRequest.*/
-	void processHistoricalSubstackRequest (const QStringList &calllist, RCommand *parent_command, RBackendRequest *request);
+	void processHistoricalSubstackRequest (const QString &call, const QVariant &args, RCommand *parent_command, RBackendRequest *request);
 /** helper function to handle the bulk backend of requests that do not involve running sub-commands */
 	GenericRRequestResult processPlainGenericRequest (const QStringList &calllist);
 /** helper function to handle backend requests that do not inolve running sub-commands. */
diff --git a/rkward/rbackend/rkrsupport.cpp b/rkward/rbackend/rkrsupport.cpp
index ee200bbc..1e2190f1 100644
--- a/rkward/rbackend/rkrsupport.cpp
+++ b/rkward/rbackend/rkrsupport.cpp
@@ -298,47 +298,53 @@ RKRShadowEnvironment* RKRShadowEnvironment::environmentFor(SEXP baseenvir) {
 	return environments[baseenvir];
 }
 
-QStringList RKRShadowEnvironment::diffAndUpdate() {
-	QStringList diffs;
-	QStringList removed;
+static bool nameInList(SEXP needle, SEXP haystack) {
+	int count = Rf_length(haystack);
+	for (int i = 0; i < count; ++i) {
+		if (!strcmp(R_CHAR(needle), R_CHAR(STRING_ELT(haystack, i)))) return true;
+	}
+	return false;
+}
+
+RKRShadowEnvironment::Result RKRShadowEnvironment::diffAndUpdate() {
+	Result res;
 
-	Rprintf("%p %p\n", baseenvir, shadowenvir);
-	// find the changed symbols, and copy them to the shadow environment
 	SEXP symbols = R_lsInternal(baseenvir, TRUE);
 	PROTECT(symbols);
 	int count = Rf_length(symbols);
+	SEXP symbols2 = R_lsInternal(shadowenvir, TRUE);
+	PROTECT(symbols2);
+	int count2 = Rf_length (symbols2);
+
+	// find the changed symbols, and copy them to the shadow environment
 	for (int i = 0; i < count; ++i) {
 		SEXP name = Rf_installChar(STRING_ELT(symbols, i));
 		PROTECT(name);
 		SEXP main = Rf_findVar(name, baseenvir);
 		SEXP cached = Rf_findVar(name, shadowenvir);
 		if (main != cached) {
-			Rf_defineVar(name, Rf_findVar(name, baseenvir), shadowenvir);
-			diffs.append(RKRSupport::SEXPToString(name));
+			Rf_defineVar(name, main, shadowenvir);
+			if (/*Rf_isNull(cached) && */ !Rf_isNull(main) && !nameInList(STRING_ELT(symbols, i), symbols2)) {
+				res.added.append(RKRSupport::SEXPToString(name));
+			} else {
+				res.changed.append(RKRSupport::SEXPToString(name));
+			}
 		}
 		UNPROTECT(1);
 	}
 
 	// find the symbols only in the shadow environment (those that were removed)
-	SEXP symbols2 = R_lsInternal(shadowenvir, TRUE);
-	PROTECT(symbols2);
-	int count2 = Rf_length (symbols2);
 	for (int i = 0; i < count2; ++i) {
-		bool found = false;
-		for (int j = 0; j < count; ++j) {
-			if (STRING_ELT(symbols, j) == STRING_ELT(symbols2, i)) {
-				found = true;
-				break;
-			}
-		}
-		if (!found) {
-			removed.append(RKRSupport::SEXPToString(Rf_installChar(STRING_ELT(symbols2, i))));
+		if (!nameInList(STRING_ELT(symbols2, i), symbols)) {
+			res.removed.append(RKRSupport::SEXPToString(Rf_installChar(STRING_ELT(symbols2, i))));
+			R_removeVarFromFrame(Rf_installChar(STRING_ELT(symbols2, i)), shadowenvir);
 		}
 	}
 
-	UNPROTECT(2);
+	UNPROTECT(2); // symbols, symbols2
 
-	RK_DEBUG(RBACKEND, DL_DEBUG, "changed %s\n", qPrintable(diffs.join(", ")));
-	RK_DEBUG(RBACKEND, DL_DEBUG, "removed %s\n", qPrintable(removed.join(", ")));
-	return diffs + removed;
+	RK_DEBUG(RBACKEND, DL_DEBUG, "added %s\n", qPrintable(res.added.join(", ")));
+	RK_DEBUG(RBACKEND, DL_DEBUG, "changed %s\n", qPrintable(res.changed.join(", ")));
+	RK_DEBUG(RBACKEND, DL_DEBUG, "removed %s\n", qPrintable(res.removed.join(", ")));
+	return res;
 }
diff --git a/rkward/rbackend/rkrsupport.h b/rkward/rbackend/rkrsupport.h
index 25c67fcb..177d296d 100644
--- a/rkward/rbackend/rkrsupport.h
+++ b/rkward/rbackend/rkrsupport.h
@@ -37,11 +37,18 @@ namespace RKRSupport {
 
 class RKRShadowEnvironment {
 public:
-	QStringList diffAndUpdate();
-	static RKRShadowEnvironment* environmentFor(SEXP baseenvir);
+	struct Result {
+		QStringList added;
+		QStringList removed;
+		QStringList changed;
+		bool isEmpty() const { return added.isEmpty() && removed.isEmpty() && changed.isEmpty(); };
+	};
+	Result diffAndUpdate();
+	static Result diffAndUpdate(SEXP envir) { return environmentFor(envir)->diffAndUpdate(); };
 private:
 	RKRShadowEnvironment(SEXP baseenvir, SEXP shadowenvir) : baseenvir(baseenvir), shadowenvir(shadowenvir) {};
 	~RKRShadowEnvironment();
+	static RKRShadowEnvironment* environmentFor(SEXP baseenvir);
 	SEXP baseenvir;
 	SEXP shadowenvir;
 	static QMap<SEXP, RKRShadowEnvironment*> environments;
diff --git a/rkward/rbackend/rpackages/rkward/DESCRIPTION b/rkward/rbackend/rpackages/rkward/DESCRIPTION
index 41e701f6..8f498d00 100755
--- a/rkward/rbackend/rpackages/rkward/DESCRIPTION
+++ b/rkward/rbackend/rpackages/rkward/DESCRIPTION
@@ -18,7 +18,7 @@ Authors at R: c(person(given="Thomas", family="Friedrichsmeier",
         role=c("aut")), person(given="the RKWard team",
         email="rkward-devel at kde.org", role=c("cre","ctb")))
 Version: 0.7.4
-Date: 2022-05-04
+Date: 2022-05-19
 RoxygenNote: 7.1.2
 Collate: 
     'base_overrides.R'
diff --git a/rkward/rbackend/rpackages/rkward/NAMESPACE b/rkward/rbackend/rpackages/rkward/NAMESPACE
index ebbb6171..2fb4c47d 100644
--- a/rkward/rbackend/rpackages/rkward/NAMESPACE
+++ b/rkward/rbackend/rpackages/rkward/NAMESPACE
@@ -34,10 +34,7 @@ export(.rk.set.meta)
 export(.rk.set.vector.mode)
 export(.rk.startPreviewDevice)
 export(.rk.try.get.namespace)
-export(.rk.unwatch.symbol)
 export(.rk.variables)
-export(.rk.watch.globalenv)
-export(.rk.watch.symbol)
 export(.rk.with.window.hints)
 export(RK)
 export(Sys.setlocale)



More information about the rkward-tracker mailing list