Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Issue 6798 - Integrate overloadings for multidimensional indexing and slicing #443

Merged
merged 10 commits into from
@9rnsr
Collaborator

http://d.puremagic.com/issues/show_bug.cgi?id=6798

This patch is additional enhancement of opDollar (issue 3474 and #442).
Enable the mixing operator overloadings of indexing and slicing

a[$-1, 2..$] is translated to a.opIndex(a.opDollar!0 - 1, a.opSlice!1(2, a.opDollar!1))

The slicing lwr..upr inside bracket is converted to a.opSlice!(dimension)(lwr, upr).
This enhancement never break existing codes.

expression newly added overloading exists overloading
a[i0, ...] a.opIndex(i0, ...)
a[] a.opIndex() a.opSlice()
a[l..u] a.opIndex(a.opSlice!0(l, u)) a.opSlice(l, u)
a[l..u, ...] a.opIndex(a.opSlice!0(l, u), ...)
op a[i0, ...] a.opIndexUnary!op(i0, ...)
op a[] a.opIndexUnary!op() a.opSliceUnary!op()
op a[l..u] a.opIndexUnary!op(a.opSlice!0(l, u)) a.opSliceUnary!op(l, u)
op a[l..u, ...] a.opIndexUnary!op(a.opSlice!0(l, u), ...)
a[i0, ...] = v a.opIndexAssign(v, i0, ...)
a[] = v a.opIndexAssign(v) a.opSliceAssign(v)
a[l..u] = v a.opIndexAssign(v, a.opSlice!0(l, u)) a.opSliceAssign(v, l, u)
a[l..u, ...] = v a.opIndexAssign(v, a.opSlice!0(l, u), ...)
a[i0, ...] op= v a.opIndexOpAssign!op(v, i0, ...)
a[] op= v a.opIndexOpAssign!op(v) a.opSliceOpAssign!op(v)
a[l..u] op= v a.opIndexOpAssign!op(v, a.opSlice!0(l, u)) a.opSliceOpAssign!op(v, l, u)
a[l..u, ...] op= v a.opIndexOpAssign!op(v, a.opSlice!0(l, u), ...)
@braddr
Owner

This patch is failing on all 64 bit platforms:

http://d.puremagic.com/test-results/pull.ghtml?runid=12403
../druntime/import/core/stdc/stdarg.di(250): Error: cannot implicitly convert expression (parmn[0LU..tsize]) of type void[] to ulong
../druntime/import/core/stdc/stdarg.di(250): Error: cannot implicitly convert expression (p[0LU..tsize]) of type void[] to ulong
../druntime/import/core/stdc/stdarg.di(250): Error: expression p[(__error)] is void and has no value
../druntime/import/core/stdc/stdarg.di(283): Error: cannot implicitly convert expression (parmn[0LU..tsize]) of type void[] to ulong
../druntime/import/core/stdc/stdarg.di(283): Error: cannot implicitly convert expression (p[0LU..tsize]) of type void[] to ulong
../druntime/import/core/stdc/stdarg.di(283): Error: expression p[(__error)] is void and has no value
../druntime/import/core/stdc/stdarg.di(292): Error: cannot implicitly convert expression (parmn[0LU..tsize]) of type void[] to ulong
../druntime/import/core/stdc/stdarg.di(292): Error: cannot implicitly convert expression (p[0LU..tsize]) of type void[] to ulong
../druntime/import/core/stdc/stdarg.di(292): Error: expression p[(__error)] is void and has no value
std/format.d(3973): Error: template instance core.stdc.stdarg.va_arg!() error instantiating

@9rnsr
Collaborator

Revamped changes.

@andralex
Owner

(background: I'm doing a review of pull requests < 1000 as they tend to be controversial)

This is nice, self-consistent, and tastefully designed.

The question is scope and intended usage. This is a relatively hefty language addition, with a very narrow clientele. I'm not sure whether those clients actually need this stuff, and if they do, whether the current form is satisfactory to them. (For example one issue is that the slice limits are computed outside the index expression, so presumably they'd make certain usages difficult.) One other issue discussed in the enhancement request is that there might be a need for strides etc.

So I'm torn about this. One one hand it's a work of beauty. On the other it seems of low impact and unclear user base. Thoughts?

@braddr braddr referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@quickfur

I vote for merging this.

Or at the very least, some subset of this that supports multidimensional opDollar with slicing. Some background: I'm working on a multidimensional array implementation that uses the variadic form of opIndex to dereference array elements, and it works wonderfully (opDollar works as well). However, slicing syntax currently is not supported, so I've implemented a workaround using opSlice with two array arguments, so one could write A[[0,0,0]..[2,2,2]] for example, but the problem is that opDollar doesn't work correctly ($ always gets mapped to opDollar!0, so A[[0,0,0]..[$,$,$]] does not do the right thing when the array has different lengths along each dimension. I've tried various alternative ways to provide convenient slicing syntax, but none of them can support opDollar correctly.

It would be very nice if slicing can also support variadic arguments, since currently it's a hole in the language -- opIndex supports multidimensional arrays but opSlice doesn't.

@WalterBright
Owner

I'd like to include with this some code that is commented out for the moment that issues a warning and corrective action when opSlice, opSliceUnary, opSliceAssign, and opSliceOpAssign as those will now be obsolete.

@andralex
Owner

Ping on this. Now that time has passed we should be a tad wiser so maybe we have new insights in the matter. Thoughts?

@MaksimZh

Slices and strides make code more expressive and can save hours of debugging especially if one deals with matrices.

The design suggested is really good. It doesn't break existing code. It reduces the number of methods one has to implement (e.g. opSliceAssign is not needed anymore). Strides can be added later and will also not break anything.

I just can not understand why it is still not merged.

@9rnsr
Collaborator

Sorry I'm failing to rebase this change to git master. Currently this change will break test/runnable/aliasthis.d...

@don-clugston-sociomantic

This needs a rebase (perhaps a rebase -i). There are dozens of unrelated commits in here.

Otherwise, I think this is awesome. This finishes the job started with opDollar, and it's something we've needed to do eventually.

What happens if you do:
void foo( int {] x) { x[2..$, 4] = 7; }
after these changes to the parser? Do you still get a nice error message?

@9rnsr
Collaborator

And, now I have a small performance worry. To represent slice, it would need to pack the start and end of the slice.

struct S {
    auto opSlice(int x, int y) { return tuple(x, y); }
    void opIndex(int, Tuple!(int, int) slice) {}
}
S s;
s[1, 2..3];   // 2..3 will always need creating tuple object

Is this acceptable?

@9rnsr
Collaborator

OK. I finished rebasing.

@aitzkora

I vote also for merging this! multidimensional slicing is very interesting for people doing numerical computations!

@9rnsr
Collaborator

Finished rebasing.

@andralex
Owner

Is my understanding that this is a large breakage of existing uses of opSlice?

src/expression.c
((10 lines not shown))
for (size_t i = 0; i < ae->arguments->dim; i++)
{
+ if (i == 0)
+ e0 = extractOpDollarSideEffect(sc, ae);
+
+ Expression *e = (*ae->arguments)[i];
+ if (e->op == TOKinterval && !(slice && slice->isTemplateDeclaration()))
+ {
+ Lfallback:
+ if (ae->arguments->dim == 1)
+ {
+ ae->e1 = Expression::combine(e0, ae->e1);
+ return NULL;
+ }
+ else
+ {
+ ae->error("multi-dimentional slicing requires template opSlice");
@andralex Owner

s/dimentional/dimensional/

@9rnsr Collaborator
9rnsr added a note

Thanks, fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@andralex
Owner

For the record Walter and I are on board with this language addition as long as there's no breakage of existing code. Thanks!

@CyberShadow

The pull description states:

This enhancement never break existing codes.

Looking at the tests, it does seem to break alias-this'd tuples. Kenji confirmed it in a comment:

Currently this change will break test/runnable/aliasthis.d...

@andralex
Owner

@9rnsr can you work some magic to keep alias this working?

9rnsr added some commits
@9rnsr 9rnsr [Refactoring] ArrayScopeSymbol::search b12585b
@9rnsr 9rnsr [Refactoring] Operator overload handling in AssignExp
Change to recursive call for later fallback mechanism.
74f5e0e
@9rnsr 9rnsr [Refactoring] More better error propagation from resolveOpDollar 05e72b2
@9rnsr 9rnsr Add IntervalExp and fix parser, and disabled test for issue 6789
All of N-dimensional array operations are now translated to ArrayExp.
But currently they are immediately translated to SliceExp, so have no effect.
ca6643e
@9rnsr 9rnsr Translate IntervalExp in ArrayExp::arguments to opSlice!dim(lwr, upr) 6124e8d
@9rnsr 9rnsr Support multi-dimensional opIndex
If it fails, fall back to opSlice for backward compatibility.
1f308ef
@9rnsr 9rnsr Support multi-dimensional opIndexAssign
If it fails, fall back to opSliceAssign for backward compatibility.
1dd0746
@9rnsr 9rnsr Support multi-dimensional opIndexUnary
If it fails, fall back to opSliceUnary for backward compatibility.
d947c75
@9rnsr 9rnsr Support multi-dimensional opIndexOpAssign
If it fails, fall back to opSliceOpAssign for backward compatibility.
b1f0106
@9rnsr 9rnsr Unmask all test for issue 6798 e46e7e5
@9rnsr
Collaborator

Is my understanding that this is a large breakage of existing uses of opSlice?

No. New opSlice!dim(lwe, upr) will be preferred, but if it does not exist, old signature opSlice(lwr, upr) is used.

Currently this change will break test/runnable/aliasthis.d

It's an exception. Issue 8735 & 9709 was fixed in 2.065, but in the test case, A[0] should not invoke alias this, because A is the type Tuple9709!(1,int,"foo"), so A[0] should be analyzed as the zero-length static array type. I think that is a bug, so the fix is legitimate.

@andralex
Owner

OK, great to know! What's with the red though?

@andralex
Owner

Auto-merge toggled on

@9rnsr
Collaborator

What's with the red though?

It seems to be caused by the merge of #2256. This and that are both big, so something would be changed. I'll fix it.

@andralex
Owner

thanks!

@braddr
Owner

Pull updated, auto_merge toggled off

@9rnsr
Collaborator

Hmm, the red seems to have been fake. Could you re-toggle auto merge?

@andralex
Owner

Auto-merge toggled on

@andralex andralex merged commit 7148ab2 into from
@9rnsr
Collaborator

Thanks!

@yebblies yebblies commented on the diff
src/expression.h
@@ -1069,6 +1069,18 @@ class ArrayLengthExp : public UnaExp
void accept(Visitor *v) { v->visit(this); }
};
+struct IntervalExp : Expression
@yebblies Collaborator

This should have been a class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Orvid

Akk...... I see a serious design flaw with the multi-dimensional slicing, namely that it's impossible to determine whether the slice is the first or second argument from within the opSlice call, because it would change what the slice actually represents in things such as multi-dimensional arrays. I'm going to put this here for now just because there is an active topic on the mailing list over this, I'll update this momentarily to illustrate the issue better with examples.

So, it appears I have mis-understood what this PR actually does, and instead have the simple question of why? Why is this useful? Why would you want a slice operation to potentially call an opIndex, which is intended for selecting a single element? All I see in this PR is the actual addition of the ability to do multi-dimensional slicing, but even that is using an opIndex, when it should be introducing it as an opSlice. This also will cause issues due to the fact an operator can't currently be overloaded based on return type. There are situations, such as with matrices, that would mean that this syntax would require a dynamic array allocation for each index in order to both allow a user to both do that single dimensional slice and use it, as well as to still allow a multi-dimensional slice to be done.

This isn't the first addition to the D language that's been merged that really shouldn't have been, not without a more public review process. Perhaps requiring a formal review, such as what is required for the addition of Phobos modules, when adding or removing features or behaviors from the D language itself?

@quickfur

Huh? This pull has been referenced multiple times over the course of (at least) the past year in the forum. Is that not public enough? Or are we supposed to copy-n-paste the comments / code from the pull to the forum in order to make it more public? I'm confused.

@quickfur

Regarding your question about opIndex, the idea here is that opSlice!n(a,b) will return some kind of type that represents a slice from a to b in the n'th position, and opIndex will be overloaded to take arguments of this type and implement the actual slicing this way. The idea is that you should be able to call opSlice to get a type representing an index range, and then you can use that range object in multiple subsequent calls to opIndex. This is more flexible than limiting opSlice to do the actual slicing, then you have to keep track of the endpoints of the range manually each time.

@Orvid

The thing is that the way it's been being done is subjective to the actual reader of the forum topics, by using the same process that phobos modules go through, it produces a non-subjective result, in the form of votes.

@Orvid

But that still doesn't address the fact that your using opIndex, which is intended to retrieve a single element, to retrieve a slice instead. The same overload that is being called for opIndex could be done as an overload for opSlice instead, and would make much more sense to the reader of the code.

@quickfur

I don't know why you think that opIndex is only intended to retrieve a single element. In my own multidimensional array implementation, I've done opIndex(Range a, Range b, ...) because it's easier to consolidate mixed slices that way, e.g., opIndex(int a, Range b, int c, ...). It doesn't make sense to me to differentiate between indexing and slicing, because then how would you implement things like arr[1, 0..$, 2, 3, 2..5]? Rather, think of opIndex as implementing a[...], which can either be a single element (all arguments are indices), a full-dimensional slice (i.e., all arguments are ranges), or a sub-dimensional slice (some arguments are indices, some are ranges).

@Orvid

Your use of the ranges there is born out of the lack of a proper way to do it, opIndex should only ever be used to refer to a single element, a sub-dimensional slice is still a slice, and could even be treated as a full-dimensional slice. Whatever way you look at it, they are still slices and thus should be using opSlice.

@quickfur

I find the use of opIndex for both purposes more uniform, because it allows generic implementations that handle all cases in a single interface. Subdimensional slicing, in my implementation, actually returns a different type depending on how many arguments are slices and how many are indices (the types are segmented by dimension; the 0-dimensional case aliases to the element type); the case where all are indices is only one of the many possible combinations. Arbitrarily segmenting opIndex to two names introduces a lot of inelegance in the code, because now everything has to specially treat the case where all arguments happen to be indices, whereas now, it simply handles all cases together.

In any case, you're arguing with the wrong person; you should be asking Kenji the rationale for this design, since he's the one who implemented it! :)

@MasonMcGill

Great work folks! I do have a concern, though: it looks like you're bringing Python 2.0-style slice handling to D, even though it turned out there were limitations to that model that could have been avoided with a different design. It worries me that Julia, a language specifically designed ~15 years later to make up for the shortcomings in existing scientific platforms, chose an alternate approach. More discussion of that here:
http://forum.dlang.org/thread/upzdamhmxrrlsexgcdva@forum.dlang.org

It looks like I timed this pretty poorly, but I actually just wrote up a DIP that addresses the problem of "advanced" indexing in a more general way, and enables more MATLAB/Julia-style numerical features, while reducing language complexity and maintaining backwards compatibility:
http://wiki.dlang.org/DIP58

Again, I know the timing's horrible, after all the work that's gone into this, but I think it's important to get numerical syntax right, and I think a bit of discussion about the pros and cons of each approach would do the language some good.

Discussion receptacle:
http://forum.dlang.org/thread/clswrooryjcudncqbkje@forum.dlang.org

@9rnsr
Collaborator

@quickfur In the proposal, the member function opIndex will be used for every bracket access syntax (a[], a[i], a[i..j], a[$, i..j], etc). It's just syntactic rewriting rule, and it's not directly related to the code meaning of indexing, as the same as that opDollar is not directly related to the length or size of container.

@9rnsr
Collaborator

@MasonMcGill Sometimes .. operator had been proposed in the newsgroup, but it is not yet exist. So this proposal did not consider it.

@MasonMcGill

@9rnsr Understood. My request was for a discussion about why this approach is better than something like http://wiki.dlang.org/DIP58 , and whether this is meant to be the "last stop" for the evolution of numerical syntax in D, or part of a larger transition.

@Orvid

I believe that it is related to the code meaning of indexing, by introducing this under the name of opIndex, it's using the wrong term for the slicing operation that is being performed.

@9rnsr
Collaborator

@Orvid I agree it was related. But, by supporting multi-dimensional slicing, a[i..j] could also be considered as one dimension indexing with an interval.
So uniformly using the name opIndex for both a[i..j] and a[i..j, k] would be practically convenient. I don't think it's so big issue.

@don-clugston-sociomantic

@Orvid

I believe that it is related to the code meaning of indexing, by introducing this under the name of opIndex, it's using the wrong term for the slicing operation that is being performed.

IMHO opSlice would not be the correct term. The only sense in which it is a "slice" is that with an n-dimensional array, it doesn't reduce the dimensionality, whereas an "index" normally does. But we don't even enforce those semantics. For example, you could create something with an infinite number of dimensions, and you could make slicing reduce the dimensionality as well.

Really it is a strided slice, which is a totally different beast, neither an index nor a slice, it doesn't have much in common with normal slices. Pragmatically, the opIndex syntax generalizes, whereas the opSlice syntax does not.

@Orvid

While I do see your point that opSlice is not correct in all cases, there are cases where it would be the correct term. This PR adds the ability to use opIndex in places where opSlice was previously the preferred operator. It also adds an overload to opIndex that opSlice should have as well, for use in the cases where opSlice would be the correct term, unless part of the intention of this PR is to start a deprecation path for opSlice in favor of opIndex?

@JakobOvrum

Is there a corresponding documentation PR for this?

@quickfur

@Orvid No, in this PR opSlice performs the duty of translating an x..y range into an aggregate index object represent that range. This aggregate index is then translated into an actual slicing operation by opIndex. The idea is to unify slicing and indexing, not to deprecate anything over the other.

@9rnsr 9rnsr deleted the branch
@John-Colvin

documentation?

@quickfur

I'll see if I can work up a PR for the docs today.

@quickfur quickfur referenced this pull request in D-Programming-Language/dlang.org
Merged

Document multidimensional array op overloading #625

@Orvid Orvid commented on the diff
test/runnable/aliasthis.d
((6 lines not shown))
static assert(a[0] == 1);
//static assert(is(A[1] == int));
//static assert(is(a[1] == int));
- static assert(A[2] == "foo");
+ //static assert(A[2] == "foo");
@Orvid
Orvid added a note

What was the reasoning behind this? (only noticed it as I went to see how this handled both opSlice and opIndex being assigned)

@9rnsr Collaborator
9rnsr added a note

I don't remember the precise reason, but it had been interfere with the new operator overloading rule. And the test case was added without deep thinking when issue 8735 was fixed (actually it was added by me).

So I was disable it again because it would not introduce serious use code breaking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 16, 2014
  1. @9rnsr
  2. @9rnsr

    [Refactoring] Operator overload handling in AssignExp

    9rnsr authored
    Change to recursive call for later fallback mechanism.
  3. @9rnsr
  4. @9rnsr

    Add IntervalExp and fix parser, and disabled test for issue 6789

    9rnsr authored
    All of N-dimensional array operations are now translated to ArrayExp.
    But currently they are immediately translated to SliceExp, so have no effect.
  5. @9rnsr
  6. @9rnsr

    Support multi-dimensional opIndex

    9rnsr authored
    If it fails, fall back to opSlice for backward compatibility.
  7. @9rnsr

    Support multi-dimensional opIndexAssign

    9rnsr authored
    If it fails, fall back to opSliceAssign for backward compatibility.
  8. @9rnsr

    Support multi-dimensional opIndexUnary

    9rnsr authored
    If it fails, fall back to opSliceUnary for backward compatibility.
  9. @9rnsr

    Support multi-dimensional opIndexOpAssign

    9rnsr authored
    If it fails, fall back to opSliceOpAssign for backward compatibility.
  10. @9rnsr

    Unmask all test for issue 6798

    9rnsr authored
This page is out of date. Refresh to see the latest.
View
53 src/dsymbol.c
@@ -1390,13 +1390,14 @@ Dsymbol *ArrayScopeSymbol::search(Loc loc, Identifier *ident, int flags)
{
//printf("ArrayScopeSymbol::search('%s', flags = %d)\n", ident->toChars(), flags);
if (ident == Id::dollar)
- { VarDeclaration **pvar;
+ {
+ VarDeclaration **pvar;
Expression *ce;
L1:
-
if (td)
- { /* $ gives the number of elements in the tuple
+ {
+ /* $ gives the number of elements in the tuple
*/
VarDeclaration *v = new VarDeclaration(loc, Type::tsize_t, Id::dollar, NULL);
Expression *e = new IntegerExp(Loc(), td->objects->dim, Type::tsize_t);
@@ -1407,7 +1408,8 @@ Dsymbol *ArrayScopeSymbol::search(Loc loc, Identifier *ident, int flags)
}
if (type)
- { /* $ gives the number of type entries in the type tuple
+ {
+ /* $ gives the number of type entries in the type tuple
*/
VarDeclaration *v = new VarDeclaration(loc, Type::tsize_t, Id::dollar, NULL);
Expression *e = new IntegerExp(Loc(), type->arguments->dim, Type::tsize_t);
@@ -1418,34 +1420,36 @@ Dsymbol *ArrayScopeSymbol::search(Loc loc, Identifier *ident, int flags)
}
if (exp->op == TOKindex)
- { /* array[index] where index is some function of $
+ {
+ /* array[index] where index is some function of $
*/
IndexExp *ie = (IndexExp *)exp;
-
pvar = &ie->lengthVar;
ce = ie->e1;
}
else if (exp->op == TOKslice)
- { /* array[lwr .. upr] where lwr or upr is some function of $
+ {
+ /* array[lwr .. upr] where lwr or upr is some function of $
*/
SliceExp *se = (SliceExp *)exp;
-
pvar = &se->lengthVar;
ce = se->e1;
}
else if (exp->op == TOKarray)
- { /* array[e0, e1, e2, e3] where e0, e1, e2 are some function of $
+ {
+ /* array[e0, e1, e2, e3] where e0, e1, e2 are some function of $
* $ is a opDollar!(dim)() where dim is the dimension(0,1,2,...)
*/
ArrayExp *ae = (ArrayExp *)exp;
-
pvar = &ae->lengthVar;
ce = ae->e1;
}
else
+ {
/* Didn't find $, look in enclosing scope(s).
*/
return NULL;
+ }
while (ce->op == TOKcomma)
ce = ((CommaExp *)ce)->e2;
@@ -1458,7 +1462,8 @@ Dsymbol *ArrayScopeSymbol::search(Loc loc, Identifier *ident, int flags)
{
Type *t = ((TypeExp *)ce)->type;
if (t->ty == Ttuple)
- { type = (TypeTuple *)t;
+ {
+ type = (TypeTuple *)t;
goto L1;
}
}
@@ -1467,12 +1472,14 @@ Dsymbol *ArrayScopeSymbol::search(Loc loc, Identifier *ident, int flags)
* multiple times, it gets set only once.
*/
if (!*pvar) // if not already initialized
- { /* Create variable v and set it to the value of $
+ {
+ /* Create variable v and set it to the value of $
*/
VarDeclaration *v;
Type *t;
if (ce->op == TOKtuple)
- { /* It is for an expression tuple, so the
+ {
+ /* It is for an expression tuple, so the
* length will be a const.
*/
Expression *e = new IntegerExp(Loc(), ((TupleExp *)ce)->exps->dim, Type::tsize_t);
@@ -1481,18 +1488,10 @@ Dsymbol *ArrayScopeSymbol::search(Loc loc, Identifier *ident, int flags)
}
else if (ce->type && (t = ce->type->toBasetype()) != NULL &&
(t->ty == Tstruct || t->ty == Tclass))
- { // Look for opDollar
+ {
+ // Look for opDollar
assert(exp->op == TOKarray || exp->op == TOKslice);
- AggregateDeclaration *ad = NULL;
-
- if (t->ty == Tclass)
- {
- ad = ((TypeClass *)t)->sym;
- }
- else if (t->ty == Tstruct)
- {
- ad = ((TypeStruct *)t)->sym;
- }
+ AggregateDeclaration *ad = isAggregate(t);
assert(ad);
Dsymbol *s = ad->search(loc, Id::opDollar);
@@ -1521,7 +1520,8 @@ Dsymbol *ArrayScopeSymbol::search(Loc loc, Identifier *ident, int flags)
e = new DotTemplateInstanceExp(loc, ce, td->ident, tiargs);
}
else
- { /* opDollar exists, but it's not a template.
+ {
+ /* opDollar exists, but it's not a template.
* This is acceptable ONLY for single-dimension indexing.
* Note that it's impossible to have both template & function opDollar,
* because both take no arguments.
@@ -1545,7 +1545,8 @@ Dsymbol *ArrayScopeSymbol::search(Loc loc, Identifier *ident, int flags)
v->storage_class |= STCtemp;
}
else
- { /* For arrays, $ will either be a compile-time constant
+ {
+ /* For arrays, $ will either be a compile-time constant
* (in which case its value in set during constant-folding),
* or a variable (in which case an expression is created in
* toir.c).
View
276 src/expression.c
@@ -9719,7 +9719,9 @@ Expression *SliceExp::semantic(Scope *sc)
if (search_function(ad, Id::slice))
{
// Rewrite as e1.slice(lwr, upr)
- Expression *e0 = resolveOpDollar(sc, this);
+ Expression *ex = resolveOpDollar(sc, this);
+ if (ex->op == TOKerror)
+ return ex;
Expressions *a = new Expressions();
assert(!lwr || upr);
if (lwr)
@@ -9729,7 +9731,6 @@ Expression *SliceExp::semantic(Scope *sc)
}
e = new DotIdExp(loc, e1, Id::slice);
e = new CallExp(loc, e, a);
- e = combine(e0, e);
e = e->semantic(sc);
return e;
}
@@ -9993,6 +9994,50 @@ Expression *ArrayLengthExp::rewriteOpAssign(BinExp *exp)
return e;
}
+/*********************** IntervalExp ********************************/
+
+// Mainly just a placeholder
+
+IntervalExp::IntervalExp(Loc loc, Expression *lwr, Expression *upr)
+ : Expression(loc, TOKinterval, sizeof(IntervalExp))
+{
+ this->lwr = lwr;
+ this->upr = upr;
+}
+
+Expression *IntervalExp::syntaxCopy()
+{
+ return new IntervalExp(loc, lwr->syntaxCopy(), upr->syntaxCopy());
+}
+
+Expression *IntervalExp::semantic(Scope *sc)
+{
+#if LOGSEMANTIC
+ printf("IntervalExp::semantic('%s')\n", toChars());
+#endif
+ if (type)
+ return this;
+
+ Expression *le = lwr;
+ le = le->semantic(sc);
+ le = resolveProperties(sc, le);
+
+ Expression *ue = upr;
+ ue = ue->semantic(sc);
+ ue = resolveProperties(sc, ue);
+
+ if (le->op == TOKerror)
+ return le;
+ if (ue->op == TOKerror)
+ return ue;
+
+ lwr = le;
+ upr = ue;
+
+ type = Type::tvoid;
+ return this;
+}
+
/*********************** ArrayExp *************************************/
// e1 [ i1, i2, i3, ... ]
@@ -10014,9 +10059,6 @@ Expression *ArrayExp::syntaxCopy()
Expression *ArrayExp::semantic(Scope *sc)
{
- Expression *e;
- Type *t1;
-
#if LOGSEMANTIC
printf("ArrayExp::semantic('%s')\n", toChars());
#endif
@@ -10026,25 +10068,37 @@ Expression *ArrayExp::semantic(Scope *sc)
if (e1->op == TOKerror)
return e1;
- t1 = e1->type->toBasetype();
+ Type *t1 = e1->type->toBasetype();
if (t1->ty != Tclass && t1->ty != Tstruct)
- { // Convert to IndexExp
- if (arguments->dim != 1)
- { error("only one index allowed to index %s", t1->toChars());
- goto Lerr;
+ {
+ // Convert to IndexExp
+ Expression *e;
+ if (arguments->dim == 0)
+ {
+ e = new SliceExp(loc, e1, NULL, NULL);
+ }
+ else if (arguments->dim == 1 && (*arguments)[0]->op == TOKinterval)
+ {
+ IntervalExp *ie = (IntervalExp *)(*arguments)[0];
+ e = new SliceExp(loc, e1, ie->lwr, ie->upr);
+ }
+ else if (arguments->dim == 1)
+ {
+ e = new IndexExp(loc, e1, (*arguments)[0]);
+ }
+ else
+ {
+ error("only one index allowed to index %s", t1->toChars());
+ return new ErrorExp();
}
- e = new IndexExp(loc, e1, (*arguments)[0]);
return e->semantic(sc);
}
- e = op_overload(sc);
- if (!e)
- { error("no [] operator overload for type %s", e1->type->toChars());
- goto Lerr;
- }
- return e;
+ Expression *e = op_overload(sc);
+ if (e)
+ return e;
-Lerr:
+ error("no [] operator overload for type %s", e1->type->toChars());
return new ErrorExp();
}
@@ -10539,42 +10593,68 @@ Expression *AssignExp::semantic(Scope *sc)
ArrayExp *ae = (ArrayExp *)e1;
ae->e1 = ae->e1->semantic(sc);
ae->e1 = resolveProperties(sc, ae->e1);
- Expression *ae1old = ae->e1;
Type *t1 = ae->e1->type->toBasetype();
AggregateDeclaration *ad = isAggregate(t1);
if (ad)
{
- L1:
// Rewrite (a[i] = value) to (a.opIndexAssign(value, i))
if (search_function(ad, Id::indexass))
{
// Deal with $
- Expression *e0 = resolveOpDollar(sc, ae);
+ Expression *ex = resolveOpDollar(sc, ae);
+ if (!ex)
+ goto Lfallback;
+ if (ex->op == TOKerror)
+ return ex;
+
Expressions *a = (Expressions *)ae->arguments->copy();
a->insert(0, e2);
Expression *e = new DotIdExp(loc, ae->e1, Id::indexass);
e = new CallExp(loc, e, a);
- e = combine(e0, e);
- e = e->semantic(sc);
+ if (ae->arguments->dim == 0)
+ e = e->trySemantic(sc);
+ else
+ e = e->semantic(sc);
+ if (!e)
+ goto Lfallback;
return e;
}
- }
- // No opIndexAssign found yet, but there might be an alias this to try.
- if (ad && ad->aliasthis && t1 != att1)
- {
- if (!att1 && t1->checkAliasThisRec())
- att1 = t1;
- ae->e1 = resolveAliasThis(sc, ae->e1);
- t1 = ae->e1->type->toBasetype();
- ad = isAggregate(t1);
- if (ad)
- goto L1;
- }
+ // No opIndexAssign found yet, but there might be an alias this to try.
+ if (ad->aliasthis && t1 != ae->att1)
+ {
+ ArrayExp *aex = (ArrayExp *)ae->copy();
+ if (!aex->att1 && t1->checkAliasThisRec())
+ aex->att1 = t1;
+ aex->e1 = new DotIdExp(loc, ae->e1, ad->aliasthis->ident);
+ this->e1 = aex;
+ Expression *ex = this->trySemantic(sc);
+ if (ex)
+ return ex;
+ this->e1 = ae; // restore
+ }
- ae->e1 = ae1old; // restore
+ Lfallback:
+ if (ae->arguments->dim == 0)
+ {
+ // a[] = e2
+ SliceExp *se = new SliceExp(ae->loc, ae->e1, NULL, NULL);
+ se->att1 = ae->att1;
+ this->e1 = se;
+ return this->semantic(sc);
+ }
+ if (ae->arguments->dim == 1 && (*ae->arguments)[0]->op == TOKinterval)
+ {
+ // a[lwr..upr] = e2
+ IntervalExp *ie = (IntervalExp *)(*ae->arguments)[0];
+ SliceExp *se = new SliceExp(ae->loc, ae->e1, ie->lwr, ie->upr);
+ se->att1 = ae->att1;
+ this->e1 = se;
+ return this->semantic(sc);
+ }
+ }
}
/* Look for operator overloading of a[i..j]=value.
* Do it before semantic() otherwise the a[i..j] will have been
@@ -10585,17 +10665,17 @@ Expression *AssignExp::semantic(Scope *sc)
SliceExp *ae = (SliceExp *)e1;
ae->e1 = ae->e1->semantic(sc);
ae->e1 = resolveProperties(sc, ae->e1);
- Expression *ae1old = ae->e1;
Type *t1 = ae->e1->type->toBasetype();
AggregateDeclaration *ad = isAggregate(t1);
if (ad)
{
- L2:
// Rewrite (a[i..j] = value) to (a.opSliceAssign(value, i, j))
if (search_function(ad, Id::sliceass))
{
- Expression *e0 = resolveOpDollar(sc, ae);
+ Expression *ex = resolveOpDollar(sc, ae);
+ if (ex->op == TOKerror)
+ return ex;
Expressions *a = new Expressions();
a->push(e2);
assert(!ae->lwr || ae->upr);
@@ -10606,25 +10686,24 @@ Expression *AssignExp::semantic(Scope *sc)
}
Expression *e = new DotIdExp(loc, ae->e1, Id::sliceass);
e = new CallExp(loc, e, a);
- e = combine(e0, e);
e = e->semantic(sc);
return e;
}
- }
- // No opSliceAssign found yet, but there might be an alias this to try.
- if (ad && ad->aliasthis && t1 != att1)
- {
- if (!att1 && t1->checkAliasThisRec())
- att1 = t1;
- ae->e1 = resolveAliasThis(sc, ae->e1);
- t1 = ae->e1->type->toBasetype();
- ad = isAggregate(t1);
- if (ad)
- goto L2;
+ // No opSliceAssign found yet, but there might be an alias this to try.
+ if (ad->aliasthis && t1 != ae->att1)
+ {
+ SliceExp *aex = (SliceExp *)ae->copy();
+ if (!aex->att1 && t1->checkAliasThisRec())
+ aex->att1 = t1;
+ aex->e1 = new DotIdExp(loc, ae->e1, ad->aliasthis->ident);
+ this->e1 = aex;
+ Expression *ex = this->trySemantic(sc);
+ if (ex)
+ return ex;
+ this->e1 = ae; // restore
+ }
}
-
- ae->e1 = ae1old; // restore
}
/* With UFCS, e.f = value
@@ -11142,7 +11221,7 @@ Expression *AssignExp::semantic(Scope *sc)
if (!e2->implicitConvTo(e1->type))
{
/* Internal handling for the default initialization
- * of multi-dimentional static array:
+ * of multi-dimensional static array:
* T[2][3] sa; // = T.init; if T is zero-init
*/
// Treat e1 as one large array
@@ -13529,10 +13608,34 @@ Expression *resolveOpDollar(Scope *sc, ArrayExp *ae)
{
assert(!ae->lengthVar);
- Expression *e0 = extractOpDollarSideEffect(sc, ae);
+ Expression *e0 = NULL;
+
+ AggregateDeclaration *ad = isAggregate(ae->e1->type);
+ Dsymbol *slice = search_function(ad, Id::slice);
+ //printf("slice = %s %s\n", slice->kind(), slice->toChars());
for (size_t i = 0; i < ae->arguments->dim; i++)
{
+ if (i == 0)
+ e0 = extractOpDollarSideEffect(sc, ae);
+
+ Expression *e = (*ae->arguments)[i];
+ if (e->op == TOKinterval && !(slice && slice->isTemplateDeclaration()))
+ {
+ Lfallback:
+ if (ae->arguments->dim == 1)
+ {
+ ae->e1 = Expression::combine(e0, ae->e1);
+ return NULL;
+ }
+ else
+ {
+ ae->error("multi-dimensional slicing requires template opSlice");
+ return new ErrorExp();
+ }
+ }
+ //printf("[%d] e = %s\n", i, e->toChars());
+
// Create scope for '$' variable for this dimension
ArrayScopeSymbol *sym = new ArrayScopeSymbol(sc, ae);
sym->loc = ae->loc;
@@ -13541,23 +13644,61 @@ Expression *resolveOpDollar(Scope *sc, ArrayExp *ae)
ae->lengthVar = NULL; // Create it only if required
ae->currentDimension = i; // Dimension for $, if required
- Expression *e = (*ae->arguments)[i];
e = e->semantic(sc);
e = resolveProperties(sc, e);
- if (!e->type)
- ae->error("%s has no value", e->toChars());
+
if (ae->lengthVar && sc->func)
{
// If $ was used, declare it now
Expression *de = new DeclarationExp(ae->loc, ae->lengthVar);
- e = new CommaExp(Loc(), de, e);
+ de = de->semantic(sc);
+ e0 = Expression::combine(e0, de);
+ }
+ sc = sc->pop();
+
+ if (e->op == TOKinterval)
+ {
+ IntervalExp *ie = (IntervalExp *)e;
+
+ Objects *tiargs = new Objects();
+ Expression *edim = new IntegerExp(ae->loc, i, Type::tsize_t);
+ edim = edim->semantic(sc);
+ tiargs->push(edim);
+
+ Expressions *fargs = new Expressions();
+ fargs->push(ie->lwr);
+ fargs->push(ie->upr);
+
+ unsigned xerrors = global.startGagging();
+ unsigned oldspec = global.speculativeGag;
+ global.speculativeGag = global.gag;
+ sc = sc->push();
+ sc->speculative = true;
+ FuncDeclaration *fslice = resolveFuncCall(ae->loc, sc, slice, tiargs, ae->e1->type, fargs, 1);
+ sc = sc->pop();
+ global.speculativeGag = oldspec;
+ global.endGagging(xerrors);
+ if (!fslice)
+ goto Lfallback;
+
+ e = new DotTemplateInstanceExp(ae->loc, ae->e1, slice->ident, tiargs);
+ e = new CallExp(ae->loc, e, fargs);
e = e->semantic(sc);
}
+
+ if (!e->type)
+ {
+ ae->error("%s has no value", e->toChars());
+ e = new ErrorExp();
+ }
+ if (e->op == TOKerror)
+ return e;
+
(*ae->arguments)[i] = e;
- sc = sc->pop();
}
- return e0;
+ ae->e1 = Expression::combine(e0, ae->e1);
+ return ae;
}
/**************************************
@@ -13570,7 +13711,8 @@ Expression *resolveOpDollar(Scope *sc, SliceExp *se)
assert(!se->lengthVar);
assert(!se->lwr || se->upr);
- if (!se->lwr) return NULL;
+ if (!se->lwr)
+ return se;
Expression *e0 = extractOpDollarSideEffect(sc, se);
@@ -13586,7 +13728,10 @@ Expression *resolveOpDollar(Scope *sc, SliceExp *se)
e = e->semantic(sc);
e = resolveProperties(sc, e);
if (!e->type)
+ {
se->error("%s has no value", e->toChars());
+ return new ErrorExp();
+ }
(i == 0 ? se->lwr : se->upr) = e;
}
@@ -13594,12 +13739,13 @@ Expression *resolveOpDollar(Scope *sc, SliceExp *se)
{
// If $ was used, declare it now
Expression *de = new DeclarationExp(se->loc, se->lengthVar);
- se->lwr = new CommaExp(Loc(), de, se->lwr);
- se->lwr = se->lwr->semantic(sc);
+ de = de->semantic(sc);
+ e0 = Expression::combine(e0, de);
}
sc = sc->pop();
- return e0;
+ se->e1 = Expression::combine(e0, se->e1);
+ return se;
}
Expression *BinExp::reorderSettingAAElem(Scope *sc)
View
12 src/expression.h
@@ -1069,6 +1069,18 @@ class ArrayLengthExp : public UnaExp
void accept(Visitor *v) { v->visit(this); }
};
+struct IntervalExp : Expression
@yebblies Collaborator

This should have been a class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+{
+ Expression *lwr;
+ Expression *upr;
+
+ IntervalExp(Loc loc, Expression *lwr, Expression *upr);
+ Expression *syntaxCopy();
+ Expression *semantic(Scope *sc);
+
+ void accept(Visitor *v) { v->visit(this); }
+};
+
// e1[a0,a1,a2,a3,...]
class ArrayExp : public UnaExp
View
7 src/hdrgen.c
@@ -1625,6 +1625,13 @@ class PrettyPrintVisitor : public Visitor
buf->writestring(".length");
}
+ void visit(IntervalExp *e)
+ {
+ expToCBuffer(buf, hgs, e->lwr, PREC_assign);
+ buf->writestring("..");
+ expToCBuffer(buf, hgs, e->upr, PREC_assign);
+ }
+
void visit(ArrayExp *e)
{
expToCBuffer(buf, hgs, e->e1, PREC_primary);
View
2  src/lexer.h
@@ -177,6 +177,8 @@ enum TOK
TOKvector,
TOKpound,
+ TOKinterval,
+
TOKMAX
};
View
159 src/opover.c
@@ -238,12 +238,24 @@ Expression *op_overload(Expression *e, Scope *sc)
Dsymbol *fd = search_function(ad, Id::opIndexUnary);
if (fd)
{
- Expression *e0 = resolveOpDollar(sc, ae);
+ // Deal with $
+ Expression *ex = resolveOpDollar(sc, ae);
+ if (!ex)
+ goto Lfallback;
+ if (ex->op == TOKerror)
+ {
+ result = ex;
+ return;
+ }
Objects *tiargs = opToArg(sc, e->op);
result = new DotTemplateInstanceExp(e->loc, ae->e1, fd->ident, tiargs);
result = new CallExp(e->loc, result, ae->arguments);
- result = Expression::combine(e0, result);
- result = result->semantic(sc);
+ if (ae->arguments->dim == 0)
+ result = result->trySemantic(sc);
+ else
+ result = result->semantic(sc);
+ if (!result)
+ goto Lfallback;
return;
}
@@ -264,6 +276,27 @@ Expression *op_overload(Expression *e, Scope *sc)
return;
}
e->att1 = NULL;
+
+ Lfallback:
+ if (ae->arguments->dim == 0)
+ {
+ // op(a[])
+ SliceExp *se = new SliceExp(ae->loc, ae->e1, NULL, NULL);
+ se->att1 = ae->att1;
+ e->e1 = se;
+ e->accept(this);
+ return;
+ }
+ if (ae->arguments->dim == 1 && (*ae->arguments)[0]->op == TOKinterval)
+ {
+ // op(a[lwr..upr])
+ IntervalExp *ie = (IntervalExp *)(*ae->arguments)[0];
+ SliceExp *se = new SliceExp(ae->loc, ae->e1, ie->lwr, ie->upr);
+ se->att1 = ae->att1;
+ e->e1 = se;
+ e->accept(this);
+ return;
+ }
}
}
else if (e->e1->op == TOKslice)
@@ -281,7 +314,13 @@ Expression *op_overload(Expression *e, Scope *sc)
Dsymbol *fd = search_function(ad, Id::opSliceUnary);
if (fd)
{
- Expression *e0 = resolveOpDollar(sc, se);
+ // Deal with $
+ Expression *ex = resolveOpDollar(sc, se);
+ if (ex->op == TOKerror)
+ {
+ result = ex;
+ return;
+ }
Expressions *a = new Expressions();
assert(!se->lwr || se->upr);
if (se->lwr)
@@ -292,7 +331,6 @@ Expression *op_overload(Expression *e, Scope *sc)
Objects *tiargs = opToArg(sc, e->op);
result = new DotTemplateInstanceExp(e->loc, se->e1, fd->ident, tiargs);
result = new CallExp(e->loc, result, a);
- result = Expression::combine(e0, result);
result = result->semantic(sc);
return;
}
@@ -382,39 +420,73 @@ Expression *op_overload(Expression *e, Scope *sc)
}
}
- void visit(ArrayExp *e)
+ void visit(ArrayExp *ae)
{
- //printf("ArrayExp::op_overload() (%s)\n", e->toChars());
- AggregateDeclaration *ad = isAggregate(e->e1->type);
+ //printf("ArrayExp::op_overload() (%s)\n", ae->toChars());
+ AggregateDeclaration *ad = isAggregate(ae->e1->type);
if (ad)
{
- Dsymbol *fd = search_function(ad, opId(e));
+ Dsymbol *fd = search_function(ad, opId(ae));
if (fd)
{
+ // Deal with $
+ Expression *ex = resolveOpDollar(sc, ae);
+ if (!ex)
+ goto Lfallback;
+ if (ex->op == TOKerror)
+ {
+ result = ex;
+ return;
+ }
+
/* Rewrite op e1[arguments] as:
* e1.opIndex(arguments)
*/
- Expression *e0 = resolveOpDollar(sc, e);
- result = new DotIdExp(e->loc, e->e1, fd->ident);
- result = new CallExp(e->loc, result, e->arguments);
- result = Expression::combine(e0, result);
- result = result->semantic(sc);
+ result = new DotIdExp(ae->loc, ae->e1, fd->ident);
+ result = new CallExp(ae->loc, result, ae->arguments);
+ if (ae->arguments->dim == 0)
+ result = result->trySemantic(sc);
+ else
+ result = result->semantic(sc);
+ if (!result)
+ goto Lfallback;
return;
}
+ if (ae->e1->op == TOKtype && ae->arguments->dim < 2)
+ goto Lfallback;
+
// Didn't find it. Forward to aliasthis
- if (ad->aliasthis && e->e1->type != e->att1)
+ if (ad->aliasthis && ae->e1->type != ae->att1)
{
/* Rewrite op(e1) as:
* op(e1.aliasthis)
*/
//printf("att arr e1 = %s\n", this->e1->type->toChars());
- Expression *e1 = new DotIdExp(e->loc, e->e1, ad->aliasthis->ident);
- UnaExp *ue = (UnaExp *)e->copy();
- if (!ue->att1 && e->e1->type->checkAliasThisRec())
- ue->att1 = e->e1->type;
+ Expression *e1 = new DotIdExp(ae->loc, ae->e1, ad->aliasthis->ident);
+ UnaExp *ue = (UnaExp *)ae->copy();
+ if (!ue->att1 && ae->e1->type->checkAliasThisRec())
+ ue->att1 = ae->e1->type;
ue->e1 = e1;
result = ue->trySemantic(sc);
+ if (result)
+ return;
+ }
+
+ Lfallback:
+ if (ae->arguments->dim == 0)
+ {
+ // a[]
+ SliceExp *se = new SliceExp(ae->loc, ae->e1, NULL, NULL);
+ result = se->semantic(sc);
+ return;
+ }
+ if (ae->arguments->dim == 1 && (*ae->arguments)[0]->op == TOKinterval)
+ {
+ // a[lwr..upr]
+ IntervalExp *ie = (IntervalExp *)(*ae->arguments)[0];
+ SliceExp *se = new SliceExp(ae->loc, ae->e1, ie->lwr, ie->upr);
+ result = se->semantic(sc);
return;
}
}
@@ -850,15 +922,28 @@ Expression *op_overload(Expression *e, Scope *sc)
Dsymbol *fd = search_function(ad, Id::opIndexOpAssign);
if (fd)
{
- Expression *e0 = resolveOpDollar(sc, ae);
+ // Deal with $
+ Expression *ex = resolveOpDollar(sc, ae);
+ if (!ex)
+ goto Lfallback;
+ if (ex->op == TOKerror)
+ {
+ result = ex;
+ return;
+ }
+
Expressions *a = (Expressions *)ae->arguments->copy();
a->insert(0, e->e2);
Objects *tiargs = opToArg(sc, e->op);
result = new DotTemplateInstanceExp(e->loc, ae->e1, fd->ident, tiargs);
result = new CallExp(e->loc, result, a);
- result = Expression::combine(e0, result);
- result = result->semantic(sc);
+ if (ae->arguments->dim == 0)
+ result = result->trySemantic(sc);
+ else
+ result = result->semantic(sc);
+ if (!result)
+ goto Lfallback;
return;
}
@@ -879,6 +964,27 @@ Expression *op_overload(Expression *e, Scope *sc)
return;
}
e->att1 = NULL;
+
+ Lfallback:
+ if (ae->arguments->dim == 0)
+ {
+ // a[] op= e2
+ SliceExp *se = new SliceExp(ae->loc, ae->e1, NULL, NULL);
+ se->att1 = ae->att1;
+ e->e1 = se;
+ e->accept(this);
+ return;
+ }
+ if (ae->arguments->dim == 1 && (*ae->arguments)[0]->op == TOKinterval)
+ {
+ // a[lwr..upr] op= e2
+ IntervalExp *ie = (IntervalExp *)(*ae->arguments)[0];
+ SliceExp *se = new SliceExp(ae->loc, ae->e1, ie->lwr, ie->upr);
+ se->att1 = ae->att1;
+ e->e1 = se;
+ e->accept(this);
+ return;
+ }
}
}
else if (e->e1->op == TOKslice)
@@ -896,7 +1002,13 @@ Expression *op_overload(Expression *e, Scope *sc)
Dsymbol *fd = search_function(ad, Id::opSliceOpAssign);
if (fd)
{
- Expression *e0 = resolveOpDollar(sc, se);
+ // Deal with $
+ Expression *ex = resolveOpDollar(sc, se);
+ if (ex->op == TOKerror)
+ {
+ result = ex;
+ return;
+ }
Expressions *a = new Expressions();
a->push(e->e2);
assert(!se->lwr || se->upr);
@@ -909,7 +1021,6 @@ Expression *op_overload(Expression *e, Scope *sc)
Objects *tiargs = opToArg(sc, e->op);
result = new DotTemplateInstanceExp(e->loc, se->e1, fd->ident, tiargs);
result = new CallExp(e->loc, result, a);
- result = Expression::combine(e0, result);
result = result->semantic(sc);
return;
}
View
40 src/parse.c
@@ -6512,45 +6512,29 @@ Expression *Parser::parsePostExp(Expression *e)
// array[lwr .. upr]
Expression *index;
Expression *upr;
+ Expressions *arguments = new Expressions();
inBrackets++;
nextToken();
- if (token.value == TOKrbracket)
- { // array[]
- inBrackets--;
- e = new SliceExp(loc, e, NULL, NULL);
- nextToken();
- }
- else
+ while (token.value != TOKrbracket && token.value != TOKeof)
{
index = parseAssignExp();
if (token.value == TOKslice)
- { // array[lwr .. upr]
+ {
+ // array[..., lwr..upr, ...]
nextToken();
upr = parseAssignExp();
- e = new SliceExp(loc, e, index, upr);
+ arguments->push(new IntervalExp(loc, index, upr));
}
else
- { // array[index, i2, i3, i4, ...]
- Expressions *arguments = new Expressions();
arguments->push(index);
- if (token.value == TOKcomma)
- {
- nextToken();
- while (token.value != TOKrbracket && token.value != TOKeof)
- {
- Expression *arg = parseAssignExp();
- arguments->push(arg);
- if (token.value == TOKrbracket)
- break;
- check(TOKcomma);
- }
- }
- e = new ArrayExp(loc, e, arguments);
- }
- check(TOKrbracket);
- inBrackets--;
+ if (token.value == TOKrbracket)
+ break;
+ check(TOKcomma);
}
+ check(TOKrbracket);
+ inBrackets--;
+ e = new ArrayExp(loc, e, arguments);
continue;
}
@@ -7427,5 +7411,7 @@ void initPrecedence()
precedence[TOKcomma] = PREC_expr;
precedence[TOKdeclaration] = PREC_expr;
+
+ precedence[TOKinterval] = PREC_assign;
}
View
2  src/visitor.h
@@ -217,6 +217,7 @@ class CastExp;
class VectorExp;
class SliceExp;
class ArrayLengthExp;
+class IntervalExp;
class ArrayExp;
class DotExp;
class CommaExp;
@@ -485,6 +486,7 @@ class Visitor
virtual void visit(VectorExp *e) { visit((UnaExp *)e); }
virtual void visit(SliceExp *e) { visit((UnaExp *)e); }
virtual void visit(ArrayLengthExp *e) { visit((UnaExp *)e); }
+ virtual void visit(IntervalExp *e) { visit((Expression *)e); }
virtual void visit(ArrayExp *e) { visit((UnaExp *)e); }
virtual void visit(DotExp *e) { visit((BinExp *)e); }
virtual void visit(CommaExp *e) { visit((BinExp *)e); }
View
10 test/fail_compilation/fail315.d
@@ -1,11 +1,11 @@
/*
TEST_OUTPUT:
---
-fail_compilation/fail315.d-mixin-16(16): Error: found ';' when expecting ']'
-fail_compilation/fail315.d-mixin-16(16): Error: found '}' when expecting ';' following return statement
-fail_compilation/fail315.d-mixin-16(16): Error: expression expected, not ')'
-fail_compilation/fail315.d-mixin-16(16): Error: found 'EOF' when expecting ')'
-fail_compilation/fail315.d-mixin-16(16): Error: found 'EOF' when expecting ';' following statement
+fail_compilation/fail315.d-mixin-16(16): Error: found ';' when expecting ','
+fail_compilation/fail315.d-mixin-16(16): Error: expression expected, not '}'
+fail_compilation/fail315.d-mixin-16(16): Error: found 'EOF' when expecting ','
+fail_compilation/fail315.d-mixin-16(16): Error: found 'EOF' when expecting ']'
+fail_compilation/fail315.d-mixin-16(16): Error: found 'EOF' when expecting ';' following return statement
fail_compilation/fail315.d-mixin-16(16): Error: found 'EOF' when expecting '}' following compound statement
fail_compilation/fail315.d(21): Error: template instance fail315.foo!() error instantiating
---
View
4 test/runnable/aliasthis.d
@@ -1268,11 +1268,11 @@ void test8735()
// 9709 case
alias A = Tuple9709!(1,int,"foo");
A a;
- static assert(A[0] == 1);
+ //static assert(A[0] == 1);
static assert(a[0] == 1);
//static assert(is(A[1] == int));
//static assert(is(a[1] == int));
- static assert(A[2] == "foo");
+ //static assert(A[2] == "foo");
@Orvid
Orvid added a note

What was the reasoning behind this? (only noticed it as I went to see how this handled both opSlice and opIndex being assigned)

@9rnsr Collaborator
9rnsr added a note

I don't remember the precise reason, but it had been interfere with the new operator overloading rule. And the test case was added without deep thinking when issue 8735 was fixed (actually it was added by me).

So I was disable it again because it would not introduce serious use code breaking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
static assert(a[2] == "foo");
}
View
174 test/runnable/opover2.d
@@ -823,6 +823,179 @@ void test10037()
}
/**************************************/
+// 6798
+
+struct Tuple6798(T...)
+{
+ T field;
+ alias field this;
+
+ bool opEquals(Tuple6798 rhs)
+ {
+ foreach (i, _; T)
+ {
+ if (this[i] != rhs[i])
+ return false;
+ }
+ return true;
+ }
+}
+auto tuple6798(T...)(T args)
+{
+ return Tuple6798!T(args);
+}
+
+int test6798a()
+{
+ //import std.typecons;
+ alias tuple6798 tuple;
+
+ static struct S1
+ {
+ auto opDollar(size_t dim)()
+ {
+ return 99;
+ }
+ auto opSlice(int dim)(int lwr, int upr)
+ {
+ return [dim, lwr, upr];
+ }
+
+ auto opIndex(A...)(A indices)
+ {
+ return tuple(" []", indices);
+ }
+ auto opIndexUnary(string op, A...)(A indices)
+ {
+ return tuple(op~"[]", indices);
+ }
+ auto opIndexAssign(A...)(string s, A indices)
+ {
+ return tuple("[] =", s, indices);
+ }
+ auto opIndexOpAssign(string op, A...)(string s, A indices)
+ {
+ return tuple("[]"~op~"=", s, indices);
+ }
+ }
+ S1 s1;
+ assert( s1[] == tuple(" []"));
+ assert( s1[10] == tuple(" []", 10));
+ assert( s1[10, 20] == tuple(" []", 10, 20));
+ assert( s1[10..20] == tuple(" []", [0, 10, 20]));
+ assert(+s1[] == tuple("+[]"));
+ assert(-s1[10] == tuple("-[]", 10));
+ assert(*s1[10, 20] == tuple("*[]", 10, 20));
+ assert(~s1[10..20] == tuple("~[]", [0, 10, 20]));
+ assert((s1[] ="x") == tuple("[] =", "x"));
+ assert((s1[10] ="x") == tuple("[] =", "x", 10));
+ assert((s1[10, 20] ="x") == tuple("[] =", "x", 10, 20));
+ assert((s1[10..20] ="x") == tuple("[] =", "x", [0, 10, 20]));
+ assert((s1[] +="x") == tuple("[]+=", "x"));
+ assert((s1[10] -="x") == tuple("[]-=", "x", 10));
+ assert((s1[10, 20]*="x") == tuple("[]*=", "x", 10, 20));
+ assert((s1[10..20]~="x") == tuple("[]~=", "x", [0, 10, 20]));
+ assert( s1[20..30, 10] == tuple(" []", [0, 20, 30], 10));
+ assert( s1[10, 10..$, $-4, $..2] == tuple(" []", 10, [1,10,99], 99-4, [3,99,2]));
+ assert(+s1[20..30, 10] == tuple("+[]", [0, 20, 30], 10));
+ assert(-s1[10, 10..$, $-4, $..2] == tuple("-[]", 10, [1,10,99], 99-4, [3,99,2]));
+ assert((s1[20..30, 10] ="x") == tuple("[] =", "x", [0, 20, 30], 10));
+ assert((s1[10, 10..$, $-4, $..2] ="x") == tuple("[] =", "x", 10, [1,10,99], 99-4, [3,99,2]));
+ assert((s1[20..30, 10] +="x") == tuple("[]+=", "x", [0, 20, 30], 10));
+ assert((s1[10, 10..$, $-4, $..2]-="x") == tuple("[]-=", "x", 10, [1,10,99], 99-4, [3,99,2]));
+
+ // opIndex exist, but opSlice for multi-dimensional doesn't.
+ static struct S2
+ {
+ auto opSlice(size_t dim)() { return [dim]; }
+ auto opSlice()(size_t lwr, size_t upr) { return [lwr, upr]; }
+
+ auto opIndex(A...)(A indices){ return [[indices]]; }
+ }
+ S2 s2;
+ static assert(!__traits(compiles, s2[] ));
+ assert(s2[1] == [[1]]);
+ assert(s2[1, 2] == [[1, 2]]);
+ assert(s2[1..2] == [1, 2]);
+ static assert(!__traits(compiles, s2[1, 2..3] ));
+ static assert(!__traits(compiles, s2[1..2, 2..3] ));
+
+ // opSlice for multi-dimensional exists, but opIndex for that doesn't.
+ static struct S3
+ {
+ auto opSlice(size_t dim)(size_t lwr, size_t upr) { return [lwr, upr]; }
+
+ auto opIndex(size_t n){ return [[n]]; }
+ auto opIndex(size_t n, size_t m){ return [[n, m]]; }
+ }
+ S3 s3;
+ static assert(!__traits(compiles, s3[] ));
+ assert(s3[1] == [[1]]);
+ assert(s3[1, 2] == [[1, 2]]);
+ static assert(!__traits(compiles, s3[1..2] ));
+ static assert(!__traits(compiles, s3[1, 2..3] ));
+ static assert(!__traits(compiles, s3[1..2, 2..3] ));
+
+ return 0;
+}
+
+int test6798b()
+{
+ static struct Typedef(T)
+ {
+ private T Typedef_payload = T.init;
+
+ alias a = Typedef_payload;
+
+ auto ref opIndex(this X, D...)(auto ref D i) { return a[i]; }
+ auto ref opSlice(this X )() { return a[]; }
+ auto ref opSlice(this X, B, E)(auto ref B b, auto ref E e)
+ {
+ assert(b == 0 && e == 3);
+ return a[b..e];
+ }
+
+ template opDispatch(string name)
+ {
+ // field or property function
+ @property auto ref opDispatch(this X)() { return mixin("a."~name); }
+ @property auto ref opDispatch(this X, V)(auto ref V v) { return mixin("a."~name~" = v"); }
+ }
+
+ static if (is(typeof(a) : E[], E))
+ {
+ auto opDollar() const { return a.length; }
+ }
+ }
+
+ Typedef!(int[]) dollar2;
+ dollar2.length = 3;
+ assert(dollar2.Typedef_payload.length == 3);
+ assert(dollar2[0 .. $] is dollar2[0 .. 3]);
+
+ return 0;
+}
+
+int test6798c()
+{
+ alias T = Tuple6798!(int, int);
+ auto n = T[].init;
+ static assert(is(typeof(n[0]) == Tuple6798!(int, int)));
+
+ return 0;
+}
+
+void test6798()
+{
+ static assert(test6798a() == 0); // CTFE check
+ test6798a();
+ static assert(test6798b() == 0);
+ test6798b();
+ static assert(test6798c() == 0);
+ test6798c();
+}
+
+/**************************************/
// 7641
mixin template Proxy7641(alias a)
@@ -1537,6 +1710,7 @@ int main()
test17();
test3789();
test10037();
+ test6798();
test7641();
test8434();
test18();
Something went wrong with that request. Please try again.