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

[D] Add shortened function definition #3966

Closed
wants to merge 1 commit into from

Conversation

ichordev
Copy link
Contributor

Fixes #3894.
Changes:

  • Support shortened function definition syntax (e.g. int main() => 0;) in all its forms:
    • plain functions;
    • functions with one-line pre/postconditions;
    • template functions with template constraints.

@deathaxe
Copy link
Collaborator

When strictly following https://dlang.org/spec/function.html#function-bodies would shorthands with FunctionContracts need to be supported as well?

    int[] map(T)(int[] array) in(array) => 1;

    int[] map(T)(int[] array) out(array) => 2;

    int[] map(T)(int[] array) in(array) out(array) => 3;

grafik

Not sure if out(array) makes sense in that context.

@ichordev
Copy link
Contributor Author

When strictly following https://dlang.org/spec/function.html#function-bodies would shorthands with FunctionContracts need to be supported as well?

One-line pre/postconditions (AKA InOutContractExpressions) are already supported by my PR, as noted in the description I provided:

  • Support shortened function definition syntax in all its forms:
    • functions with one-line pre/postconditions;

Multi-line pre/postconditions (AKA InOutStatement) are not allowed to be the last contract before the shortened function syntax. My PR currently ignores this rule for simplicity.

Not sure if out(array) makes sense in that context.

You can't use out(someThing), you will get a compiler error. It's out(; someThing) or out(identifier; identifier == someOtherThing).

@deathaxe deathaxe changed the title [D] Add shortened function definition (#3894) [D] Add shortened function definition Apr 27, 2024
@deathaxe
Copy link
Collaborator

deathaxe commented May 2, 2024

You can't use out(someThing), you will get a compiler error. It's out(; someThing) or out(identifier; identifier == someOtherThing).

The point or question is not about content of parentheses, but rather whether the => being scoped illegal after the out(...) contract is wanted.

According to the syntax specs, I'd expect => to be scoped the same in all three lines of provided screenshot/code example, but the last 2 differ.

Note: I am reading specs only, I don't have an compiler or sdk installed for testing.

@ichordev
Copy link
Contributor Author

ichordev commented May 2, 2024

The point or question is not about content of parentheses, but rather whether the => being scoped illegal after the out(...) contract is wanted.

That is how the syntax already behaves with regular function declaration syntax. Modifying it is not even within the scope of this pull request.

According to the syntax specs, I'd expect => to be scoped the same in all three lines of provided screenshot/code example, but the last 2 differ.

Note: I am reading specs only, I don't have an compiler or sdk installed for testing.

If you read the grammar defined in the spec, you will see that the semicolon in out is required. It is also required by the pre-existing D syntax from this repository.
You are complaining that your second two examples are ‘scoped differently’, which is caused by you writing invalid code. If you don’t like the way the syntax parses syntactically invalid out contracts, then submit a PR to change it instead of complaining about it in this unrelated PR.

Furthermore, the pathetic cadence and bafflingly reasoned comments I have received from the reviewing of such trivial improvements has been stifling to say the least. There are a whole host of outdated aspects of this syntax that should’ve been fixed years ago, and now I see why nobody has. If I had expected this much friction over a barebones 4 line addition to support a feature added 3 years ago then I would’ve kept my changes upstream.

@ichordev ichordev closed this May 2, 2024
@deathaxe
Copy link
Collaborator

deathaxe commented May 3, 2024

According to the following rules extracted from https://dlang.org/spec/function.html#function-bodies

FuncDeclaration:
    StorageClassesopt BasicType FuncDeclarator FunctionBody
    AutoFuncDeclaration

AutoFuncDeclaration:
    StorageClasses Identifier FuncDeclaratorSuffix FunctionBody

ShortenedFunctionBody:
    InOutContractExpressionsopt => AssignExpression ;

InOutContractExpressions:
    InOutContractExpression
    InOutContractExpression InOutContractExpressions

InOutContractExpression:
    InContractExpression
    OutContractExpression

OutContractExpression:
    out ( ; AssertArguments )
    out ( Identifier ; AssertArguments )

a shortend function definition such as

int[] map(T)(int[] array) in(array) out(array) => 3;

appears to be valid, while with this PR => still is scoped invalid. The question was just, whether that's ok or whether a further change is required to scope => as operator/keyword as well after an OutContractExpression.

@ichordev
Copy link
Contributor Author

ichordev commented May 3, 2024

According to the following rules extracted from https://dlang.org/spec/function.html#function-bodies

OutContractExpression:
    out ( ; AssertArguments )
    out ( Identifier ; AssertArguments )

a [shortened] function definition such as
int[] map(T)(int[] array) in(array) out(array) => 3;
appears to be valid

out ( AssertArguments ) 'appears to be valid' does it?

OutContractExpression:
    out ( ; AssertArguments )
    out ( Identifier ; AssertArguments )

And yet out ( AssertArguments ) is not on this list. Perhaps you forgot to read the list before sending it?

with this PR => still is scoped invalid. The question was just, whether that's ok or whether a further change is required to scope => as operator/keyword as well after an OutContractExpression.

When you write an OutContractExpression, => is still scoped as an operator:

int[] map(T)(int[] array) out(; array) => 2;
int[] map(T)(int[] array) in(array) out(; array) => 3;

image
When you write out(array) it matches the start of an OutStatement, so it scopes => as invalid because it is expecting a BlockStatement. This is a helpful feature, and can even save you from looking stupid by insisting that a D syntax error 'appears to be valid' according to a grammar that you didn't even bother to read.

@michaelblyons
Copy link
Collaborator

save you from looking stupid by insisting that a D syntax error 'appears to be valid' according to a grammar that you didn't even bother to read.

Keep it civil. Your previous post was likewise somewhat lacking in civility.

@deathaxe
Copy link
Collaborator

deathaxe commented May 3, 2024

Actually I wouldn't have expected a semicolon within the parentheses to make a difference, which is IMHO an over pedantic treatment - not caused by this PR though. A syntax should ideally be able to handle incomplete expressions in sane ways without breaking syntax highlighting - to some extend at least. I therefore haven't realized the difference between OutStatement and OutContractExpression as I followed the latter path only.

Maybe some additional test cases could help verifying those valid forms/variants of a function definition work as expected.

With the misanderstanding clarified this PR is correct and valid.

@deathaxe
Copy link
Collaborator

deathaxe commented May 3, 2024

Keep it civil.

Well, true. Let's claim disappointment and unfamilarity with how open source projects try to ensure a least level of quality.

The truth is all maintainers here just being unpaid volunteers spending their spare time to help improving syntaxes. Most of us with limited time and scope with regards to familiar or used languages/syntaxes.

Despite those limitation we can't just merge anything reaching in, if parts of it seem unclear.

D is not one of the mainline syntaxes. It's also true, me not studying its specs to the deepest detail because I don't use or need it at all. Still trying to fix bugs, which seem simple to fix, despite limited knowledge about or motivation for it.

As such I can just appologize if the attempt to fill some gaps offended anyone.

@ichordev
Copy link
Contributor Author

ichordev commented May 3, 2024

Most of us with limited time and scope with regards to familiar or used languages/syntaxes.
Despite those limitation we can't just merge anything reaching in, if parts of it seem unclear.

Which is exactly why I explained from the outset that you were writing the wrong thing:

Not sure if out(array) makes sense in that context.

You can't use out(someThing), you will get a compiler error. It's out(; someThing) or out(identifier; identifier == someOtherThing).

Only to be completely run-over by the assertion that what I was saying was irrelevant:

The point or question is not about content of parentheses

This exercise in not listening to the person telling you exactly what's wrong and is just trying to help you is a waste of everyone's time, which as you pointed out is limited.

Still trying to fix bugs, which seem simple to fix, despite limited knowledge about or motivation for it.

There are examples listed in the spec that you could copy and slightly modify, there is an online D compiler to help you check whether your code is even valid. You can also read the 4 lines of the syntax that I changed and come to the logical conclusion that the parts of the syntax that I wrote interact with the aftermath of an out-contract in the exact same way as a regular function declaration, and therefore if something is wrong then it follows that it must be a pre-existing issue with the syntax. In fact, the way that I made shortened function declaration work is so similar to regular function declaration that it even accepts this completely invalid code:

int main()
out{} => 3;

It's also true, me not studying its specs to the deepest detail because I don't use or need it at all.

I am the one who actually uses this syntax on a day-to-day basis, and I was doing my best to provide you with accurate information about my changes from the beginning, which was ignored ad nauseam.

As such I can just appologize if the attempt to fill some gaps offended anyone.

Please do.

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

Successfully merging this pull request may close these issues.

[D] Add support for shortened function syntax
3 participants