AdaCore / ada-spark-rfcs

Platform to submit RFCs for the Ada & SPARK languages
63 stars 28 forks source link

[RFC]: Simpler accessibility rules #47

Closed yannickmoy closed 1 year ago

yannickmoy commented 4 years ago

Full text here: https://github.com/yannickmoy/ada-spark-rfcs/blob/accessibility/considered/rfc-simpler-accessibility.md

mosteo commented 4 years ago

I've been perplexed a few times by accessibility failures at runtime, so moving checks to compile-time only is a win in my book.

yannickmoy commented 4 years ago

I've been perplexed a few times by accessibility failures at runtime, so moving checks to compile-time only is a win in my book.

That's the heart of this proposal. But in some cases you may need to use Unchecked_Access, and when that happens, you're on your own to make sure memory is properly handled.

jere-software commented 4 years ago

NOTE: This probably sounds more harsh than I actually mean it. It's not meant be a "downer", but I do have concerns on this one.

I definitely like having more compile time checking and I agree the anonymous access rules are too difficult to wrap your head around easily (or at all in some cases), but I feel like this might be a huge step backwards for Ada, especially in the current time/context where compile time checking of dangling references/pointers has been mostly solved.

I would have preferred to see Ada move to compile time checking while also being able to allow for object based lifetimes. One of the big issues with the Ada 2012 rules is the idea that we try to tie the accessibility level to the the return object from a function. I feel like this is completely backwards and leads to a lot of the crazy rule sets that were added to the heart of darkness. The accessibility of an object can be tracked by the compiler and I don't feel the rules have to be terribly convoluted for it.

If we completely forgo object based accessibility, then I feel like that puts Ada solidly in the "doesn't have pointer safety built in" group, even if it technically still has rules (they would just be impractical rules). At that point your options are to use what are essentially global variables (which is already awful for named access types) or just use Unchecked_Access all over the place (which means the rules that do exist aren't really useful). Things like bounded containers no longer have any chance of pointer/reference/access safety without giving up things like modularity, readability, maintainability, and reusability (which are given up depends on the solution chosen).

yannickmoy commented 4 years ago

I would have preferred to see Ada move to compile time checking while also being able to allow for object based lifetimes.

We feel it's not possible to do both at the same time. That's why this is only a first step, and the next is to look at other means to ensure memory safety, as outlined at the end of the RFC. Note that these two (or more) steps need only be separate while we're experimenting with GNAT. Once we've reached a better stage, we could propose the new complete solution for standardization.

yannickmoy commented 4 years ago

Things like bounded containers no longer have any chance of pointer/reference/access safety without giving up things like modularity, readability, maintainability, and reusability (which are given up depends on the solution chosen).

Are you referring to container implementation or container usage? If the former, see the current implementation of bounded containers in GNAT standard library, which is full of Unchecked_Access and even Unrestricted_Access for good reasons. If the latter, can you explain why you feel this way? Maybe an example would help.

jere-software commented 4 years ago

I would have preferred to see Ada move to compile time checking while also being able to allow for object based lifetimes.

We feel it's not possible to do both at the same time. That's why this is only a first step, and the next is to look at other means to ensure memory safety, as outlined at the end of the RFC. Note that these two (or more) steps need only be separate while we're experimenting with GNAT. Once we've reached a better stage, we could propose the new complete solution for standardization.

Can you give more context to this? Why do you feel it is not possible to do both at the same time? Languages like Rust have worked it out pretty neatly.

jere-software commented 4 years ago

Things like bounded containers no longer have any chance of pointer/reference/access safety without giving up things like modularity, readability, maintainability, and reusability (which are given up depends on the solution chosen).

Are you referring to container implementation or container usage? If the former, see the current implementation of bounded containers in GNAT standard library, which is full of Unchecked_Access and even Unrestricted_Access for good reasons. If the latter, can you explain why you feel this way? Maybe an example would help.

Implementing complex objects like containers so that others don't accidentally misuse them.

Take the 9.2.0 cursor implementation for Bounded_Doubly_Linked_Lists. It uses a named access type as a component of the Cursor type and Unrestricted_Access to assign to it. You can have dangling cursors with this implementation. Even if you turn on assertions (which you don't want to do for release code since the Vet operation is really intense), the Vet operation cannot catch all instances of dangling references. This can't be called dangling reference safe. It might have a high probability of dangling reference detection, but it certainly does not have a probability of detection of 100%, so it is still unsafe to use from the context of dangling references (cursors in this case).

yannickmoy commented 4 years ago

Can you give more context to this? Why do you feel it is not possible to do both at the same time? Languages like Rust have worked it out pretty neatly.

I said precisely that we want to do that in steps during our experiments. The solution that was adopted in Rust did not appear in one go. And the version of ownership that we adopted in SPARK also took three years to converge. Ada (and SPARK) have a number of specificities, in particular the ability to allocate data on the stack and point to it, that need an original solution.

yannickmoy commented 4 years ago

Take the 9.2.0 cursor implementation for Bounded_Doubly_Linked_Lists. It uses a named access type as a component of the Cursor type and Unrestricted_Access to assign to it. You can have dangling cursors with this implementation. Even if you turn on assertions (which you don't want to do for release code since the Vet operation is really intense), the Vet operation cannot catch all instances of dangling references. This can't be called dangling reference safe. It might have a high probability of dangling reference detection, but it certainly does not have a probability of detection of 100%, so it is still unsafe to use from the context of dangling references (cursors in this case).

Precisely, you are talking about the current implementation here, which is based on the current rules. We feel that we need to simplify the current rules before we can make them richer with object ownership or other safe dynamic memory management mechanisms. Note that a 100% safe solution will incur a cost in terms of usage flexibility. You won't be able to have aliasing (between a cursor and a container) and allow mutation through multiple handles to the same memory location, which is what Ada allows currently with the API of containers and cursors.

raph-amiard commented 4 years ago

Hi @jeremiahbreeden, I want to bring my own "clarification" to what Yannick said: For us this is a stepping stone, more than an end goal: We want to have pointer safety in Ada ultimately. Getting rid of dynamic accessibility rules can be justified several ways in that aspect:

  1. We want to get rid of existing rules that we consider sub-optimal, and that will cause a combinatory explosion when trying to add new rules. Before enhancing the status quo, we want to simplify.

  2. Dynamic accessibility is a very complex part of Ada that, to the best of our knowledge, nobody completely understands when it comes to writing code, so people very often fall back on using unsafe accesses. The lesson to drag from your examples from Ada containers, which use unsafe accesses, is that it's not really usable for the problems that we want to solve. In fact, the experience we had at AdaCore, is that we implemented missing safety checks wrt. dynamic accessibility, and suddenly a lot of our code stopped working and had to stop using anonymous accesses.

Our long term goal is to have a model similar to Rust, with:

  1. Statically verified lifetime verification for cases where it applies.
  2. Refcounted pointers for cases where 1. doesn't apply and you don't care about the performance penalty of reference counting.
  3. Unsafe accesses of some sort for the (hopefully small) remaining use cases.

Hope that clarifies things for you :slightly_smiling_face:

jere-software commented 4 years ago

Can you give more context to this? Why do you feel it is not possible to do both at the same time? Languages like Rust have worked it out pretty neatly.

I said precisely that we want to do that in steps during our experiments. The solution that was adopted in Rust did not appear in one go. And the version of ownership that we adopted in SPARK also took three years to converge. Ada (and SPARK) have a number of specificities, in particular the ability to allocate data on the stack and point to it, that need an original solution.

I just wanted to note here that I wasn't referring to the ownership model in Rust (in case I caused confusion here). That isn't what gives dangling reference safety. It more targets things like thread safety (compile time prevention of data races for example) and ensuring good design practices. Rust's lifetime model is what prevents dangling reference issues.

My question wasn't so much focused on the intended progression plan, but you had mentioned you thought it was not possible to have compile time checking and object based lifetimes at the same time. There is quite a wide area between "not possible" and "easier to do in steps". I get the latter, but you had said the former, which was the source of my confusion.

Either way, I definitely don't want to spark an argument (I'm worried I have potentially waded into that territory), so I'll just drop my question. I apologize for the inconvenience.

jere-software commented 4 years ago

Take the 9.2.0 cursor implementation for Bounded_Doubly_Linked_Lists. It uses a named access type as a component of the Cursor type and Unrestricted_Access to assign to it. You can have dangling cursors with this implementation. Even if you turn on assertions (which you don't want to do for release code since the Vet operation is really intense), the Vet operation cannot catch all instances of dangling references. This can't be called dangling reference safe. It might have a high probability of dangling reference detection, but it certainly does not have a probability of detection of 100%, so it is still unsafe to use from the context of dangling references (cursors in this case).

Precisely, you are talking about the current implementation here, which is based on the current rules. We feel that we need to simplify the current rules before we can make them richer with object ownership or other safe dynamic memory management mechanisms. Note that a 100% safe solution will incur a cost in terms of usage flexibility. You won't be able to have aliasing (between a cursor and a container) and allow mutation through multiple handles to the same memory location, which is what Ada allows currently with the API of containers and cursors.

Only if you choose to do both lifetime and ownership models. I think you only need the lifetime model for dangling reference protection. That eliminates the aliasing and multiple handles issues.

In current Ada, for my own containers, I make use of anonymous access types for iterators, since you can more easily use discriminants to get object accessibility tracking at runtime (and sometimes it picks things up at compile time). For cursors, I abandon the functions that only take cursors for object access and instead only access objects through a pairing of cursors and containers. At least this way I can test the cursor to ensure that it points to non erroneous data (since the container that is passed in has to exist at the point of the call). I won't claim it is a perfect solution, but I like it better than the assertions model GNAT uses in 9.2.0, so a matter of preference mostly.

I do get that it is difficult to implement the current rules, even GNAT has trouble avoiding bugs here. So I do understand and welcome rule changes. My first thought when I read this was that all my containers would cease to compile and the safety that I did have would be gone.

I know earlier you mentioned that in some cases one would need to change to Unchecked_Access, but I feel like that might under-represent the amount of changes people might have to make. Some folks might have to change a lot of code. And to be clear, I am not against dropping backwards compatibility for a new version of Ada, but my preference is if I need to adjust for breaking changes, I definitely want to have an improved situation. I think Ada could use quite a few breaking changes.

yannickmoy commented 4 years ago

My question wasn't so much focused on the intended progression plan, but you had mentioned you thought it was not possible to have compile time checking and object based lifetimes at the same time. There is quite a wide area between "not possible" and "easier to do in steps". I get the latter, but you had said the former, which was the source of my confusion.

Sorry for being the source of the confusion here. I did not mean that it's not possible at all, just that indeed it's far easier to introduce these changes in stages. </confusion>

yannickmoy commented 4 years ago

Only if you choose to do both lifetime and ownership models. I think you only need the lifetime model for dangling reference protection. That eliminates the aliasing and multiple handles issues.

True. Interesting to explore both I think, separately or in combination.

yannickmoy commented 4 years ago

For cursors, I abandon the functions that only take cursors for object access and instead only access objects through a pairing of cursors and containers. At least this way I can test the cursor to ensure that it points to non erroneous data (since the container that is passed in has to exist at the point of the call). I won't claim it is a perfect solution, but I like it better than the assertions model GNAT uses in 9.2.0, so a matter of preference mostly.

Interesting, that's also the decision we took in SPARK for the formal containers, in order to be able to deal with aliasing in proof. That was also explored in a library of traits based containers although this work was not properly finished. Maybe we could look at that for the future.

yannickmoy commented 4 years ago

I know earlier you mentioned that in some cases one would need to change to Unchecked_Access, but I feel like that might under-represent the amount of changes people might have to make. Some folks might have to change a lot of code.

That's part of the experiment. We won't change Ada at this stage, but offer alternative rules in GNAT, initially under a switch in any case.

And to be clear, I am not against dropping backwards compatibility for a new version of Ada, but my preference is if I need to adjust for breaking changes, I definitely want to have an improved situation. I think Ada could use quite a few breaking changes.

That's part of what we want to explore with these RFCs, feel free to submit Issues (to explose a problem) or RFCs (to propose a solution)!

jere-software commented 4 years ago

Hi @jeremiahbreeden, I want to bring my own "clarification" to what Yannick said: For us this is a stepping stone, more than an end goal: We want to have pointer safety in Ada ultimately. Getting rid of dynamic accessibility rules can be justified several ways in that aspect:

1. We want to get rid of existing rules that we consider sub-optimal, and that will cause a combinatory explosion when trying to add new rules. Before enhancing the status quo, we want to simplify.

2. Dynamic accessibility is a very complex part of Ada that, to the best of our knowledge, nobody completely understands when it comes to writing code, so people very often fall back on using unsafe accesses. The lesson to drag from your examples from Ada containers, which use unsafe accesses, is that it's _not really usable for the problems that we want to solve_. In fact, the experience we had at AdaCore, is that we implemented missing safety checks wrt. dynamic accessibility, and suddenly a lot of our code stopped working and had to stop using anonymous accesses.

Our long term goal is to have a model similar to Rust, with:

1. Statically verified lifetime verification for cases where it applies.

2. Refcounted pointers for cases where 1. doesn't apply and you don't care about the performance penalty of reference counting.

3. Unsafe accesses of some sort for the (hopefully small) remaining use cases.

Hope that clarifies things for you 🙂

It does. Thanks. Just some notes for consideration, I didn't get the impression from the RFC that this would be a stepped process to an end goal, or that it would eventually lead to object based accessibility checking at compile time. That's why I brought up my concern. The RFC also said that:

The new rules should both be effective at preventing errors and feel natural in Ada.

I don't feel this step makes Ada more effective at preventing errors (technically it would make it less effective). That may not be the goal of the step, but wanted to at least present my feelings on it.

I'm not saying "don't do it", but if the goal of this RFC is a step in a process, then the Motivation section should probably more clearly indicate that. The current Motivation section sounded like this was a complete solution.

If any of that sounds combative at all, I apologize, That is definitely not the intention of any of my responses to this RFC. I just care about Ada and want to at least express my feelings.

jere-software commented 4 years ago

I know earlier you mentioned that in some cases one would need to change to Unchecked_Access, but I feel like that might under-represent the amount of changes people might have to make. Some folks might have to change a lot of code.

That's part of the experiment. We won't change Ada at this stage, but offer alternative rules in GNAT, initially under a switch in any case.

That's my mistake then. I didn't realize this would only be an option. I thought it would be a replacement. As an option to explore, I have no objections, as long as there is a way to use the existing rules still (if needed).

jere-software commented 4 years ago

My question wasn't so much focused on the intended progression plan, but you had mentioned you thought it was not possible to have compile time checking and object based lifetimes at the same time. There is quite a wide area between "not possible" and "easier to do in steps". I get the latter, but you had said the former, which was the source of my confusion.

Sorry for being the source of the confusion here. I did not mean that it's not possible at all, just that indeed it's far easier to introduce these changes in stages. </confusion>

I'm used to being the source of confusion on a lot of things, so no worries. Thanks for taking the time to respond to my queries!

QuentinOchem commented 4 years ago

I personally would greatly welcome these changes. I've been bitten by dynamic accessibility checks in Ada many times in the past, mostly for using allocators on anonymous types rather than uncovering actual problems in the applications.

Two comments on details:

The differentiation between standalone and component accessibility rules is confusing to me - and reminds me of the endless confusion with Ada 2005, where “access bla” was treated differently depending on the context (parameter, component, etc.). Could we merge the two rules, and say that whatever the context, access T is the same accessibility level of T, unless using ‘Unchecked_Access?

Forbidding allocators on anonymous types is also a strong constraint. It’s tempting to allocate directly on an access parameter or a anonymous field / variable. I am personally often writing this kind of code naturally (up until I remember it doesn’t work because of the accessibility rules, see above). Having to create a named type in these cases feel unnecessarily complex - as long as the accessibility of the allocated object is the same as the target type. This does not preclude removing support to co-extensions and access discriminants.

yannickmoy commented 4 years ago

The differentiation between standalone and component accessibility rules is confusing to me - and reminds me of the endless confusion with Ada 2005, where “access bla” was treated differently depending on the context (parameter, component, etc.). Could we merge the two rules, and say that whatever the context, access T is the same accessibility level of T, unless using ‘Unchecked_Access?

The problem with that idea is that it is much more restrictive than what we propose, for a common use case here you're taking a local handle on some data in order to perform modifications, or to pass it to another subprogram. The current proposal is to give it an infinite accessibility level, so that these can be done safely. If we adopt instead the accessibility level of T, then you'll be forced to use Unchecked_Access whenever the source pointer is of a type with deeper accessibility level than T (simplest is just taking the address of a local variable in Var'Acces). In many cases, this will be indeed unsafe (for example, when you take the address of a local variable), because the compiler won't be able to check that you don't assign this value to a longer-lived object, so I don't think it's the way to go.

yannickmoy commented 4 years ago

Forbidding allocators on anonymous types is also a strong constraint. It’s tempting to allocate directly on an access parameter or a anonymous field / variable. I am personally often writing this kind of code naturally (up until I remember it doesn’t work because of the accessibility rules, see above). Having to create a named type in these cases feel unnecessarily complex - as long as the accessibility of the allocated object is the same as the target type. This does not preclude removing support to co-extensions and access discriminants.

Indeed, we could opt for giving to anonymous allocators the same accessibility level as the designated type. Let's see what other people think of it?

sttaft commented 4 years ago

If you make anonymous allocators passed to an access parameter equivalent to declaring a local aliased variable and taking 'Access, you might end up with cleaner semantics. It means they get deallocated automatically, just like any stack-resident variable. Furthermore, if access parameters are shifted to use "infinite" accessibility level, you aren't introducing any holes. You could disallow anonymous allocators in most other situations to keep the rules simple.

-Tuck

On Wed, Jun 24, 2020 at 9:28 AM yannickmoy notifications@github.com wrote:

Forbidding allocators on anonymous types is also a strong constraint. It’s tempting to allocate directly on an access parameter or a anonymous field / variable. I am personally often writing this kind of code naturally (up until I remember it doesn’t work because of the accessibility rules, see above). Having to create a named type in these cases feel unnecessarily complex - as long as the accessibility of the allocated object is the same as the target type. This does not preclude removing support to co-extensions and access discriminants.

Indeed, we could opt for giving to anonymous allocators the same accessibility level as the designated type. Let's see what other people think of it?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/AdaCore/ada-spark-rfcs/pull/47#issuecomment-648819677, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANZ4FM32RLMSKFTTPZWTWDRYH5OLANCNFSM4OAZGKVA .

yannickmoy commented 4 years ago

If you make anonymous allocators passed to an access parameter equivalent to declaring a local aliased variable and taking 'Access, you might end up with cleaner semantics. It means they get deallocated automatically, just like any stack-resident variable.

I think you're getting ahead of the proposal here, we're not looking at treating such anonymous allocators like local variables, only at their (static) accessibility level. What you're proposing looks interesting, but should IMO be part of a more comprehensive proposal on automatic memory reclamation.

Furthermore, if access parameters are shifted to use "infinite" accessibility level, you aren't introducing any holes.

exactly

You could disallow anonymous allocators in most other situations to keep the rules simple.

or give them the accessibility level of the designated type, like for components and function results. That seems simple too.

sttaft commented 4 years ago

You could disallow anonymous allocators in most other situations to keep the rules simple.

or give them the accessibility level of the designated type, like for components and function results. That seems simple too.

But is a guaranteed storage leak, pretty much.

yannickmoy commented 4 years ago

But is a guaranteed storage leak, pretty much.

I'll let @QuentinOchem comment on his use case, but that might be used where you don't want to reclaim the memory, and otherwise the user is responsible to reclaim this memory after converting to a named type (which can be done safely if the named type has the same accessibility level as the designated type).

QuentinOchem commented 4 years ago

The problem with that idea is that it is much more restrictive than what we propose, for a common use case here you're taking a local handle on some data in order to perform modifications, or to pass it to another subprogram. The current proposal is to give it an infinite accessibility level, so that these can be done safely. If we adopt instead the accessibility level of T, then you'll be forced to use Unchecked_Access whenever the source pointer is of a type with deeper accessibility level than T (simplest is just taking the address of a local variable in Var'Acces). In many cases, this will be indeed unsafe (for example, when you take the address of a local variable), because the compiler won't be able to check that you don't assign this value to a longer-lived object, so I don't think it's the way to go.

In my personal experience in Ada, the case you describe is extremely rare. There are many ways to avoid it, as you can usually rely on the fact that e.g. parameters are passed by reference. The only case where it's useful is when not only do you need to take a handle on that local data, but you also need to store it some way below (so not only modify it). This case is BTW only partially covered by your proposal, since you can write the relatively uncommon case:

G : access Bla;

procedure P1 (V : access Bla) is
begin
   G := V;
end P1;

but you can't write the much more likely case:

type X is record
   G : access Bla;
end record;

procedure P1 (V : access Bla) is
begin
   <some way to access some X>.G := V;
end P1;

To add to the confusion, now I can play around with parameters and write:

type X is record
   G : access Bla;
end record;

procedure P1 (V : X) is

To get my infinite access pointer instead of bound to X. I would much rather force Unchecked_Access for both cases and have a consistent syntax, in particular in light of the advertised use cases. Note that having infinite accessibility level for both would probably work as well, as long as there's only one semantic.

But is a guaranteed storage leak, pretty much.

I'll let @QuentinOchem comment on his use case, but that might be used where you don't want to reclaim the memory, and otherwise the user is responsible to reclaim this memory after converting to a named type (which can be done safely if the named type has the same accessibility level as the designated type).

I would like to treat:

type A_Bla is access Bla;

procedure P1 (V : acccess Bla)
procedure P2 (V : A_Bla)

P1 (new Bla);
P2 (new Bla);

exactly the same. So both create a Bla object at the same accessibility level as the Bla type. The creation of some kind of a virtual local scope for the first case is extremely confusing, and not that useful.

As for memory reclaim, a possibility is to have an named access type indeed. I would argue however in that case that it would be useful to have such a procedure available at all time, so one could write something like:

P1 (V : acccess Bla) is
begin
   Bla'Unchecked_Free (V);
sttaft commented 4 years ago

On Wed, Jun 24, 2020 at 1:25 PM QuentinOchem notifications@github.com wrote:

...

But is a guaranteed storage leak, pretty much.

I'll let @QuentinOchem https://github.com/QuentinOchem comment on his use case, but that might be used where you don't want to reclaim the memory, and otherwise the user is responsible to reclaim this memory after converting to a named type (which can be done safely if the named type has the same accessibility level as the designated type).

I would like to treat:

type A_Bla is access Bla;

procedure P1 (V : acccess Bla) procedure P2 (V : A_Bla)

P1 (new Bla); P2 (new Bla);

exactly the same. So both create a Bla object at the same accessibility level as the Bla type. The creation of some kind of a virtual local scope for the first case is extremely confusing, and not that useful.

As for memory reclaim, a possibility is to have an named access type indeed. I would argue however in that case that it would be useful to have such a procedure available at all time, so one could write something like:

P1 (V : acccess Bla) is begin Bla'Unchecked_Free (V);

Access parameters are "in" parameters, so the normal safety measure of setting the access value to Null after unchecked deallocation would not be possible. Any use of "V" after this statement would be erroneous, so not a great solution for access parameters. Other variables of an anonymous access type could be treated safely as you propose. Access discriminants, which serve a unique purpose currently (associated with object-level accessibility), would probably not want to allow allocators at all, if such discriminants are retained.

-Tuck

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/AdaCore/ada-spark-rfcs/pull/47#issuecomment-648956712, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANZ4FK6GB27FGMPQ4YF2ATRYIZILANCNFSM4OAZGKVA .

jere-software commented 4 years ago

The problem with that idea is that it is much more restrictive than what we propose, for a common use case here you're taking a local handle on some data in order to perform modifications, or to pass it to another subprogram. The current proposal is to give it an infinite accessibility level, so that these can be done safely. If we adopt instead the accessibility level of T, then you'll be forced to use Unchecked_Access whenever the source pointer is of a type with deeper accessibility level than T (simplest is just taking the address of a local variable in Var'Acces). In many cases, this will be indeed unsafe (for example, when you take the address of a local variable), because the compiler won't be able to check that you don't assign this value to a longer-lived object, so I don't think it's the way to go.

In my personal experience in Ada, the case you describe is extremely rare. There are many ways to avoid it, as you can usually rely on the fact that e.g. parameters are passed by reference. The only case where it's useful is when not only do you need to take a handle on that local data, but you also need to store it some way below (so not only modify it). This case is BTW only partially covered by your proposal, since you can write the relatively uncommon case:

I would just like to give my feedback and say my experience is the complete opposite of this. For me the case described is very common and the potential workarounds to it are usually undesirable.. My personal opinion is the more that the programmer needs to specify Unchecked_Access or (in GNAT's case) Unrestricted_Access, the farther from the ideal we get.

QuentinOchem commented 4 years ago

@jeremiahbreeden can you comment as to whether you are already in a situation that would require 'Unchecked_Access under the current proposal, or if you wouldn't need it, and if so, what kind of patterns are you implementing requiring an access type?

jere-software commented 4 years ago

@QuentinOchem Any time I try to make a reusable type that has an access component and that I intend to use with non library level stack allocated objects (bounded containers for example or even just variables declared in a subprogram instead of a package). Library level objects are really messy in large programs and lead to very difficult to track down problems. I prefer to keep my variables restricted to the scope they are used in. If you make access T always the same accessibility level as T as you propose, then I lose the capability to leverage discriminants to get object level accessibility.

EDIT: That said, the OP and team indicated this would be a phased approach, so as long as the long term plan is to get me in a place where I can do this safely (and they said it was the goal), then I am for it in general. I just don't want safety in Ada to go permanently backwards.

QuentinOchem commented 4 years ago

@jeremiahbreeden thanks for the extra details. So just to be clear, you have cases like

type X is record
   G : access Bla;
end record;

procedure P1 is
   L1 : aliased Bla;
   L2 : X;
begin
   L2.G := L1'Access; 

   P2 (L1'Access);
end P1;

procedure P2 (V : access Bla);
   L3 : X;
begin
   L3.G := V;
end P2;

Correct?

Note that the above pattern will require 'Unchecked_Access with the new proposal, no matter if you include my feedback or not. Anyway, if that's the case, then we are in agreement (this is indeed the kind of useful case that I was identifying). An interesting perspective to factor in.

sttaft commented 4 years ago

I think Jeremiah was also talking about the access discriminant case, where you don't need to use 'Unchecked_Access in general. -Tuck

On Wed, Jun 24, 2020 at 3:51 PM QuentinOchem notifications@github.com wrote:

@jeremiahbreeden https://github.com/jeremiahbreeden thanks for the extra details. So just to be clear, you have cases like

type X is record G : access Bla; end record;

procedure P1 is L1 : aliased Bla; L2 : X; begin L2.G := L1'Access;

P2 (L1'Access); end P1;

procedure P2 (V : access Bla); L3 : X; begin L3.G := V; end P2;

Correct?

Note that the above pattern will require 'Unchecked_Access with the new proposal, no matter if you include my feedback or not. Anyway, if that's the case, then we are in agreement (this is indeed the kind of useful case that I was identifying). An interesting perspective to factor in.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/AdaCore/ada-spark-rfcs/pull/47#issuecomment-649033820, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANZ4FLIGTKZVYLZFWJUCETRYJKLDANCNFSM4OAZGKVA .

QuentinOchem commented 4 years ago

@sttaft @jeremiahbreeden

I think Jeremiah was also talking about the access discriminant case, where you don't need to use 'Unchecked_Access in general.

Ok - if that's the case, I believe the initial proposal removes access discriminant (it's not entirely clear on that point to me, maybe @yannickmoy you can confirm is the suggestion is only to remove co extensions or all forms of access discriminant).

yannickmoy commented 4 years ago

I think @jeremiahbreeden was pointing to the common case that @QuentinOchem illustrated with an example. This case will require indeed Unchecked_Access with the new proposal, until we have a better way to manage the lifetime and/or ownership of components in relation with the enclosing object.

yannickmoy commented 4 years ago

In my personal experience in Ada, the case you describe is extremely rare. There are many ways to avoid it, as you can usually rely on the fact that e.g. parameters are passed by reference. The only case where it's useful is when not only do you need to take a handle on that local data, but you also need to store it some way below (so not only modify it).

@QuentinOchem I don't understand it this way. Take a parameter of a local type T_Ptr or an anonymous type access T. How can you assign safely it to a local variable Var (just for reading or modification purpose, say you're traversing the corresponding list)? With the current proposal, you can have type access T for Var precisely because it has infinite accessibility level. If instead it has the accessibility level of T you can't assign to it the parameter in both cases I mention above.

yannickmoy commented 4 years ago

To add to the confusion, now I can play around with parameters and write:

type X is record
   G : access Bla;
end record;

procedure P1 (V : X) is

@QuentinOchem sorry I don't follow your point here. In the code above, with the current proposal, G has the accessibility level of Bla.

yannickmoy commented 4 years ago

Answering your last proposal @QuentinOchem

I would much rather force Unchecked_Access for both cases and have a consistent syntax, in particular in light of the advertised use cases.

If you're proposing to completely drop any memory safety for access to local variables, indeed we could do that. :-) That does not seem a good intermediate step though.

Note that having infinite accessibility level for both would probably work as well, as long as there's only one semantic.

So you're proposing to use infinite accessibility level for all anonymous access types, right? Including for components and function results, where the current proposal is to adopt instead the accessibility level of the designated type. For components, that's unsafe, as that means you'll be able to store the address of a variable defined in a nested scope to a record variable defined in an outer scope. For function results, that just means you can't store these values in an object of a named access type anymore. None looks acceptable to me.

sttaft commented 4 years ago

Depending on the accessibility level of the designated type could be seen to add complexity, not remove it. It means that declaring a subtype of Integer, a type derived from Integer, or a new integer type, might affect any anonymous access type that happened to designate that (sub)type. This can create maintenance headaches, since the anonymous access type might be declared relatively far away from the integer (sub)type, and the normal decision process about whether to define a subtype or a derived type has in the past had no such concern.

yannickmoy commented 4 years ago

Depending on the accessibility level of the designated type could be seen to add complexity, not remove it.

@sttaft That's what this RFC proposes for components and function returns. It's hard to see how the fact that access Derived_T has the same accessibility as Derived_T introduces conceptual complexity, so I'm not following your argument.

sttaft commented 4 years ago

On Thu, Jun 25, 2020 at 8:14 AM yannickmoy notifications@github.com wrote:

Depending on the accessibility level of the designated type could be seen to add complexity, not remove it.

That's what this RFC proposes for components and function returns. It's hard to see how the fact that access Derived_T has the same accessibility as T introduces conceptual complexity, so I'm not following your argument.

Is that actually true, if the derived type is declared more locally than T? E.g.:

 package body Blah is
      type T is range ...
      procedure P is
           type Local_Derived_T is new T range ...;
           subtype Local_Sub_T is T range ...
           type Rec is record
                 Comp1 : access Local_Derived_T;  --  accessibility of

Comp1's type? Comp2 : access T; -- accessibility of Comp2's type? Comp3 : access Local_Sub_T; -- accessibility of Comp3's type? end record;

In Ada 2012, the anonymous access types of Comp1, 2, and 3 all have the same accessibility level.

yannickmoy commented 4 years ago

Is that actually true, if the derived type is declared more locally than T? In Ada 2012, the anonymous access types of Comp1, 2, and 3 all have the same accessibility level.

@sttaft Well no, we're precisely proposing alternative rules here.

QuentinOchem commented 4 years ago

@QuentinOchem I don't understand it this way. Take a parameter of a local type T_Ptr or an anonymous type access T. How can you assign safely it to a local variable Var (just for reading or modification purpose, say you're traversing the corresponding list)? With the current proposal, you can have type access T for Var precisely because it has infinite accessibility level. If instead it has the accessibility level of T you can't assign to it the parameter in both cases I mention above.

But if you're doing modification, why would you need an access type for traversing in the first place? You should only use in out mode - unless you want to escape the value, which is precisely the unsafe pattern where Unchecked_Access makes sense.

To add to the confusion, now I can play around with parameters and write:

type X is record
   G : access Bla;
end record;
procedure P1 (V : X) is

@QuentinOchem sorry I don't follow your point here. In the code above, with the current proposal, G has the accessibility level of Bla.

My point is that in the example before, you have different semantic for:

procedure P1 (V : X)

and

procedure P1 (V : access Bla)

The first one is accessibility of Bla (I might have gotten lost - not trying to play dumb, but the different semantic really confuse me) and the second has infinite accessibility.

My point is that - at least as far as I'm concerned - having different semantics seemingly similar things (anonymous access types) is very confusing. Ada 2012 at least had that right compared to the similarly confusing Ada 2005 semantic - anonymous access types became one and unique thing. In this regards, I would consider the current proposal as a step back.

sttaft commented 4 years ago

On Thu, Jun 25, 2020 at 5:49 PM yannickmoy notifications@github.com wrote:

Is that actually true, if the derived type is declared more locally than T? In Ada 2012, the anonymous access types of Comp1, 2, and 3 all have the same accessibility level.

Well no, we're precisely proposing alternative rules here.

I still wonder about the wisdom in having the accessibility level depend on the level of the designated (sub?)type, as the choice between defining a local name for a type using a derived type vs. a subtype vs. a new type with similar range, do not normally take into account any indirect effect on anonymous access types declared elsewhere, and so during maintenance a change from one to the other could have unexpected and potentially confusing side effects. Fundamentally, the level of the designated (sub)type seems somewhat irrelevant to the lifetime of an object of an anonymous access type.

-Tuck

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/AdaCore/ada-spark-rfcs/pull/47#issuecomment-649834941, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANZ4FMPCRAYT5GRYT6MGJ3RYPA57ANCNFSM4OAZGKVA .

swbaird commented 4 years ago

One point in the RFC (referring to certain anonymous access types) led to some discussion: We propose to define their accessibility level to be the same as the one of their designated type.

The question was raised of what this would mean (with respect to compile-time semantics) if the designated type is a generic formal type. Presumably we would want to be as permissive as possible in the generic (i.e., allow a construct in a generic if it would possible to declare an instantiation where the corresponding construct in the instance is ok) and then recheck the instance. In Ada, accessibility rules in a generic body are more restrictive than in a generic spec because Ada has the rule that legality rules are not enforced in an expanded instance body (there are also special rules about enforcement in the private part of an instance) - see Ada RM 12.3(11). This leads to Ada defining optimistic "assume the best" rules in a generic spec which rely on instance rechecking and pessimistic "assume the worst" rules in a generic body (because we cannot rely on instance rechecking). One option to consider is to follow SPARK's example (see SPARK RM 12.1, the part beginning with "Ada has a rule that legality rules are not enforced in an instance body") and require rechecking of instance bodies; this would allow use of more permissive "assume the best" rules throughout a generic unit at the cost of weakening Ada's contract model (Ada tries to avoid having cases where the legality of an instantiation of a generic unit depends on the contents of the body of that generic unit).

yannickmoy commented 4 years ago

@QuentinOchem

But if you're doing modification, why would you need an access type for traversing in the first place? You should only use in out mode - unless you want to escape the value, which is precisely the unsafe pattern where Unchecked_Access makes sense.

If you're taking a local handle to traverse the data structure, then you can't use in out like you propose. You need an access variable. For this to be safe whatever the type of the variable and component, the variable needs to have anonymous access type.

My point is that in the example before, you have different semantic for: [...] The first one is accessibility of Bla (I might have gotten lost - not trying to play dumb, but the different semantic really confuse me) and the second has infinite accessibility.

I agree that's not ideal. The model proposed here tries to maximize the safety guarantees provided by the compiler. But if users end up still not understanding the rules, that's indeed counterproductive. I just realized also that this model prevents writing the simple identify function (and any similar accessors) taking an anonymous access type:

function Id (X : access T) return access T is (X);

Indeed, the parameter has infinite access type, so it cannot be returned, as the return type has the accessibility level of T. This kind of examples makes me doubt we've found the right solution.

My point is that - at least as far as I'm concerned - having different semantics seemingly similar things (anonymous access types) is very confusing. Ada 2012 at least had that right compared to the similarly confusing Ada 2005 semantic - anonymous access types became one and unique thing. In this regards, I would consider the current proposal as a step back.

I would not say Ada 2012 considers anonymous access types as one and unique thing. More like 7 different things really (really!). But I agree with you using the accessibility level of T everywhere would make it dead simple to understand what happens. At the cost of requiring Unchecked_Access each time you want to take the address of a local variable, until we have a better lifetime/ownership solution. To be discussed.

yannickmoy commented 4 years ago

Fundamentally, the level of the designated (sub)type seems somewhat irrelevant to the lifetime of an object of an anonymous access type.

One direction is obvious: the lifetime of such an object cannot be larger than the level of the designated type. Then, any refinement seems to need something better than what we have right now, based on lifetime/ownership possibly.

QuentinOchem commented 4 years ago

@yannickmoy

If you're taking a local handle to traverse the data structure, then you can't use in out like you propose. You need an access variable. For this to be safe whatever the type of the variable and component, the variable needs to have anonymous access type.

My point is that you usually don't need to take a local handle to traverse a data structure in Ada. But notwithstanding this:

Reading again the proposal, I also realize that unchecked conversion between pointers look like:

type T_Ptr is access T;
Anon  : access T := ...
Named : T_Ptr := Anon.all'Unchecked_Access;

I would like to point out that this is a double penalty. You incur the accessibility risk with the Unchecked_Access, but you also incur the risk of having Anon being null here - so potential constraint error. In this case, this is actually introducing a new vulnerability that will need to be mitigated (in this case, through e.g. an if expression). That does't feel like going to the right direction to me.

But reading the proposal over and over again, let's consider this case:

procedure Traverse (V : access Bla); -- OK, let's assume you really really need a handle here

procedure Something is
   L : aliased Bla;
begin
   Traverse (L'Unchecked_Access);

No matter the proposal, yours or mine, you will always need Unchecked_Access in the above case, correct? The parameter is infinite, L is local.

So the only thing that the so called infinite accessibility level allows you to do is to convert from an anonymous access (from a parameter or a global) to a named pointer, correct? And this is really only interesting when you have named pointers types that are not declared at same level as their designated types, correct?

sttaft commented 4 years ago

On Mon, Jun 29, 2020 at 10:50 AM QuentinOchem notifications@github.com wrote:

... let's consider this case:

''' procedure Traverse (V : access Bla); -- OK, let's assume you really really need a handle here

procedure Something is L : aliased Bla; begin Traverse (L'Unchecked_Access); '''

No matter the proposal, yours or mine, you will always need Unchecked_Access in the above case, correct? The parameter is infinite, L is local.

I think you might have it exactly backwards. The meaning of "infinite" accessibility level is that you can convert anything to it, but you will always need Unchecked_Access to convert to a non-infinite accessibilty level. Remember accessibility level grows as lifetime shrinks, so infinite accessibility level can denote things that have very short lifetimes.

So the only thing that the so called infinite accessibility level allows you to do is to convert from an anonymous access (from a parameter or a global) to a named pointer, correct?

I think the intent is just the opposite. You can convert any named access type to an infinite-accessibility-level type, but never the reverse.

...

-Tuck

AdaDoom3 commented 3 years ago

I have updated the proposal to include changes which Steve, Gary, and I believe will improve the "simplification" of accessibility rules in so far as the user can understand them while also allow more obviously safe cases to avoid the cumbersome ".all'Unchecked_Access" notation.