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

Refactor Expression and TypeExpression #960

Closed
dalance opened this issue Sep 12, 2024 · 13 comments · Fixed by #1016
Closed

Refactor Expression and TypeExpression #960

dalance opened this issue Sep 12, 2024 · 13 comments · Fixed by #1016
Assignees
Labels
lang Language design

Comments

@dalance
Copy link
Collaborator

dalance commented Sep 12, 2024

Now type parameter overriding by built-in types is syntax error.

module ModuleA {
    inst u: ModuleB #(
        p: logic,
    );
}

This is because InstParameterItem can take Expression only.

InstParameterItem: Identifier [ Colon Expression ]; 

If we simply add TypeExpression to it, syntax ambiguous occurs because both Expression and TypeExpression can reach ScopedIdentifier.

InstParameterItem: Identifier [ Colon ( Expression | TypeExpression ) ]; 

So I think this case requires to refactor whole Expression and TypeExpression.

Refer: #958

@dalance dalance added the lang Language design label Sep 12, 2024
@dalance
Copy link
Collaborator Author

dalance commented Sep 12, 2024

There are some ideas.

  1. Merging Expression and TypeExpression
    This simplify each call site of Expression, but the definition of Expression becomes more difficult to analyze.
    For example, X[1] may be "part select of variable X" or "1-length packed array of type X".

  2. Specifing TypeExpression by any keyword or symbol
    For example, type keyword show the successor expression is TypeExpression.
    This may be bit redundant, especially at which the expression is clearly type expression.

module ModuleA {
    inst u: ModuleB #(
        p: type logic,
    );

    const X: type = logic;        // simple, but not consistent
    const Y: type = type logic;   // consistent and redundant
    const Z: type = type type(a); // type operator should be considered too
}

@nananapo
Copy link
Contributor

I think 2 is a simple and good solution, but I don't really want to write type keyword.
How about implementing 2 once, and making it possible to omit type keyword if it is clear that type of identifier is type in the future?

@nblei
Copy link
Contributor

nblei commented Sep 15, 2024

syntax ambiguous occurs because both Expression and TypeExpression can reach ScopedIdentifier.

This is a limitation of Parol, yes? Is there no way to force it to parse an ambiguous grammar, and then make the correct choice between TypeExpression vs Expression given additional context collected by the Veryl compiler?

@dalance
Copy link
Collaborator Author

dalance commented Sep 16, 2024

I think 2 is a simple and good solution, but I don't really want to write type keyword. How about implementing 2 once, and making it possible to omit type keyword if it is clear that type of identifier is type in the future?

Instead of adding type keyword to TypeExpression definition, type can be defined as a keyword which shows TypeExpression is used in Expression context like below:

Factor: type TypeExpression

If so, type parameter override and $size require type keyword, but const type and typedef don't.

@dalance
Copy link
Collaborator Author

dalance commented Sep 16, 2024

This is a limitation of Parol, yes? Is there no way to force it to parse an ambiguous grammar, and then make the correct choice between TypeExpression vs Expression given additional context collected by the Veryl compiler?

Yes. This is because Parol is LL(k) parser, but this is intentional decision.
In my experience implementing SystemVerilog parser, too many ambiguity causes parsing difficulty, depending on case impossible. So I think LL(k) is reasonable restriction.

I think choosing Expression or TypeExpression by context probably means "Merging Expression and TypeExpression".
Syntactically Expression accepts all expression and type expression, and semantic analyzer rejects unexpected combination of expression like logic + logic.

@taichi-ishitani
Copy link
Contributor

Instead of introducing type keyword, how about wrapping TypeExpression with </> or [/]?
For example:

inst u: ModuleB #(
    p: <logic>,
);

const X: type = <logic <32>>;

@nblei
Copy link
Contributor

nblei commented Sep 17, 2024

I like your idea, taichi.

I think maybe also the type keyword for TypeExpression will be less cumbersome if type inference is added. Then instead of const X: type = type ..., one could simply write const X = type ....

@dalance
Copy link
Collaborator Author

dalance commented Sep 18, 2024

Especially I like <> because it seems to be consistent with type generics syntax ::<>.

@dalance
Copy link
Collaborator Author

dalance commented Oct 7, 2024

Unfortunatly, <> can't be used because >> of <logic<32>> is interpreted as right shift operator.
So now [] is a candidate.
There is another idea :. It is consistent with type annotation of variable, but parameter override p: :logic may be bit strange.

inst u: ModuleB #(
    p: [logic<32>],
);

inst u: ModuleB #(
    p: :logic<32>,
);

const X: type = [logic<32>];
const X: type = :logic<32>;

@taichi-ishitani
Copy link
Contributor

I like [] but there are no clear reasons.

@dalance
Copy link
Collaborator Author

dalance commented Oct 7, 2024

My concern about [] is that it is used as array in many programming languages including Veryl itself.
For example, [logic] is looked like logic's array type.

@taichi-ishitani
Copy link
Contributor

taichi-ishitani commented Oct 8, 2024

Unfortunatly, <> can't be used because >> of <logic<32>> is interpreted as right shift operator.

To resolve this issue, how about introducing a limitation that Width and Array rules can accept Factor only but not Expression?

@dalance
Copy link
Collaborator Author

dalance commented Oct 8, 2024

I'm thinking merging Expression and TypeExpression may be reasonable.

Merging Expression and TypeExpression
This simplify each call site of Expression, but the definition of Expression becomes more difficult to analyze.
For example, X[1] may be "part select of variable X" or "1-length packed array of type X".

The above ambiguous does not actually occur because packed-array is x<1>.
Unpack-array like x[1] can't be used as TypeExpression as the same as SV.
I think it is reasonable restriction.

So the followng syntax can merge TypeExpression into Expression.

ExpressionIdentifier  : ScopedIdentifier ( Width | { Select } { Dot Identifier { Select } } );
Factor: IntegratedType

In the above syntax, ScopedIdentifier Width is always packed-array type, and ScopedIdentifier { Select } { Dot Identifier { Select } } is always normal factor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang Language design
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants