Skip to content
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

fix: Correct null-related checking of type bounds, and add axioms #5738

Merged
merged 25 commits into from
Sep 5, 2024

Conversation

RustanLeino
Copy link
Collaborator

@RustanLeino RustanLeino commented Aug 30, 2024

Fixes #5719
Fixes #5726

Description

This PR solves both soundness and incompleteness problems related to when a nullable type extends a trait.

To fix the soundness issue (#5726), the resolver was changed to check type bounds after type inference. For example, the resolver now generates an error if a nullable reference type E? is used as a type argument with a bound B that is not a nullable reference type. Note that this check cannot be done completely during pre-type inference, because pre-types don't distinguish between E? and E; therefore, the definitive check for this is done in the same place where type arguments are checked to have the specified type characteristics ((==), (0), ...).

To fix the incompleteness issues (#5719), changes were made in the generated axioms. To describe them, consider the following types:

trait RefTrait extends object { }
trait GeneralTrait { }  // does not extend object, and thus is not a reference type
class /* or trait */ E extends RefTrait, GeneralTrait { }
  • Previously, this generated the following type axioms (and analogous allocation axioms for $IsAlloc):

    // type axiom for trait parent: E extends RefTrait
    axiom (forall $o: ref ::
      { $Is($o, Tclass.E?()) }
      $o != null && $Is($o, Tclass.E?())         // CHANGE: remove o != null
         ==> $Is($o, Tclass.RefTrait()));        // CHANGE: RefTrait --> RefTrait?
    
    // type axiom for trait parent: E extends GeneralTrait
    axiom (forall $o: ref ::
      { $Is($o, Tclass.E?()) }                   // CHANGE:  replace E? with E
      $o != null && $Is($o, Tclass.E?())         // CHANGE:  remove o != null, and replace E? with E
         ==> $IsBox($Box($o), Tclass.GeneralTrait()));

    The new axioms say:

    // type axiom for trait parent: E extends RefTrait
    axiom (forall $o: ref ::
      { $Is($o, Tclass.E?()) }
      $Is($o, Tclass.E?()) ==> $Is($o, Tclass.RefTrait?()));
    
    // type axiom for trait parent: E extends GeneralTrait
    axiom (forall $o: ref ::
       { $Is($o, Tclass.E()) }
      $Is($o, Tclass.E()) ==> $IsBox($Box($o), Tclass.GeneralTrait()));
  • To make the first of these axioms still be able to connect E? with RefTrait, the axiom that connects a nullable type X? with its non-null reference type X was extended with an additional matching pattern:

    // $Is axiom for non-null type X
    axiom (forall c#0: ref :: 
      { $Is(c#0, Tclass.X()) }
      { $Is(c#0, Tclass.X?()) }      // CHANGE: this matching pattern was added
      $Is(c#0, Tclass.X()) <==> $Is(c#0, Tclass.X?()) && c#0 != null);
  • The property that a type parameter T with a bound B has is (elsewhere) given as a quantification over boxes. To make this property known of a type that extends a trait, the following axioms were added:

    axiom (forall bx: Box :: 
      { $IsBox(bx, Tclass.E?()) } 
      $IsBox(bx, Tclass.E?()) ==> $IsBox(bx, Tclass.RefTrait?()));
    
    axiom (forall bx: Box :: 
      { $IsBox(bx, Tclass.E()) } 
      $IsBox(bx, Tclass.E()) ==> $IsBox(bx, Tclass.GeneralTrait()));

    These are essentially box versions of the type axioms above.

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

```
// type axiom for trait parent: E extends RefTrait
axiom (forall $o: ref ::
  { $Is($o, Tclass._module.E?()) }
  $o != null && $Is($o, Tclass._module.E?())                // CHANGE: remove o != null
     ==> $Is($o, Tclass._module.RefTrait()));             // CHANGE: RefTrait --> RefTrait?

// allocation axiom for trait parent: E extends RefTrait
axiom (forall $o: ref, $heap: Heap ::
  { $IsAlloc($o, Tclass._module.E?(), $heap) }
  $o != null && $IsAlloc($o, Tclass._module.E?(), $heap)       // CHANGE: like above
     ==> $IsAlloc($o, Tclass._module.RefTrait(), $heap));    // CHANGE: like above

// type axiom for trait parent: E extends GeneralTrait
axiom (forall $o: ref ::
  { $Is($o, Tclass._module.E?()) }                           // CHANGE:  replace E? with E
  $o != null && $Is($o, Tclass._module.E?())                   // CHANGE:  remove o != null, and replace E? with E
     ==> $IsBox($Box($o), Tclass._module.GeneralTrait()));

// allocation axiom for trait parent: E extends GeneralTrait
axiom (forall $o: ref, $heap: Heap ::
  { $IsAlloc($o, Tclass._module.E?(), $heap) }                       // CHANGE: like above
  $o != null && $IsAlloc($o, Tclass._module.E?(), $heap)               // CHANGE: like above
     ==> $IsAllocBox($Box($o), Tclass._module.GeneralTrait(), $heap));
```
# Conflicts:
#	Source/IntegrationTests/TestFiles/LitTests/LitTest/dafny0/SubsetTypes.dfy.expect
#	Source/IntegrationTests/TestFiles/LitTests/LitTest/dafny1/SchorrWaite.dfy.expect
#	Source/IntegrationTests/TestFiles/LitTests/LitTest/dafny1/SchorrWaite.dfy.refresh.expect
@RustanLeino RustanLeino marked this pull request as ready for review September 3, 2024 22:55
# Conflicts:
#	Source/DafnyPipeline.Test/expectedProverLog.smt2
#	Source/IntegrationTests/TestFiles/LitTests/LitTest/dafny1/SchorrWaite.dfy.expect
#	Source/IntegrationTests/TestFiles/LitTests/LitTest/dafny1/SchorrWaite.dfy.refresh.expect
@RustanLeino RustanLeino enabled auto-merge (squash) September 4, 2024 22:36
Copy link
Member

@atomb atomb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks reasonable. And it's thoroughly tested!

@RustanLeino RustanLeino merged commit ad812af into dafny-lang:master Sep 5, 2024
22 checks passed
@RustanLeino RustanLeino deleted the issue-5719 branch September 5, 2024 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Null-related issues with bounded polymorphism Verification fragility on trait extension
2 participants