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

Add borrow!/borrow? ... [as ...] syntax #164

Closed
5 tasks
turbolent opened this issue Jun 18, 2020 · 7 comments
Closed
5 tasks

Add borrow!/borrow? ... [as ...] syntax #164

turbolent opened this issue Jun 18, 2020 · 7 comments
Labels
Feature Good First Issue Good for newcomers

Comments

@turbolent
Copy link
Member

Context

It was suggested that in addition/instead of borrow<T>() function calls,
the dedicated syntax borrow!/borrow? ... [as ...] might be more readable and explicit.

Definition of Done

  • Parser: Add rule
  • Sema: Check
  • Interpreter: Interpret like borrow call
  • Tests
  • Documentation
@benjaminkvm
Copy link
Contributor

From the issue it appears we would like to implement the following syntax to borrow a reference to an object in storage:

borrow!/borrow? <path> [as <type>]

The above seems to imply that the downcast to the <type> is optional. Also note. we currently only have a as? downcast operator, which returns an optional -> downcast variable or nil (if the type could not be cast).

My understanding is that borrow! would force-unwrap the object reference from storage (if not nil, panic otherwise).

Thinking through this, I think it makes sense to have borrow? be just borrow, which returns an optional:

// a is &Any?
let a = borrow /public/a

and the force-unwrapped version:

// a is &Any
let a = borrow! /public/a

Mainly because otherwise, borrow? ssets a precedent of having <fn>? indicate an optional is returned — which we don’t really do anywhere else as far as I know.

Now if we want to support the above however, speaking with Bastian, it seems unlikely anyone would want just an &Any and will likely always downcast to a specific type. So perhaps we have the borrow expression always in the form of:

// a is &String
let a = borrow! /public/a as &String

Or perhaps we always force-unwrap the object reference, by definition, and just have the following which is semantically the same:

// a is &String
let a = borrow /public/a as &String

Would love feedback! Trying to determine a rational syntax that is internally consistent with the existing as? operator also if possible.

@benjaminkvm
Copy link
Contributor

Going to put this on pause for a bit as it is a lower prority and Dete is away next week -- would love to get his input on this.

@sifmoon
Copy link

sifmoon commented Aug 25, 2020

Assigning this to Tim to re-assign out.

@cybercent
Copy link

cybercent commented Sep 30, 2020

As you're updating borrow could a way to getCapability on multiple paths be added?

To avoid doing this:

    var collectionRef = getAccount(address).getCapability(/public/CollectionOne)!
        .borrow<&{Contract.Collection}>()
        
    if collectionRef == nil {
        collectionRef = getAccount(address).getCapability(/public/CollectionTwo)!
        .borrow<&{Contract.Collection}>()
    }
    
    if collectionRef == nil {
        collectionRef = getAccount(address).getCapability(/public/CollectionThree)!
        .borrow<&{Contract.Collection}>()
    }

We could do this:

  var collectionRef = getAccount(address).getCapability(/public/*)!
        .borrow<&{Contract.Collection}>()

This might be linked to #208

@rheaplex
Copy link
Contributor

rheaplex commented Sep 30, 2020

Capabilities are a security measure so specifying which one is to be used seems like something that should be explicit. And a wildcard should return all matches, not just the first one.

That said, combining Storage Querying API #208 with Typed Paths #369 would allow the program to find if any capabilities of a given type exist under a given path and to use just the first one if desired. I'm wary of recommending a pattern rather than a feature but I think making this explicit in this way might make the program's intent clearer.

@cybercent
Copy link

cybercent commented Sep 30, 2020

Hey @robmyers, sorry for not making this more clear. I meant /public/* to return all matches.
In my use-case, I'm looking for a way to fetch info about all NFTs no matter the path.

I expect Dapps to move NFTs around as they see fit and not necessarily put them back to the path from where they took them from.

Example:

  1. As a user I list my NFT for sale on a marketplace, my NFT is moved and instead of /public/kittiesCollection I now need to get info about the NFT from /public/kittiesForSaleCollection.

  2. I delist my sale, the marketplace might put the kitty in another place and now I need to fetch the info from /public/marketplaceKittiesCollection instead of /public/kittiesCollection.

I agree with making things explicit, but from another angle, /public can be used in a script, so there might be no security implications in this case.

To make an analogy, imagine on your Mac you need to search for a PDF file, you don't remember the name of it, so it's just *.pdf and instead of opening Spotlight to search, you would need to go folder by folder and manually look if you have any PDF files in each and every folder of your Mac. This was done like this so that the user can't edit the PDF, but in reality the user only wanted to read it and never intended to modify it.

@turbolent turbolent removed their assignment Jan 15, 2021
SupunS added a commit that referenced this issue Nov 29, 2023
Reject resources-methods being used as function-pointers
@turbolent
Copy link
Member Author

No progress in a long time, closing for now. Please open a new issue if you still feel like this should exist

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Good First Issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

6 participants