- 
                Notifications
    You must be signed in to change notification settings 
- Fork 700
Extensible encoding of function signatures #640
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
          
     Merged
      
      
    
  
     Merged
                    Changes from all commits
      Commits
    
    
            Show all changes
          
          
            13 commits
          
        
        Select commit
          Hold shift + click to select a range
      
      f78543d
              
                Prettify section names
              
              
                rossberg bb8a6e4
              
                Restructure encoding of function signatures
              
              
                rossberg 247d7b6
              
                Merge pull request #637 from WebAssembly/binary_0xb_version
              
              
                 7c8be53
              
                Revert "[Binary 11] Update the version number to 0xB."
              
              
                 0ac338d
              
                Merge pull request #642 from WebAssembly/revert-637-binary_0xb_version
              
              
                 c63829e
              
                Leave index space for growing the number of base types
              
              
                rossberg ec21463
              
                Comments addressed
              
              
                rossberg 04c63fb
              
                clarify how export/import names convert to JS strings (#569) (#573)
              
              
                pizlonator 2312afd
              
                Access to proprietary APIs apart from HTML5 (#656)
              
              
                loxal 903d997
              
                Merge branch 'master' into types-sec
              
              
                rossberg 3c53609
              
                Merge branch 'master' into types-sec
              
              
                rossberg df569d7
              
                Merge branch 'binary_0xb' into types-sec
              
              
                rossberg aa0407d
              
                comments
              
              
                rossberg File filter
Filter by extension
Conversations
          Failed to load comments.   
        
        
          
      Loading
        
  Jump to
        
          Jump to file
        
      
      
          Failed to load files.   
        
        
          
      Loading
        
  Diff view
Diff view
There are no files selected for viewing
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could keep the disjointness of primitive and structural type constructors by giving the former monotonically increasing integers and the latter monotonically decreasing integers (starting by giving functions a
formof-1). So same aesthetic preference for avoiding arbitrary-feeling statements like "noone will ever need more than0x3fprimitive type constructors", but different meaning for the negative index than in the previous comment. If so then, the type would be avarint32.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with using var(u)int. But the type space potentially indexes 3 sorts of things: primitive constructors, structural constructors, type ids. If we want to use the signedness overlay trick, than it seems much more beneficial to reserve that for distinguishing between ids and constructors in the future, so that type id references could be single byte (in which case we probably need to assign negative numbers to all constructors now). WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't understand this part of the OP: why would we want to add the complication of "inlining" certain structural constructors instead of just using a type id?
Yes, if we can rule out the third case as I'm asking above then the encoding could be pretty simple: if positive, it's a pritimive, if negative, it's a (negated) type-id. But that'd be the encoding of a value type.
formis the encoding of a different set: the set of compound constructors. So as observed earlier, it could completely overlap with both primitives and type-ids. I can see the argument for keeping the encoding of compound ctors disjoint from that of primitive ctors (so that you can represent the set of all constructors with an int), but that seems to work just fine with giving the compound ctors negative indices.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
E.g. to reduce the overhead of one-off uses of structural types. Or imagine
you want to emit a more complicated nested struct, then you wouldn't need
to separate out and name each level.
Perhaps the other way round is a more compelling scenario: you may want to
allow primitive constructors in type definitions. Or they'll start to mix
when we introduce type import/exports some day. Also, it's hard to predict
what other thing might come along and change the story (generics? who
knows).
I'm not suggesting that there currently is a concrete reason to join the
spaces. But it doesn't seem completely unlikely to arise later, and given
that the cost of keeping it an option is zero, why preclude it?
Yes, if we can rule out the third case as I'm asking above then the
I'd actually invert that scheme, to avoid the negation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I don't understand is why all those future new things can't just be new
forms of entries in the types section such that a type-id is all you need?Wouldn't that give all the primitive types today (
i32, etc) negative indices?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@titzer, ha, I knew I shouldn't have mentioned generics. This is getting OT, but let me just say that I would like to avoid their complexity as much as the next guy, while I'm also aware that "our language doesn't need generics" have become famous last words of language/VM designers. Static compilation only works for fairly weak, second-class polymorphism; in more expressive cases (which all big languages but C++ support) you'd be forced to introduce unions and lots of expensive runtime checks. Maybe that's okay for Wasm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also don't understand what is being proposed in this PR to address this: superficially this is just a question of
0x40vs.-1. What are you proposing happens for these new-kinds-of-types?Still OT, but: yes, for Java-style. For C#-style, though, I was assuming that a C#-on-wasm runtime would actually need to ship with its own runtime machinery to do runtime generation of wasm for instantiations that only show up at runtime since I'd be surprised if we could design a feature in wasm that wasn't overly specialized to C# but that could still handle the C# use case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If non-nullary constructors don't show up in value types (only their type-ids), then both could have negative indices. But I guess the counterargument is: maybe not the 3 non-nullary ctors we're thinking about now (func, struct, array), but perhaps some new thing in the the future and if positive indices are "reserved" for nullary and negative is "reserved" for type-ids, then we're out of luck (or, at the very least, we'd have to do break the pattern). I guess I buy that, so
0x40is fine. More than just the aesthetic-1vs.0x40has been question of where this is going which I think I now have a better understanding of.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incidentally, I came across a situation which supports the current design: in Wasm.Table we want to be able to declare the types of elements in the table definition/import. The abovementioned scheme for local types seems to fit (allowing table elements to have any type that you can put in a local, or some restriction thereof), but the question is how to say "any function". Well, since we already have this 0x40 "Function" constructor in the index space, that seems to be a good candidate (even if it's a slight abuse of logical category). Similarly, the "Struct" and "Array" constructors could mean "any struct type" / "any array type" which could make sense one day.