public inbox for gsl-discuss@sourceware.org
 help / color / mirror / Atom feed
* const gsl_vector *
@ 2014-08-13  0:03 Gerard Jungman
  2014-08-19  9:19 ` John D Lamb
  0 siblings, 1 reply; 6+ messages in thread
From: Gerard Jungman @ 2014-08-13  0:03 UTC (permalink / raw)
  To: gsl-discuss

[-- Attachment #1: Type: text/plain, Size: 4760 bytes --]

The constness of gsl_vector interfaces makes no sense. This is a
simple observation, but it seems to have escaped discussion for
over a decade. Consider the following code (file attached).

  #include <gsl/gsl_vector.h>

  void notwhatyouthink(const gsl_vector * v)
  {
    v->data[0] = 42.0;
  }

  int main()
  {
    gsl_vector * v = gsl_vector_calloc(32);

    printf("%g\n", v->data[0]);
    notwhatyouthink(v);
    printf("%g\n", v->data[0]);

    gsl_vector_free(v);

    return 0;
  }

Obviously, the constness of the vector is not the same as the
constness of the data. But the interface leads you to believe
the function is safe. Every function in GSL which takes a
(gsl_vector *) argument is declared in precisely this way,
either with or without the 'const', depending on the
obvious intent. But the compiler knows nothing about
the obvious intent, so it has no objection to the
example code, which is obviously not good.

This is a rookie error. I don't know how it happened. But things
like this force me to question my own sanity. Was it intended to
be this way? I don't think so. Am I missing something about the
overall design which explains why it is this way? Probably not.


The "view" objects do the more correct thing, introducing two
distinct types, with designed const-correctness. You can see
how the constness was an unavoidable issue there, because
views are indirect by design.


As I was thinking about fixes for the containers, I was trying
to preserve interfaces as much as possible. In particular, I
was trying to preserve the "gsl_vector *" and "const gsl_vector *"
argument types in function prototypes. This led me to question the
semantics of these interfaces as they are, which led to the above
stupid observation.

I then wondered what GSL interfaces for view objects look like,
and discovered something interesting. No GSL interface (except
the view headers themselves) uses a view object. Not one.
So why do views exist? It seems that they are used by GSL
implementations in lots of places. So, by construction
(or historical accident, depending your emphasis), the
views are essentially an implementation detail. Yet
they are structured as if they are for client use.
Very odd.


It seems like the whole thing is standing on its head. The
views should be the main client interface. GSL interfaces
should be written in terms of views and should have true
const-correctness. The memory management inherent in the
container objects (as distinct from the views) should
be properly factored out.

The const-incorrectness of the "gsl_vector *" interfaces
is just one important symptom of this deep disease.


The question: What to do?
There are several options, none very enticing.

1) Punt on const-correctness and the other issues with the
    current design. Try to wrap the oozing wounds in some
    bandages and forget about it.

2) Completely re-design the guts of containers while preserving
    the current interfaces. This can be done, in such a way as
    to bring views to the forefront and rationalize the design.
    But it punts on the interface issues, including the
    constness problem.

3) Re-design the guts of the containers and change all the
    GSL interfaces to conform to a rationalized and const-correct
    container architecture. This would change everything in the
    world that ever touches a GSL container.

    It also implies a near doubling of interfaces, if we follow the
    lead of the current view design. Doubling the interfaces just
    because some objects have a 'const' keyword somewhere is
    really disgusting.

4) Find some magic way to implement choice (3) without a need
    to double the interfaces. Try to get more value semantics
    (rather than the current pointer semantics) into the
    picture. Views currently have a value semantic; they
    live on the stack. Vectors should really be views,
    and should have similar semantics.


You can mix and match the various ideas to create other
approaches beyond these four.

I have spent a lot of time looking for C container library designs
that were relevant for numerical containers and solved some or all
of these problems in some way. I have found none. There are lots
of container libraries, like the glib containers, etc. But these
are just irrelevant for GSL. Trust me, if you don't want to look
for yourself.

Yet, I feel that there are solutions for these problems. They may
involve some casting tricks, but I don't think there has to any
compromise of safety. There will, however, have to be some
compromises.

The goals:
   - const-correctness
   - no "doubling" of interfaces
   - an architecture where views are central
   - memory management factored out (allocation/deallocation)

Ideas?

--
G. Jungman


[-- Attachment #2: gsl-vector-constness.c --]
[-- Type: text/x-csrc, Size: 274 bytes --]

#include <gsl/gsl_vector.h>

void notwhatyouthink(const gsl_vector * v)
{
  v->data[0] = 42.0;
}

int main()
{
  gsl_vector * v = gsl_vector_calloc(32);

  printf("%g\n", v->data[0]);
  notwhatyouthink(v);
  printf("%g\n", v->data[0]);

  gsl_vector_free(v);

  return 0;
}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: const gsl_vector *
  2014-08-13  0:03 const gsl_vector * Gerard Jungman
@ 2014-08-19  9:19 ` John D Lamb
  2014-08-19 22:28   ` Patrick Alken
  2014-08-19 22:33   ` Patrick Alken
  0 siblings, 2 replies; 6+ messages in thread
From: John D Lamb @ 2014-08-19  9:19 UTC (permalink / raw)
  To: gsl-discuss



On 13/08/14 01:03, Gerard Jungman wrote:
> The constness of gsl_vector interfaces makes no sense. This is a
> simple observation, but it seems to have escaped discussion for
> over a decade. Consider the following code (file attached).
>
>   #include <gsl/gsl_vector.h>
>
>   void notwhatyouthink(const gsl_vector * v)
>   {
>     v->data[0] = 42.0;
>   }
>
> The goals:
>    - const-correctness
>    - no "doubling" of interfaces
>    - an architecture where views are central
>    - memory management factored out (allocation/deallocation)
>
> Ideas?

I mainly use C++. But I think C now works so that you could use

gsl_vector* const v

in place of a

const gsl_vector* v

argument, and get the desired result.

I donÂ’t know if there would be unintended consequences of rewriting the 
arguments of the GSL functions this way. I know that C canÂ’t handle both of

int function( const gsl_vector* v );
int function( gsl_vector* const v );

in the same code. So it would need a rewrite.

One issue would be that if the code were rewritten any code that passed 
a const gsl_vector* argument would cause a compilation failure. The 
workaround might be to use a

gsl_vector const* const v

argument. But I donÂ’t know if that would cause problems, for example 
with older compilers.

-- 
John D Lamb

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: const gsl_vector *
  2014-08-19  9:19 ` John D Lamb
@ 2014-08-19 22:28   ` Patrick Alken
  2014-08-20  6:37     ` John D Lamb
  2014-08-19 22:33   ` Patrick Alken
  1 sibling, 1 reply; 6+ messages in thread
From: Patrick Alken @ 2014-08-19 22:28 UTC (permalink / raw)
  To: gsl-discuss

On 08/19/2014 03:18 AM, John D Lamb wrote:
>
> I mainly use C++. But I think C now works so that you could use
>
> gsl_vector* const v
>
> in place of a
>
> const gsl_vector* v
>
> argument, and get the desired result.
>
> I donÂ’t know if there would be unintended consequences of rewriting the
> arguments of the GSL functions this way. I know that C canÂ’t handle both of
>
> int function( const gsl_vector* v );
> int function( gsl_vector* const v );
>
> in the same code. So it would need a rewrite.
>
> One issue would be that if the code were rewritten any code that passed
> a const gsl_vector* argument would cause a compilation failure. The
> workaround might be to use a
>
> gsl_vector const* const v
>
> argument. But I donÂ’t know if that would cause problems, for example
> with older compilers.
>

I don't have a solution to this problem, but with my gcc (4.4.7), 
changing to gsl_vector* const doesn't fix the issue. Compiling with 
-Wall -W produces no warnings and the program runs as before.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: const gsl_vector *
  2014-08-19  9:19 ` John D Lamb
  2014-08-19 22:28   ` Patrick Alken
@ 2014-08-19 22:33   ` Patrick Alken
  2014-08-27  0:08     ` Gerard Jungman
  1 sibling, 1 reply; 6+ messages in thread
From: Patrick Alken @ 2014-08-19 22:33 UTC (permalink / raw)
  To: gsl-discuss

Also, the compiler should issue warnings/errors if you try to change 
anything else in the gsl_vector container. And since gsl_vector_set is 
the preferred way to set data elements, it will certainly detect a 
problem if you try to call gsl_vector_set on a const gsl_vector*

I think the main purpose of the current design was to abstract away all 
the details of gsl_vector, so in principle the user will never need to 
directly access the variables in v or directly access v->data. In 
practice, however, sometimes it is necessary to directly use those 
parameters.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: const gsl_vector *
  2014-08-19 22:28   ` Patrick Alken
@ 2014-08-20  6:37     ` John D Lamb
  0 siblings, 0 replies; 6+ messages in thread
From: John D Lamb @ 2014-08-20  6:37 UTC (permalink / raw)
  To: gsl-discuss

On 19/08/14 23:28, Patrick Alken wrote:
>> One issue would be that if the code were rewritten any code that passed
>> a const gsl_vector* argument would cause a compilation failure. The
>> workaround might be to use a
>>
>> gsl_vector const* const v
>>
>> argument. But I donÂ’t know if that would cause problems, for example
>> with older compilers.
>>
> I don't have a solution to this problem, but with my gcc (4.4.7),
> changing to gsl_vector* const doesn't fix the issue. Compiling with
> -Wall -W produces no warnings and the program runs as before.

Actually my suggested solution wonÂ’t work. A function declaration like

void f( gsl_vector *const v );

is treated by the compiler as  equivalent to

void f( gsl_vector * v );

and itÂ’s possible to use the second form in a declaration and the first 
in a definition.

The only compiler advantage of the first form is in a function 
definition: you guarantee that v (though not what it points to) wonÂ’t be 
changed. In a declaration it does no more than hint to the user of the 
function that the data wonÂ’t be changed.

Although the gsl_vector functions canÂ’t offer a guarantee not to change 
a const gsl_vector * argument, IÂ’m not sure this is a problem. In 
practice they donÂ’t; so there are no surprises for the user.

I think the root problem is this. Neither C nor C++ has a way to declare 
a struct const so that the data pointed to by any pointer in the struct 
is also const. There are workarounds. But I donÂ’t know of any that donÂ’t 
involve copying data, which is an unnecessary overhead when the only 
real requirement is to reassure a function user that nothing pointed to 
by a function parameter will be changed.

-- 
John D Lamb

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: const gsl_vector *
  2014-08-19 22:33   ` Patrick Alken
@ 2014-08-27  0:08     ` Gerard Jungman
  0 siblings, 0 replies; 6+ messages in thread
From: Gerard Jungman @ 2014-08-27  0:08 UTC (permalink / raw)
  To: gsl-discuss

The discussion seems to have died out, but I was glad for
the handful of replies that did come.


On 08/19/2014 04:33 PM, Patrick Alken wrote:

> Also, the compiler should issue warnings/errors if you try to change 
> anything else in the gsl_vector container. And since gsl_vector_set is 
> the preferred way to set data elements, it will certainly detect a 
> problem if you try to call gsl_vector_set on a const gsl_vector*

There is a (limited) sense in which this is the right answer.
Consider C++, where the access control happens by declaring
the accessor member functions with a 'const' attribute.
By analogy, controlling the "member" functions in C
gives some control over the access.

But this is not very satisfying, for two reasons. First, the
language does not provide compiler-enforced access control,
so there is never a guarantee. We just have to live with that.
Second, and more important, is that the current design is
an illusion.

   void   gsl_vector_set (gsl_vector * v, const size_t i, double x);
   double gsl_vector_get (const gsl_vector * v, const size_t i);

They look nice. And they seem to declare a contract with the
client, since one of them has the 'const' keyword and the
other one does not. As you say, the compiler can detect
something about this.

But as we have seen, it is an illusion. The fact that the 'const'
keywords are "sort of in the right place" is just misleading.
That the compiler can detect anything at all here is
basically an accident.

> I think the main purpose of the current design was to abstract away 
> all the details of gsl_vector, so in principle the user will never 
> need to directly access the variables in v or directly access v->data. 
> In practice, however, sometimes it is necessary to directly use those 
> parameters.

Not necessarily. The abstraction is ok, up to a point. But the
current abstraction is too fat. It tries to do too much, notably
it balls up the memory management with the basic implementation,
making it impossible to separate the simple indexing semantics
from the more client-specific memory management.

This is where the gsl_vector discussion began some months ago,
with the need to factor the memory management out of the objects.

My first suggestion was to allow clients to pass alloc/free
functions to the initialization functions. But, after a little
thought, I realized this was not right. First, it smells bad,
because any sort of framework design is unworkable in the
long run. Second, because the containers should really be
lighter, not heavier with yet more memory management semantics.

So the right thing must be to lighten the containers, making
them essentially simple handles. They become "view" types,
which can be initialized along with allocation (as they are
now), or by simply wrapping existing storage.

All GSL interfaces should take the light-weight views as
arguments, since that is the most flexible choice. The
name 'gsl_vector' need not change, so there would appear
to be no need to change the prototypes for existing interfaces...
EXCEPT for the apparent need for a second 'const' type to get
the needed const-correctness for views initialized with
existing const storage.

This is where we are now. Const-correctness seems to require
a second type and therefore changes to existing interfaces.


I looked at the C interfaces for numpy ndarray objects, to
see what they do. It seems that const-ness is simply lost
through those interfaces. As long as you use them through
python, it doesn't matter. But if you try to export a
C array through the numpy C interfaces, somewhere the
constness must be discarded.

The ndarray struct (and the python type) carries a flag
called WRITEABLE, which is some kind of half-baked substitute
for mutability/immutability. It can generate runtime errors
under some circumstances, but does not constitute any form
of guarantee.

I looked at "typed memoryview" from cython. This is a good
thing, in its domain. But it seems like it is just a nicer
version of the python standard PEP 3118, which is the
basis for numpy ndarray.

Somebody also suggested I look at Datashape and Blaze. This
is not so clear to me. At least I didn't see how it could
help either.


So, none of these external designs seem to help much. The
only thing they seem to suggest is that constness is sort
of optional in that world...


Is const-correctness optional or not?


If clients choose to wrap existing data in a gsl_vector, are
they also willing to cast away any constness? It sounds
very wrong to me, but if you held a gun to my head...

--
G. Jungman

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-08-27  0:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-13  0:03 const gsl_vector * Gerard Jungman
2014-08-19  9:19 ` John D Lamb
2014-08-19 22:28   ` Patrick Alken
2014-08-20  6:37     ` John D Lamb
2014-08-19 22:33   ` Patrick Alken
2014-08-27  0:08     ` Gerard Jungman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).