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

Add Identifiers tuple to std.typecons.Tuple #1493

Closed
wants to merge 6 commits into from

Conversation

John-Colvin
Copy link
Contributor

Useful to have, trivial to implement, awkward to do from outside.

Useful to have, trivial to implement, awkward to do from outside
@John-Colvin
Copy link
Contributor Author

Ugh... We really need to agree on a solution to this tuple naming mess. Any suggestions as to how to word the docs for this to work around it for now?

*
* Examples:
* ----
* auto t = Tuple!(int, "num", string, "name", bool);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it just me, or does that code not compile? I get:

Error: type Tuple!(int, "num", string, "name", bool) has no value

Adding a paren, with either no args, or full args, fixes it though:

    //auto t0 = Tuple!(int, "num", string, "name", bool)();               //FAILS
    auto t1 = Tuple!(int, "num", string, "name", bool)();                 //OK
    //auto t2 = Tuple!(int, "num", string, "name", bool)(5);              //FAILS
    auto t3 = Tuple!(int, "num", string, "name", bool)(5, "hello", true); //OK

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm. One of the unittests does this:
Tuple!(int, "x", real, "y", double, "z", string) t;

which passes.

Copy link
Member

Choose a reason for hiding this comment

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

Tuple!(...) is a type. In a declaration, a type is followed by an identifier token, so the snippet from the unittest is perfectly fine. However, if the declaration has an initializer, that initializer is the expression followed by the = token. A type, on its own, is not a valid expression, which is why the proposed example code will not and should not compile.

To illustrate, it's the difference between:

int a = int; // ?

and

int a = int.init; // OK

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Woops, I had assumed that the no-parenthesis syntax worked there (like it does when using new)

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.

@JakobOvrum
Copy link
Member

I think expression tuples are typically not capitalized.

@John-Colvin
Copy link
Contributor Author

should be good to go now.

@9rnsr
Copy link
Contributor

9rnsr commented Aug 19, 2013

I'm against this change. When we add "identifiers" member, Tuple!(SomeType, "identifiers") would become not workable.

Essentially, I think std.typecons.Tuple should have few library-specific members as far as possible so they would conflict with user-specific field names. (Based on the principle, even current Types and expand would also be redundant. The former could be replaced by FieldTypeTuple!(typeof(tup)), and the latter could be replaced by tup[] ...Unfortunately they would be necessary for the backward compatible).

I think followings would be more generic and better ways.

  1. Add FieldIdentifierTuple which is similar to FieldTypeTuple in std.traits module.
  2. Modify std.typecons.Tuple implementation that for FieldIdentifierTuple workable.

@JakobOvrum
Copy link
Member

@9rnsr, I completely agree, except (again) I think it should be named fieldIdentifierTuple (not capitalized).

Also, tup.expand cannot be replaced with tup[], because as far as I can see, the latter requires opSlice, which cannot return an expression tuple, which is what expand is.

edit:

Unless opSlice could be an alias to an expression tuple and work as expected... though surely that would require a language change.

@monarchdodra
Copy link
Collaborator

What is the status of this pull? I mean... its already decided we don't want it...? Can I close this?

@John-Colvin
Copy link
Contributor Author

Closed for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants