Project

General

Profile

Design #925

MachineInt or long for args which are indices (yet again)

Added by John Abbott over 7 years ago. Updated 3 days ago.

Status:
In Progress
Priority:
Normal
Assignee:
-
Category:
Safety
Target version:
CoCoALib-0.99880
Start date:
20 Sep 2016
Due date:
% Done:

50%

Estimated time:
Spent time:
2.65 h

Description

This matter has already been rasied in #89 and #124.
Much to my surprise in matrix.H there are several SetEntry fns which have MachineInt args.

Resolve the matter.

Related issues

Related to CoCoALib - Bug #89: MachineInt or long as fn arg type for indicesClosed2012-02-09

Related to CoCoALib - Feature #124: change long args in matrices into MachineInt (?)Rejected2012-04-04

Related to CoCoALib - Bug #830: Use MachineInt instead of long for params to ZeroMat, IdentityMat, MatByCols, MatByRowsClosed2015-12-01

Related to CoCoALib - Design #934: MachineInt: change semantics?In Progress2016-09-30

Related to CoCoALib - Design #581: C++14: MachineIntClosed2014-07-04

History

#1 Updated by John Abbott over 7 years ago

  • Related to Bug #89: MachineInt or long as fn arg type for indices added

#2 Updated by John Abbott over 7 years ago

  • Related to Feature #124: change long args in matrices into MachineInt (?) added

#3 Updated by John Abbott over 7 years ago

  • Priority changed from Normal to High

Evidently I am not fully decided in my own mind whether it better to use MachineInt for safety or long for simplicity and speed.

Today I'm inclined to the view that anyone who uses unsigned integral types gets what he deserves if things go wrong... perhaps not unlike people who use raw pointers?

Anyway, the current version of the code is at odds with the rejection of #124.
Rectify the situation!

#4 Updated by John Abbott over 7 years ago

  • Related to Bug #830: Use MachineInt instead of long for params to ZeroMat, IdentityMat, MatByCols, MatByRows added

#5 Updated by John Abbott over 7 years ago

Now I see that issue #830 went ahead even though it is incompatible with #89 and #124.
Oops!

What to do?

#6 Updated by John Abbott over 7 years ago

  • Status changed from New to In Progress
  • % Done changed from 0 to 10

After some reflection and after speaking to Anna...
I now think that MachineInt is wrong for indexes; the type MachineInt was originally conceived for avoiding problems with conversion from C++ integral types to RingElem.

The clean approach would be to invent a new type (say, IndexInt) for representing indexes, and presumably also for representing the size of indexable containers. Strictly there is a slight problem (with using IndexInt to specify the size) in that it is not possible to specify the very biggest indexable object since the largest such object is one greater than the largest representable index value (assuming we adopt the C++ approach of indexing from 0).

How confusing would it be to have both MachineInt and IndexInt (or whatever name we choose)?

#7 Updated by John Abbott over 7 years ago

  • Target version changed from CoCoALib-0.99550 spring 2017 to CoCoALib-0.99560

#8 Updated by John Abbott over 7 years ago

  • Related to Design #934: MachineInt: change semantics? added

#9 Updated by John Abbott over 6 years ago

  • Target version changed from CoCoALib-0.99560 to CoCoALib-0.99600

#10 Updated by John Abbott about 6 years ago

Should indices be constrained to be non-negative?
Note that symbol currently does allow negative "indices".

It would be good to decide this issue before too long!

#11 Updated by John Abbott almost 6 years ago

  • Target version changed from CoCoALib-0.99600 to CoCoALib-0.99650 November 2019

#12 Updated by John Abbott about 5 years ago

#13 Updated by John Abbott almost 5 years ago

  • Priority changed from High to Normal
  • Target version changed from CoCoALib-0.99650 November 2019 to CoCoALib-1.0

Postponing because there are too many other more pressing issues. Anyway the current code works adequately, so it is not that harmful to postpone.

#14 Updated by John Abbott about 3 years ago

  • % Done changed from 10 to 50

I checked that NTL happily uses long for indices into matrices.
So it is reasonable for us to use long for "indices" (perhaps in a wider sense).

I have just updated the code for symbol, and it has become slightly simpler/cleaner.

#15 Updated by John Abbott about 3 years ago

  • Target version changed from CoCoALib-1.0 to CoCoALib-0.99850

#16 Updated by John Abbott 3 months ago

  • Target version changed from CoCoALib-0.99850 to CoCoALib-0.99880

#17 Updated by John Abbott 3 days ago

Indices into matrices (and other indexable objects, e.g. ModuleElem?) must be non-negative.
As already noted indices for symbols may be negative -- and the implementation uses long.

Proposal: use long for indexes into matrices (& other indexable data-structures):
  • PRO coherent with indexes for symbol
  • CON technically limits max size (but not a problem in practice); not coherent with C++ indexable data-structures
Proposal: use unsigned long or size_t for indexes into matrices (& other indexable data-structures):
  • PRO coherent with C++; unimportantly does not limit max size
  • CON not coherent with symbol

Note that if the caller tries to supply a negative index when the parameter type is unsigned long then it will still be recognized as "out of range" --- and a heuristic could also indicate that it was probably negative, but do we really want to distinguish "negative index" from "out of range"? When would the distinction ever be useful?

Also available in: Atom PDF