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

[documentation] some expanded documentation types are invalid pony-code #2145

Closed
mfelsche opened this issue Aug 8, 2017 · 4 comments
Closed
Assignees

Comments

@mfelsche
Copy link
Contributor

mfelsche commented Aug 8, 2017

What i was trying to achieve

I wanted to write a method which accepts everything a Set would accept. I copy and pasted the type parameter As signature into my method.

Code with bug

use "collections"

actor Main
  new create(env: Env) =>
    let singleton = singleton_set[String]("A")

  fun singleton_set[A: (Hashable #read & Equatable[A #read] #read)](t: A)
    : Set[A] 
  =>
   Set[A](1) .> add(t)

http://playground.ponylang.org/?gist=7b903395cd00e4840d185be3761eba60

Actual Result

0.17.0 [release]
compiled with: llvm 3.9.1 -- cc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609
Error:
main.pony:5:54: a capability set can only appear in a type constraint
  fun singleton_set[T: (Hashable #read & Equatable[A #read] #read)](t: T)

Expected Result

I would expect the type signatures in the autogenerated pony documentation to be valid pony code.

The actual definition of Set is:

type Set[A: (Hashable #read & Equatable[A] #read)] is HashSet[A, HashEq[A]]

The documentation signature is:

type Set[A: (Hashable #read & Equatable[A #read] #read)] is
  HashSet[A #read, HashEq[A #read] val] ref

After discussing this with @SeanTAllen it is clear that the docgen process needs expanded types in order to establish the links between classes and traits etc.
It would really help if the actual signature was the real code representation of the type/class/actor definition. The expanded form should also stay available.

Is there already a link from the ast_t of the type (class/actor/trait) definition to its textual representation?

@jemc
Copy link
Member

jemc commented Aug 8, 2017

For whatever it's worth, when this came up in IRC I was of the opinion that showing invalid Pony syntax for the trait signature of the type is a documentation bug.

Is there already a link from the ast_t of the type (class/actor/trait) definition to its textual representation?

If I recall correctly, it links to the position of the start of the text, but I don't think you necessarily get the length, which you would need.


However, I think there is an easier way to solve this. When printing the type in docgen, you should be able to add logic to print the cap for a type, only if the following conditions are met:

    if((ast_id(type) != TK_TYPEPARAMREF) ||
      (opt->check.frame->constraint != NULL) ||
      (opt->check.frame->iftype_constraint != NULL))

In other words, don't print the type's cap if it is a type param ref outside of a constraint context.

@plietar
Copy link
Contributor

plietar commented Aug 9, 2017

Type params don't allow refcap in any context, even in constraints.

@jemc
Copy link
Member

jemc commented Aug 9, 2017

Right, thanks @plietar.

SeanTAllen added a commit that referenced this issue Aug 21, 2017
Per
#2145 (comment),
this shouldn't be allowed as its not legal Pony. As #2145 generally
comments on we only want legal code output in the documentation.

This addresses the specific issue in #2145, however, there are other
instances we might want to expand this to. I decided to limit to this
specifically as I think any change we make in other sections of docgen
would require a thorough auditing of output documentation to make sure
there weren't any unintended consquences.

Prior to this change, generated documentation for:

```pony
type Set[A: (Hashable #read & Equatable[A] #read)] is HashSet[A,
HashEq[A]]
```

would be

```pony
type Set[A: (Hashable #read & Equatable[A #read] #read)] is
  HashSet[A #read, HashEq[A #read] val] ref
```

now it is

```
type Set[A: (Hashable #read & Equatable[A] #read)] is
  HashSet[A, HashEq[A] val] ref
```
@SeanTAllen SeanTAllen self-assigned this Aug 21, 2017
jemc pushed a commit that referenced this issue Aug 21, 2017
Per
#2145 (comment),
this shouldn't be allowed as its not legal Pony. As #2145 generally
comments on we only want legal code output in the documentation.

This addresses the specific issue in #2145, however, there are other
instances we might want to expand this to. I decided to limit to this
specifically as I think any change we make in other sections of docgen
would require a thorough auditing of output documentation to make sure
there weren't any unintended consquences.

Prior to this change, generated documentation for:

```pony
type Set[A: (Hashable #read & Equatable[A] #read)] is HashSet[A,
HashEq[A]]
```

would be

```pony
type Set[A: (Hashable #read & Equatable[A #read] #read)] is
  HashSet[A #read, HashEq[A #read] val] ref
```

now it is

```
type Set[A: (Hashable #read & Equatable[A] #read)] is
  HashSet[A, HashEq[A] val] ref
```
@jemc
Copy link
Member

jemc commented Aug 21, 2017

This should be fixed by #2184.

@jemc jemc closed this as completed Aug 21, 2017
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

No branches or pull requests

4 participants