Deprecate and phase out use of `DeclareAutoreadableVariables` / `AUTO`
These "automatic" variables are a rather obscure GAP feature that few people know. I'd argue that "magic" happening just because one inspects a variable is not a good design. I can totally see why one might think it's a great idea initially, but ultimately I don't think it warrants the added complexity for users, developers, and maintainers of the language.
GAP itself does not use this. As far as I can tell, only these packages do, and I propose we try to eliminate this usage from those packages over time.
- [ ] atlasrep
- [ ] browse
- [ ] ctbllib
- [ ] liealgdb -- https://github.com/gap-packages/liealgdb/issues/11
- [ ] smallsemi -- https://github.com/gap-packages/smallsemi/issues/58
- [ ] sonata -- https://github.com/gap-packages/sonata/issues/33
To illustrate how one could replace it, here are a few concrete examples:
DeclareAutoreadableVariables( "atlasrep", "gap/mindeg.g",
[ "MinimalRepresentationInfoData" ] );
This ensures the first access to the variable MinimalRepresentationInfoData results in the atlasrep file gap/mindeg.g being read. Afterwards MinimalRepresentationInfoData is a plain GAP record.
An easy alternative to this would be to replace MinimalRepresentationInfoData by a function which returns said record. Then this function is free to have logic at the start of the form if not is_data_loaded then load_data(); fi;.
The main problem with this kind of change is if there is code in other packages accessing the auto-variables (so here: MinimalRepresentationInfoData). Luckily this is not the case here.
As an example, here is a patch that avoids use of DeclareAutoreadableVariables in browse. Perhaps @frankluebeck and @ThomasBreuer would be willing to accept that as a patch there and make a new release?
From d1b1cee44f4aefb472ad53ac3e50aba42d98763b Mon Sep 17 00:00:00 2001
From: Max Horn <[email protected]>
Date: Wed, 24 Sep 2025 01:38:11 +0200
Subject: [PATCH] Avoid use of DeclareAutoreadableVariables
---
app/tmdbatt2.g | 4 ++--
app/tmdbattr.g | 14 ++++++++++++--
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/app/tmdbatt2.g b/app/tmdbatt2.g
index 7fe0885..52ffaef 100644
--- a/app/tmdbatt2.g
+++ b/app/tmdbatt2.g
@@ -285,8 +285,8 @@ DatabaseAttributeAdd( TomLibData.IdEnumerator, rec(
##
#T Admit an argument, as in BrowseTransitiveGroupsInfo?
##
-BindGlobal( "BrowseTomLibInfo", function()
- NCurses.BrowseGeneric(
+BindGlobal( "_BrowseTomLibInfo", function()
+ NCurses.BrowseGeneric(
BrowseTableFromDatabaseIdEnumerator( TomLibData.IdEnumerator,
[ "self" ],
[ "Size", "NrClasses", "FusionsFrom", "FusionsTo", "Filename" ],
diff --git a/app/tmdbattr.g b/app/tmdbattr.g
index 779e6ed..4a5c388 100644
--- a/app/tmdbattr.g
+++ b/app/tmdbattr.g
@@ -76,8 +76,18 @@
## </ManSection>
## <#/GAPDoc>
##
-DeclareAutoreadableVariables( "Browse", "app/tmdbatt2.g",
- [ "BrowseTomLibInfo" ] );
+BindGlobal( "BrowseTomLibInfo", function()
+ if not IsBoundGlobal(" _BrowseTomLibInfo" ) then
+ # Avoid nested calls to `RereadPackage',
+ # which could cause that `REREADING' is set to `false' too early.
+ if REREADING then
+ ReadPackage( "Browse", "app/tmdbatt2.g" );
+ else
+ RereadPackage( "Browse", "app/tmdbatt2.g" );
+ fi;
+ fi;
+ ValueGlobal("_BrowseTomLibInfo" )();
+end );
#############################################################################
--
2.51.0
O.k., I can remove the use of DeclareAutoreadableVariables in the packages where I am involved.
(On the other hand, it is sad that one has to adjust packages whenever some nice feature gets classified as bad design.)