Skip to content

Set length variable for slice and index expressions during semantic #7682

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.

Sign up for GitHub

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

Merged
dlang-bot merged 1 commit into from
Feb 10, 2018
Merged

Set length variable for slice and index expressions during semantic #7682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/dmd/expressionsem.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5706,6 +5706,9 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
if (exp.type.equals(t1b))
exp.type = exp.e1.type;

// We might know $ now
setLengthVarIfKnown(exp.lengthVar, t1b);

if (exp.lwr && exp.upr)
{
exp.lwr = exp.lwr.optimize(WANTvalue);
Expand Down Expand Up @@ -5735,7 +5738,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor

exp.lowerIsLessThanUpper = (lwrRange.imax <= uprRange.imin);

//printf("upperIsInBounds = %d lowerIsLessThanUpper = %d\n", upperIsInBounds, lowerIsLessThanUpper);
//printf("upperIsInBounds = %d lowerIsLessThanUpper = %d\n", exp.upperIsInBounds, exp.lowerIsLessThanUpper);
}

result = exp;
Expand Down Expand Up @@ -6115,6 +6118,9 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
return setError();
}

// We might know $ now
setLengthVarIfKnown(exp.lengthVar, t1b);

if (t1b.ty == Tsarray || t1b.ty == Tarray)
{
Expression el = new ArrayLengthExp(exp.loc, exp.e1);
Expand Down
74 changes: 46 additions & 28 deletions src/dmd/optimize.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,52 @@ private Expression fromConstInitializer(int result, Expression e1)
return e;
}

/* It is possible for constant folding to change an array expression of
* unknown length, into one where the length is known.
* If the expression 'arr' is a literal, set lengthVar to be its length.
*/
package void setLengthVarIfKnown(VarDeclaration lengthVar, Expression arr)
{
if (!lengthVar)
return;
if (lengthVar._init && !lengthVar._init.isVoidInitializer())
return; // we have previously calculated the length
size_t len;
if (arr.op == TOK.string_)
len = (cast(StringExp)arr).len;
else if (arr.op == TOK.arrayLiteral)
len = (cast(ArrayLiteralExp)arr).elements.dim;
else
{
Type t = arr.type.toBasetype();
if (t.ty == Tsarray)
Copy link
Copy Markdown
Contributor

@wilzbach wilzbach Jan 15, 2018

Choose a reason for hiding this comment

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

Looks like this doesn't work.

Copy link
Copy Markdown
Member Author

@ibuclaw ibuclaw Jan 15, 2018

Choose a reason for hiding this comment

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

Control flow has likely moved this to the other function. Should still be kept around though, as it's a utility function.

len = cast(size_t)(cast(TypeSArray)t).dim.toInteger();
else
return; // we don't know the length yet
}
Expression dollar = new IntegerExp(Loc.initial, len, Type.tsize_t);
lengthVar._init = new ExpInitializer(Loc.initial, dollar);
lengthVar.storage_class |= STC.static_ | STC.const_;
}

/* Same as above, but determines the length from 'type'. */
package void setLengthVarIfKnown(VarDeclaration lengthVar, Type type)
{
if (!lengthVar)
return;
if (lengthVar._init && !lengthVar._init.isVoidInitializer())
return; // we have previously calculated the length
Copy link
Copy Markdown
Contributor

@wilzbach wilzbach Jan 15, 2018

Choose a reason for hiding this comment

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

Apparently this caching doesn't work.

Copy link
Copy Markdown
Member Author

@ibuclaw ibuclaw Jan 15, 2018

Choose a reason for hiding this comment

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

You mean coverage wise? That's probably because semantic only calls it once. Doesn't make the safeguards any less correct.

size_t len;
Type t = type.toBasetype();
if (t.ty == Tsarray)
len = cast(size_t)(cast(TypeSArray)t).dim.toInteger();
else
return; // we don't know the length yet
Expression dollar = new IntegerExp(Loc.initial, len, Type.tsize_t);
lengthVar._init = new ExpInitializer(Loc.initial, dollar);
lengthVar.storage_class |= STC.static_ | STC.const_;
}

/*********************************
* Constant fold an Expression.
* Params:
Expand Down Expand Up @@ -1001,34 +1047,6 @@ extern (C++) Expression Expression_optimize(Expression e, int result, bool keepL
}
}

/* It is possible for constant folding to change an array expression of
* unknown length, into one where the length is known.
* If the expression 'arr' is a literal, set lengthVar to be its length.
*/
static void setLengthVarIfKnown(VarDeclaration lengthVar, Expression arr)
{
if (!lengthVar)
return;
if (lengthVar._init && !lengthVar._init.isVoidInitializer())
return; // we have previously calculated the length
size_t len;
if (arr.op == TOK.string_)
len = (cast(StringExp)arr).len;
else if (arr.op == TOK.arrayLiteral)
len = (cast(ArrayLiteralExp)arr).elements.dim;
else
{
Type t = arr.type.toBasetype();
if (t.ty == Tsarray)
len = cast(size_t)(cast(TypeSArray)t).dim.toInteger();
else
return; // we don't know the length yet
}
Expression dollar = new IntegerExp(Loc.initial, len, Type.tsize_t);
lengthVar._init = new ExpInitializer(Loc.initial, dollar);
lengthVar.storage_class |= STC.static_ | STC.const_;
}

override void visit(IndexExp e)
{
//printf("IndexExp::optimize(result = %d) %s\n", result, e.toChars());
Expand Down
71 changes: 45 additions & 26 deletions test/runnable/testbounds.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,34 +185,53 @@ void test13976()
{
int[] da = new int[](10);
int[10] sa;
size_t l = 0; // upperInRange
size_t u = 9; // | lowerLessThan
// | | check code
{ auto s = da[l .. u]; } // 0 0 (u <= 10 && l <= u )
{ auto s = da[1 .. u]; } // 0 0 (u <= 10 && l <= u )
{ auto s = da[l .. 10]; } // 0 0 (u <= 10 && l <= u )
{ auto s = da[1 .. u%5]; } // 0 0 (u <= 10 && l <= u%5)

{ auto s = da[l .. u]; } // 0 0 (u <= 10 && l <= u)
{ auto s = da[0 .. u]; } // 0 1 (u <= 10 )
{ auto s = da[l .. 10]; } // 0 0 (u <= 10 && l <= u)
{ auto s = da[0 .. u%5]; } // 0 1 (u%5 <= 10 )

{ auto s = sa[l .. u]; } // 0 0 (u <= 10 && l <= u )
{ auto s = sa[1 .. u]; } // 0 0 (u <= 10 && l <= u )
{ auto s = sa[l .. 10]; } // 1 0 ( l <= u )
{ auto s = sa[1 .. u%5]; } // 1 0 ( l <= u%5)

{ auto s = sa[l .. u]; } // 0 0 (u <= 10 && l <= u )
{ auto s = sa[0 .. u]; } // 0 1 (u <= 10 )
{ auto s = sa[l .. 10]; } // 1 0 ( l <= 10)
{ auto s = sa[0 .. u%5]; } // 1 1 NULL
enum size_t two = 2;
enum size_t five = 5;
size_t lb = 0; // upperInRange
size_t ub = 9; // | lowerLessThan
// | | check code
{ auto s = da[lb .. ub]; } // 0 0 (ub <= $ && lb <= ub )
{ auto s = da[1 .. ub]; } // 0 0 (ub <= $ && 1 <= ub )
{ auto s = da[lb .. 10]; } // 0 0 (10 <= $ && lb <= 10 )
{ auto s = da[1 .. ub%5]; } // 0 0 (ub%5 <= $ && 1 <= ub%5)

{ auto s = da[lb .. ub]; } // 0 0 (ub <= $ && lb <= ub )
{ auto s = da[0 .. ub]; } // 0 1 (ub <= $ )
{ auto s = da[lb .. 10]; } // 0 0 (10 <= $ && lb <= 10 )
{ auto s = da[0 .. ub%5]; } // 0 1 (ub%5 <= $ )

{ auto s = da[0 .. 0]; } // 0 1 (0 <= $ )
{ auto s = da[0 .. $]; } // 0 1 ($ <= $ )
Copy link
Copy Markdown
Member Author

@ibuclaw ibuclaw Feb 3, 2018

Choose a reason for hiding this comment

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

These two can be improved in a latter PR by using the check for 0 or $ from #7677.

{ auto s = da[1 .. $]; } // 0 0 ($ <= $ && 1 <= $ )
{ auto s = da[$ .. $]; } // 0 0 ($ <= $ && $ <= $ )
{ auto s = da[0 .. $/two]; } // 0 1 ($/2 <= $ )
{ auto s = da[$/two .. $]; } // 0 0 ($ <= $ && $/2 <= $ )
{ auto s = da[$/five .. $/two]; } // 0 0 ($/2 <= $ && $/5 <= $/2)

{ auto s = sa[lb .. ub]; } // 0 0 (ub <= 10 && lb <= ub )
{ auto s = sa[1 .. ub]; } // 0 0 (ub <= 10 && 1 <= ub )
{ auto s = sa[lb .. 10]; } // 1 0 ( lb <= 10 )
{ auto s = sa[1 .. ub%5]; } // 1 0 ( 1 <= ub%5)

{ auto s = sa[lb .. ub]; } // 0 0 (ub <= 10 && lb <= ub )
{ auto s = sa[0 .. ub]; } // 0 1 (ub <= 10 )
{ auto s = sa[lb .. 10]; } // 1 0 ( lb <= 10 )
{ auto s = sa[0 .. ub%5]; } // 1 1 NULL

{ auto s = sa[0 .. 0]; } // 1 1 NULL
{ auto s = sa[0 .. $]; } // 1 1 NULL
{ auto s = sa[1 .. $]; } // 1 1 NULL
{ auto s = sa[$ .. $]; } // 1 1 NULL
{ auto s = sa[0 .. $/two]; } // 1 1 NULL
{ auto s = sa[$/two .. $]; } // 1 1 NULL
{ auto s = sa[$/five .. $/two]; } // 1 1 NULL

int* p = new int[](10).ptr;
{ auto s = p[0 .. u]; } // 1 1 NULL
{ auto s = p[l .. u]; } // 1 0 (l <= u)
{ auto s = p[0 .. u%5]; } // 1 1 NULL
{ auto s = p[1 .. u%5]; } // 1 0 (l <= u%5)
{ auto s = p[0 .. ub]; } // 1 1 NULL
{ auto s = p[lb .. ub]; } // 1 0 (lb <= ub )
{ auto s = p[0 .. ub%5]; } // 1 1 NULL
{ auto s = p[1 .. ub%5]; } // 1 0 (1 <= ub%5)
{ auto s = p[0 .. 0]; } // 1 1 NULL
}

/******************************************/
Expand Down