- 
                Notifications
    
You must be signed in to change notification settings  - Fork 124
 
Unambiguous View #1598
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
base: main
Are you sure you want to change the base?
Unambiguous View #1598
Conversation
| 
           The test failures are a little baffling, since they are all(?) about duplicate diagnostic items; this PR does nothing related to diagnostic items, so I am not quite sure what to do to fix this. My current hypothesis is that I might be missing some attribute somewhere, but am not sure what. The failures are reproduced locally too with: $ vargo test -p rust_verify_test --test basic | 
    
| 
           The   | 
    
| 
           Ooh, that is neat!  I tried a similar version but was placing another  I'll update this PR when I get a few spare cycles, thanks for providing the demonstration case! I think that might also unblock a subtle issue I was facing with containers here (which is the main reason why I switched this PR to a draft).  | 
    
| 
           the test failure is due to an obscure test that makes sure that builtin can be embedded as a submodule rather than imported as an external crate. The only way I can make sense of the test failure is that something in your change is prompting Verus to import   | 
    
| 
           Can you write up a bit about the intended use-case for UnambiguousView?  | 
    
| 
           update: I believe c38aa46 should fix the test failure, though I still don't understand why this particular PR is triggering the bug when it wasn't an issue before.  | 
    
| 
           Updated to remove the  Additionally, I think it unblocks support for containers. I want to mull over one specific detail for a little bit before I push  Thanks @tjhance for looking into the CI failure. I too am surprised this triggered it, but I haven't tried to bisect it. Instead, it appears that things are passing on my latest push, so I guess things are ok again? Use case: I mention this in the description, but I would like  As a particular example, working with (say)  Anyways, to actually be able to test things out (possibly via a Veritas run) to know if we can switch to this explicit shallow/deep world, it is helpful to know how many situations actually break when we switch  Again, this PR is not making any of those shifts in syntax/etc, nor is it proposing explicitly to make such shifts (other than by a singular doc comment), it is merely introducing  Oh, and iirc, @parno had some specific use case(s) for it too.  | 
    
| 
           I think establishing a clear connection between  Ideally, we could introduce more general type constraints that establish relationships between  Specifically, 
 I've drafted an example in Haskell demonstrating this approach. I'm unsure if expressing this approach cleanly in Rust is straightforward, and I recognize it would impose constraints on the types of   | 
    
| 
           To be honest, I'm still not sure I understand what problem this is solving. As I understand it, the biggest complaint about the view system is that  The premise of  
 However, I don't find this case to be particularly common, as shallow and deep views will essentially never coincide when working with generics. The specification of   | 
    
| 
           Thanks @amaurremi for writing up the Haskell version. I suspect that it is not translatable to Rust trait world due to trait resolution semantics (specifically, at least to my current understanding, we can't have specialization in the way that the OVERLAPPABLE/OVERLAPPING are doing things in the Haskell version). If you are able to translate it to Verus (or even into Rust, without yet passing Verus checks), that would be great! The Fwiw, this PR is not introducing a new Rust/Verus type, or even a new type of view, instead it just is a constraint available for types that already have things coinciding. Users should not need to actually know of, or deal with the unambiguous view (at least in the intended final state, see below). The "for particular subsets of types" in Rust is just represented via traits, which is why we end up introducing a new trait, that's all. @tjhance, to answer your question: this PR is not the final state of this feature, but is introducing the scaffolding necessary to reach the final intended state. I write more about intended final state elsewhere too, but I'll summarize below. For  Regarding the usage: there are both expert and novice users (of both Verus and Rust), and it would help cater to both users well, especially if we can provide a better on-ramp to expert usage.  Travis, you are an expert user who regularly works in parametric land: it is perfectly normal for you to work in the space where shallow/deep do not coincide; I trust you to use the correct view in any use case, making yourself explicit if necessary.  However, for novice users, it is surprising and confusing to get  Now obviously, one can say "that is incorrect, and they should learn that  The vision here is that for most users, they don't need to worry about the complexities of there existing various kinds of views. If they are reading someone else's code, often the specific view being taken is not particularly important either, so if they saw an   | 
    
          
 I didn't mean to imply that I thought this PR was the "final state". I was operating under the assumption that the "final state" is what was given in the PR description: 
 Which I also took to mean that  Unless I'm misunderstanding something, this would mean an enormous amount of code that uses  
 While I acknowledge there may be an issue for novice users, I simply don't believe this issue is significant enough to justify such a large downgrade in the utility of   | 
    
| 
           I 100% agree that if most code needs to be changed to  So yes, I agree with you that if there is a massive downgrade then the benefit is not worth it. However, I suspect that the final state is not as stark as it might initially appear.  | 
    
| 
           [retreat 2025] there may be open design decisions to resolve here  | 
    
| 
           At the retreat, there was some discussion of trying to be more disciplined about (a) implementing both  There was also a desire for a shorthand for   | 
    
| 
           Fwiw, I wasn't present at this discussion, but I'd also vote against redefining @ as   | 
    
| 
           Similarly, I wasn't at this discussion, and I too would vote against redefining  As one might guess with this PR draft, my viewpoint (heh), is that the unambiguous view is often sufficient for most tasks.  I do believe that there is work to be done to actually demonstrate that it won't be too much of a breaking change, before we even seriously consider  
  | 
    

At a high level,
UnambiguousViewstates that theViewandDeepViewcoincide exactly.To do this, we first introduce Rust type-level equality (withTypeEq) which allows us to then defineUnambigiousView.I would like to additionally suggest that we (eventually) rename
View->ShallowViewandUnambiguousView->View; but I think that should be a future migration, since it would break codebases at least somewhat. For now, I have left in a comment related to this possible migration.Additionally, obviously, naming and so on is up for bike-shedding---I am not particularly thrilled with "unambiguous view" as the name (but I do think it is better than the "both views" name that has been informally used in the past to refer to this idea).
Furthermore, whileTypeEqimplemented via sealed traits is kinda cute, ideally we wouldn't need to rely on sealed traits. There is some support in Rust to have equalities via associated types, but unfortunately, there appears to be a bad recursion that occurs, and so instead we have theTypeEqapproach. The reason I thinkTypeEqis suboptimal is that the type system itself isn't aware of the equality, and thus we need to have thereflexivityconversion to remind the type system that equality is fine; if we were using a built-in type equality, then we wouldn't need that level of indirection.EDIT: With @zeldovich noticing that associated type equality works when you try just
View::V==DeepView::V(rather than what I was previously attempting, which was introducingUnambiguousView::Vand trying to doUnambiguousView::V == View::V == DeepView::V, which leads to a self-recursion that the compiler does not like), we no longer needTypeEq, and can directly do equality.By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.