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

Bug7097 #577

Merged
merged 4 commits into from
Oct 29, 2012
Merged

Bug7097 #577

merged 4 commits into from
Oct 29, 2012

Conversation

MartinNowak
Copy link
Member

  • resolve opDollar for all array operator overloads (op[Index,Slice][, Unary, Assign, OpAssign])
  • throw out the deprecated resolving of 'length' to opDollary
    not doing so could break code that uses a length variable/property within
    one of the added expressions

}
assert(ad);

Dsymbol *dsym = search_function(ad, Id::opDollar);
Copy link
Member

Choose a reason for hiding this comment

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

It is probably worth using ad->search instead here, this allows opDollar to match any declaration instead of just functions.

See #203 for an example.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we should do that.

@9rnsr
Copy link
Contributor

9rnsr commented May 26, 2012

I'm surprised that bug 7097 is not yet fixed. I've found it in the reviewing of this pull.

@MartinNowak
Copy link
Member Author

I could probably get to this within this week. Are you working on that one too?

@9rnsr
Copy link
Contributor

9rnsr commented May 28, 2012

Yesterday, I've started to fix, but I found this pull after that. Then I stopped my work.
This pull is enough better.

@MartinNowak
Copy link
Member Author

Sorry, I should have created a backlink in bugzilla.

@dnadlinger
Copy link
Member

Can we get this in before the next release?

@9rnsr
Copy link
Contributor

9rnsr commented Jul 21, 2012

I'd like to merge this in 2.060. @dawgfoto , can you rebase this?

@andralex
Copy link
Member

@andralex
Copy link
Member

It was unclear to me from a cursory look - does opDollar work with infinite ranges? The following should work:

struct A {
    enum empty = false;
    struct Dullah {}
    Dullah opDollar() { return Dullah.init; }
    A opSlice(size_t from, Dullah) { ... }
}

In other words, it should be possible to slice an infinite range with r[1 .. $].

@9rnsr
Copy link
Contributor

9rnsr commented Sep 25, 2012

In other words, it should be possible to slice an infinite range with r[1 .. $].

It would work. All of operator overloading mechanisms would work just like syntactic rewriting.
So, r[1 .. $] should be rewritten to r.opSlice(1, r.opDollar()), and such call expression would work.

@andralex
Copy link
Member

Then I approve the design of this diff. Committers, please review implementation and merge. Thanks!

@9rnsr
Copy link
Contributor

9rnsr commented Sep 25, 2012

@dawgfoto, you had already rebased the commits two months ago, but I didn't noticed it. Sorry.
And now, it is necessary again.

So, please re-rebase commits, and write comment in here. Github would notify me the comment by e-mail, and I'll be able to merge this immediately.

@donc
Copy link
Collaborator

donc commented Sep 26, 2012

This has not worked up until now, because my original opDollar implementation was very conservative (I made the patch before the feature was actually accepted). It only worked with indexing, and I tried to make sure it as unintrusive as possible. This LGTM.

braddr pushed a commit to braddr/dmd that referenced this pull request Oct 22, 2012
Remove std.contracts (deprecated a long, long time ago).
@yebblies
Copy link
Member

@dawgfoto Bump for a rebase.

@MartinNowak
Copy link
Member Author

OK

- length has been deprecated for almost 2 years
- extending opDollar to all array indexing would
  affect code using length otherwise.
- extend ArrayScopeSymbol::search to handle opDollar for SliceExp
- merge existing code into resolveOpDollar helper functions
@9rnsr
Copy link
Contributor

9rnsr commented Oct 29, 2012

Thanks for your work, @dawgfoto!

9rnsr added a commit that referenced this pull request Oct 29, 2012
fix Issue 7097 - opDollar doesn't work with slicing
@9rnsr 9rnsr merged commit a88decb into dlang:master Oct 29, 2012
MartinNowak added a commit to MartinNowak/dmd that referenced this pull request Oct 29, 2012
- fix usage of templated opDollar with one-dimensional opSlice
9rnsr added a commit that referenced this pull request Oct 29, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants