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

Issue 481 - Letting compiler determine length for fixed-length arrays #3615

Merged
merged 3 commits into from
Jan 28, 2015

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Jun 3, 2014

https://issues.dlang.org/show_bug.cgi?id=481

Add syntax support for partially typed variable declaration.

void main()
{
    // static array type
    int[$]   a1 = [1,2];    // int[2]
    auto[$]  a2 = [3,4,5];  // int[3]
    const[$] a3 = [6,7,8];  // const(int[3])

    // dynamic array type
    immutable[] a4 = [1,2];    // immutable(int)[]
    shared[]    a5 = [3,4,5];  // shared(int)[]
    // partially specified part is unqualified.

    // pointer type
    auto*  p1 = new int(3);  // int*
    const* p2 = new int(3);  // const(int)*

    // mixing
    auto[][$] x1 = [[1,2,3],[4,5]];  // int[][2]
    shared*[$] x2 = [new int(1), new int(2)];  // shared(int)*[2]
}

TemplateInstance *ti = se->sds->isTemplateInstance();
if (ti && ti->semanticRun == PASSsemantic && !ti->aliasdecl)
se->error("cannot infer type from %s %s, possible circular dependency", se->sds->kind(), se->toChars());
else
se->error("cannot infer type from %s %s", se->sds->kind(), se->toChars());
return Type::terror;
goto Lerror;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer you just returned new ErrorInitializer(); everywhere here instead of using goto.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@bearophile
Copy link

This looks like a small but nice improvement for the D language. I hope to see this merged in dmd 2.066.
Is code like this accepted?

void foo(T)(T a) @nogc {
    pragma(msg, T);
}
void main() @nogc {
    foo(cast(auto[$])[10, 20]);
}

@Dgame
Copy link

Dgame commented Jun 7, 2014

Nice work, I'm glad to see you found the time to work on it.
But @bearophile: The cast is still ugly. :/

@9rnsr
Copy link
Contributor Author

9rnsr commented Jun 7, 2014

@bearophile To me supporting deduction in cast-expression is subtle feature. Rather than that, we need to think decreasing use of casts.

@9rnsr
Copy link
Contributor Author

9rnsr commented Jun 7, 2014

Spec change: dlang/dlang.org#590

@bearophile
Copy link

@9rnsr: I am not asking for that feature. I was just curious to know if that works or not. And I agree with decreasing the usage of casts as much as possible. The recent double(x) syntax has allowed me to remove several casts.

@9rnsr
Copy link
Contributor Author

9rnsr commented Jun 7, 2014

This PR does not support $ deduction in cast expression.

@bearophile
Copy link

After the merge of this patch a common problem left in D code is:

void foo(in int x) {
    auto y = x;
    y++;
}

The current solution is long, requires a cast and is not fully DRY (because the 'x' name is repeated twice):

import std.traits;
void foo(in int x) {
    Unqual!(typeof(x)) y = x;
    y++;
}

Because we don't have a 'mutable' keyword:

void foo(in int x) {
    !const y = x;
    y++;
}

@yebblies
Copy link
Member

@bearophile

Actually it's really really easy:

    void foo(in int x) {
        int y = x;
        y++;
    }

or

    void foo(int x) {
        auto y = x;
        y++;
    }

@bearophile
Copy link

@yebblies: yes I considered those possibilities, but the first is not DRY for the type of x (but I sometimes use it). The second is worse because it leaves x mutable, that is bug-prone.

@WalterBright
Copy link
Member

Thanks, Kenji, for doing this. My reservation on this is I keep thinking there must be a better way than [$].

@WalterBright
Copy link
Member

After the merge of this patch a common problem left in D code is:

Please, let's keep PR discussions on topic. Other issues belong in bugzilla or the n.g.

@bearophile
Copy link

@WalterBright > Please, let's keep PR discussions on topic.

This nice enhancement allows D partial inference, allowing the programmer to omit useless information, and to specify what's important for the program:

int[$] a1 = [1, 2];
auto[$] a2 = [3, 4, 5];
const[$] a3 = [6, 7, 8];
immutable[] a4 = [1, 2];
shared[] a5 = [3, 4, 5];
auto* p1 = new int(3);
const* p2 = new int(3);

With "immutable[$]" you are specifying you want an immutable fixed-size array, that you want the compiler to infer its length, but you don't want to specify its item type. So you are omitting part of the information, and you are changing part of the information in a way different from the standard (because by default if you use auto you get a dynamic array, not a fixed size one).

So while the following problem is not covered by this enhancement, I don't agree it's off topic:

void foo(in int x) {
    auto y = x;
    y++;
}

It's a related problem, it's still a case where you want to partially specify a type, so you want to omit the type of the variable y but you want to change part of its type compared to what it gets by default if you use auto (auto makes it const, as the variable x). So using the same syntax as the enhancement request you get a syntax like this:

void foo(in int x) {
    mutable y = x;
    y++;
}

Currently in D there is no mutable keyword, so that's impossible. But when you design a new feature, like this partial inference, it's important to look at the wider picture.

@bearophile
Copy link

@WalterBright: >My reservation on this is I keep thinking there must be a better way than [$].

This is an old enhancement request, several people have discussed and given their mind to this topic, so far I have not seen better ideas. The $ symbol is already used to denote length so I think the syntax proposed here is sufficiently intuitive for D programmers. And there's need for this enhancement, now even more than before because @nogc allows us to spot the unnecessary heap allocations. The [$] syntax helps remove some heap allocations and at the same time to keep the code easy to write and not bug prone (because the programmer doesn't need to count the items in the array literal, so the code is more DRY).

Regarding this enhancement patch, as you see it's not just about [$]. It adds some more value on the table, like "immutable[]" and "auto*" that while not as useful as the [$] syntax, will make D code a little nicer.

@MasonMcGill
Copy link

Just a thought: wouldn't having static array literals (discussed in the links below) solve the problem for rvalues as well as lvalues, while this approach only solves the problem for lvalues?

Partially-typed variable declaration:

enum[$] imageSize = [200, 200];
crop(image, cast(size_t[$]) [0, 0], cast(size_t[$]) [100, 100]);

Static array literals:

enum imageSize = [200, 200]s;
crop(image, [0, 0]s, [100, 100]s);

Relevant links:

@9rnsr
Copy link
Contributor Author

9rnsr commented Jun 26, 2014

@MasonMcGill I think "static array literal syntax" is a bad idea.

@quickfur
Copy link
Member

quickfur commented Aug 7, 2014

@WalterBright Is your objection just to the syntax [$]? If so, what about [auto]? (meaning, "infer length for me")?

int[4] a1; // explicit length
int[auto] a2 = [1,2,3]; // implicit length = 3

@MetaLang
Copy link
Member

MetaLang commented Aug 7, 2014

@quickfur That looks a lot like an associative array declaration, but I guess you can tell that it's not from the right hand side.

@quickfur
Copy link
Member

quickfur commented Aug 7, 2014

Hmm, good point. It could be construed to mean "automatically infer key type". :-(
So it seems that [$] is still the best candidate for this feature.

@9rnsr
Copy link
Contributor Author

9rnsr commented Aug 15, 2014

Support partial type deduction in AA key. For example:

int[auto] aa = ["a":1, "b":2, "c":3];  // typeof(aa) will be int[string]

@quickfur
Copy link
Member

@9rnsr Very nice!! I like it very much. Does it also support auto[string] for type inference of value type?

@9rnsr
Copy link
Contributor Author

9rnsr commented Aug 16, 2014

@quickfur Yes.

@quickfur
Copy link
Member

Awesome!

@andralex
Copy link
Member

Let's pull this in. I approve, and @WalterBright implicitly approves by liking the feature and not finding a better syntax :).

@andralex
Copy link
Member

Auto-merge toggled on

andralex added a commit that referenced this pull request Jan 28, 2015
Issue 481 - Letting compiler determine length for fixed-length arrays
@andralex andralex merged commit fa296e9 into dlang:master Jan 28, 2015
@bearophile
Copy link

See https://issues.dlang.org/show_bug.cgi?id=14066 (found about one minute after installing the new patch).

@bearophile
Copy link

Is all this correct? I use a const[$] and I receive an immutable(char[5]).

void main() {
    const[$] s1 = "hello";
    pragma(msg, typeof(s1)); // immutable(char[5])
    char[$] s2 = "hello";    // OK?
    s2[0]++;                 // OK?
    pragma(msg, typeof(s2)); // char[5]

    const[$] s3 = "hello"w;
    pragma(msg, typeof(s3)); // immutable(char[5])
    wchar[$] s4 = "hello"w;  // OK?
    s4[0]++;                 // OK?

    pragma(msg, typeof(s4)); // wchar[5]

    const[$] s5 = "hello"d;
    pragma(msg, typeof(s5)); // immutable(char[5])
    dchar[$] s6 = "hello"d;  // OK?
    s6[0]++;                 // OK?
    pragma(msg, typeof(s6)); // dchar[5]

    char[] s7 = "hello";     // Error
}

dmd 2.067alpha gives:

immutable(char[5])
char[5]
immutable(wchar[5])
wchar[5]
immutable(dchar[5])
dchar[5]
test.d(17,17): Error: cannot implicitly convert expression ("hello") of type string to char[]

@bearophile
Copy link

Is this a good idea?

void main() {
    int[2][] m = [[1, 2], [3, 4], [5, 6]];
    foreach (auto[2] a; m) {}
    foreach (int[$] a; m) {}
}

@9rnsr
Copy link
Contributor Author

9rnsr commented Jan 28, 2015

void main() {
    const[$] s1 = "hello";
    pragma(msg, typeof(s1)); // immutable(char[5])

it's a bug in partial type deduction implementation. The s1 type should be deduced to const(char[5]) from the initializer.
I opened: https://issues.dlang.org/show_bug.cgi?id=14069

    char[$] s2 = "hello";    // OK?
    s2[0]++;                 // OK?
    pragma(msg, typeof(s2)); // char[5]

That's intended behavior. The s2 type is deduced to char[5], then each string characters will be copied into the static array elements.

@9rnsr
Copy link
Contributor Author

9rnsr commented Jan 28, 2015

Is this a good idea?

Legitimate enhancement. Please file it in bugzilla!

@bearophile
Copy link

@MartinNowak
Copy link
Member

Great to see this in.
Can we get a short note in the changelog please.

@ghost
Copy link

ghost commented Jan 29, 2015

@9rnsr: Does static[$] arr = [1, 2, 3] work? Note that I'm talking about the storage, not the type of the array (I'd prefer if we used the term fixed-length array instead of "static").

@9rnsr
Copy link
Contributor Author

9rnsr commented Jan 29, 2015

@AndrejMitrovic It works and properly allowed by the grammar.

@9rnsr
Copy link
Contributor Author

9rnsr commented Jan 29, 2015

@MartinNowak I'll do it in the documentation PR: dlang/dlang.org#590

@andralex
Copy link
Member

andralex commented Feb 2, 2015

I hate to say this but I just can't bring myself to agree with this feature - [$] but partial deduction, too.

  • Complicates the language without a corresponding increase in power
  • Small obscure feature of dubious usefulness that creates a precedent for other small obscure features of increasingly dubious usefulness - https://issues.dlang.org/show_bug.cgi?id=14070 is just the beginning. How about accepting stuff like that in function parameters, etc? It's easy to argue that on grounds of completeness, now that there's a foot in the door.

It is my opinion we should retire this feature before 2.067 is released. I am really sorry for not being more decisive on this and left all this work to be done.

@WalterBright
Copy link
Member

I agree with @andralex

@9rnsr
Copy link
Contributor Author

9rnsr commented Feb 2, 2015

私はこの変更が戻されようとすることには反対しない。
I'm not against to that the change is being reverted due to the 2.067 release.
2.067にこの機能を取り込まないという決定には私は同意できる。
I can agree with the decision that we won't add this new feature in the release.

しかし、私はあなたに怒っている。
However, Andrei, I'm angry to you.

あなたの行動はつまり、このPRをマージするときあなたはこの変更について深く考えていなかったことを示している。
You're wishing to revert this PR. It means that you had not seriously thought about this change when you clicked the merge button.

従って、私は今あなたのD言語に対する真剣さを疑っている。
Therefore now, I'm really doubt that your seriouslness for D.

あなたがDに対して真剣でないなら、私はあなたの主張全てを受け入れることが出来ない。
If you're not serious for D, I should say your all arguments are pointless.

このPRをマージするとき、あなたは何を考えていた?
What did you think when you merging this PR?

@9rnsr
Copy link
Contributor Author

9rnsr commented Feb 3, 2015

I reverted this PR in #4373. But @andralex, I'm still waiting your answer.

@skl131313
Copy link
Contributor

skl131313 commented Oct 1, 2016

@andralex

I'd like to reopen discussion for partial specialization, at the very least.

Small obscure feature of dubious usefulness that creates a precedent for other small obscure features of increasingly dubious usefulness - https://issues.dlang.org/show_bug.cgi?id=14070

For the case of "int[$]" this is true, as it would be an entirely new feature and would need to be propagated to places where declarations can occur. For partial specialization, it already exists for function templates. Thus partial specialization already exists in the language, but it only exists in functions. What I'm asking is to extend it to auto declarations for variables and in foreach statements.

void func(Auto)(Auto* ptr) // existing behavior
{
}

void func2(Auto)(Auto[string] aa) // existing behavior
{
}

int* p;
func(p);

int[string] aa;
func2(aa);

func(int.init); // err
func2(int.init); // err

@Dgame
Copy link

Dgame commented Oct 1, 2016

I don't quite get it, int.init is neither a pointer nor an assoc. array. Besides it would be wiser to open a thread in the ng.

@skl131313
Copy link
Contributor

skl131313 commented Oct 1, 2016

@Dgame

That's the point, int.init would just be an int value, which neither function allows do to the partial specialization.

http://ideone.com/0Ywjlx

What this pull request added was partial specialization to "auto":

auto* ptr = null; // ok
auto* ptr = 0;    // err

What's "ng"?

@Dgame
Copy link

Dgame commented Oct 1, 2016

NG = Newsgroup. Related Thread is: http://forum.dlang.org/post/bewroetnschakoqjzowa@forum.dlang.org

@skl131313
Copy link
Contributor

@Dgame yah I posted that and had no way to ping @andralex so I posted here

ibuclaw pushed a commit to ibuclaw/dmd that referenced this pull request Jul 10, 2022
Merge `stable` in `mater`

Signed-off-by: Iain Buclaw <ibuclaw@users.noreply.github.com>
Merged-on-behalf-of: Petar Kirov <PetarKirov@users.noreply.github.com>
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.