-
Notifications
You must be signed in to change notification settings - Fork 160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
kernel: catch some invalid uses of '~'; add tests #2750
Conversation
Also, print a (more) helpful error message. Before: gap> ~; Error, '~' does not have a value here not in any function at *stdin*:1 gap> x -> ~; function( x ) ... end gap> x -> (1,~); function( x ) ... end gap> (1,~); Error, '~' does not have a value here not in any function at *stdin*:4 After: gap> ~; Syntax error: '~' not allowed here ~; ^ gap> x -> ~; Syntax error: '~' not allowed here x -> ~; ^ gap> x -> (1,~); Syntax error: '~' not allowed here x -> (1,~); ^ gap> (1,~); Syntax error: '~' not allowed here (1,~); ^
Just to clarify: this is far from perfect, e.g. this is still accepted: |
The more I think about it, the less sure am I whether we should do it... This is related to the somewhat unclear definition of what Perhaps allowing The tilde operator is really useful in some cases, but also under documented (actually... is there any documentation for it? where?); and it very tightly interlaces the reader, interpreter, coder/executor and compiler. I tried to isolate # global record to hold most data and functions of this section
BindGlobal("Sudoku", rec(
# we count fields of board rowwise from 1..81; first collect relevant
# indices in row, column and subsquare for each entry
rows := List([0..8], c-> c*9+[1..9]),
cols := List([1..9], i-> i+[0,9..72]),
squs := List([1,4,7,28,31,34,55,58,61], i-> i-1+[1,2,3,10,11,12,19,20,21]),
rcsind := List([1..81], i-> [QuoInt(i-1,9)+1, (i-1) mod 9 + 1,
First([1..9], j-> i in ~.squs[j])]),
inds := List(~.rcsind, ind -> Set(Concatenation(~.rows[ind[1]],
~.cols[ind[2]], ~.squs[ind[3]])))
)); But this can be rewritten quite easily to avoid that. E.g.: BindGlobal("Sudoku", rec(
# we count fields of board rowwise from 1..81; first collect relevant
# indices in row, column and subsquare for each entry
rows := List([0..8], c-> c*9+[1..9]),
cols := List([1..9], i-> i+[0,9..72]),
squs := List([1,4,7,28,31,34,55,58,61], i-> i-1+[1,2,3,10,11,12,19,20,21]),
));
Sudoku.rcsind := List([1..81], i-> [QuoInt(i-1,9)+1, (i-1) mod 9 + 1,
First([1..9], j-> i in Sudoku.squs[j])]);
Sudoku.inds := List(Sudoku.rcsind, ind -> Set(Concatenation(Sudoku.rows[ind[1]],
Sudoku.cols[ind[2]], Sudoku.squs[ind[3]]))); or in many other ways. At worst, one can convert any code using ~ to code which doesn't via a trick: change BindGlobal("Sudoku", rec(
# we count fields of board rowwise from 1..81; first collect relevant
# indices in row, column and subsquare for each entry
rows := List([0..8], c-> c*9+[1..9]),
cols := List([1..9], i-> i+[0,9..72]),
squs := List([1,4,7,28,31,34,55,58,61], i-> i-1+[1,2,3,10,11,12,19,20,21]),
rcsind := (r->List([1..81], i-> [QuoInt(i-1,9)+1, (i-1) mod 9 + 1,
First([1..9], j-> i in r.squs[j])]))(~),
inds := (r->List(r.rcsind, ind -> Set(Concatenation(r.rows[ind[1]],
r.cols[ind[2]], r.squs[ind[3]]))))(~)
)); The only other example of code using this "feature" is in the lpres package: return rec(group := TrivialSubgroup(Q),
quo := GroupHomomorphismByFunction(G,~.group,x->One(~.group)),
set := X,
top := Q,
wreath := WreathProduct(~.group,Q),
epi := GroupHomomorphismByImages(~.wreath,~.group,GeneratorsOfGroup(~.wreath),List(GeneratorsOfGroup(~.wreath),x->One(Q)))); It's clear how to change this code away from using |
I'm happy with these fixes. My feeling on tilde in general is that it is (as you say) horribly underspecified and underdocumented. However, I don't think it's worth the work to either (a) specify it better and handle every horrible corner case, or (b) remove it. As you showed, both in packages (and I've seen in code in the wild) some weird uses, which I'd prefer not to break unless we have to. If at some point it arises some new feature / optimisation we want to add is blocked by Tilde, then I'm happy to re-evaluate the pain to make substansal changes, but for now I'd just leave it as a minor feature, applying (like this PR) minor fixes and improves where necessary, and trying not to break it. |
Also, print a (more) helpful error message.
Before:
After: