Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
100 changes: 61 additions & 39 deletions src/mtype.d
Original file line number Diff line number Diff line change
Expand Up @@ -6456,6 +6456,7 @@ extern (C++) final class TypeFunction : TypeNext
}

/********************************************
* Set 'purity' field of 'this'.
* Do this lazily, as the parameter types might be forward referenced.
*/
void purityLevel()
Expand All @@ -6464,73 +6465,94 @@ extern (C++) final class TypeFunction : TypeNext
if (tf.purity != PUREfwdref)
return;

/* Evaluate what kind of purity based on the modifiers for the parameters
/* Determine purity level based on mutability of t
* and whether it is a 'ref' type or not.
*/
tf.purity = PUREstrong; // assume strong until something weakens it

size_t dim = Parameter.dim(tf.parameters);
if (!dim)
return;
for (size_t i = 0; i < dim; i++)
static PURE purityOfType(bool isref, Type t)
{
Parameter fparam = Parameter.getNth(tf.parameters, i);
Type t = fparam.type;
if (!t)
continue;

if (fparam.storageClass & (STClazy | STCout))
{
tf.purity = PUREweak;
break;
}
if (fparam.storageClass & STCref)
if (isref)
{
if (t.mod & MODimmutable)
continue;
return PUREstrong;
if (t.mod & (MODconst | MODwild))
{
tf.purity = PUREconst;
continue;
}
tf.purity = PUREweak;
break;
return PUREconst;
return PUREweak;
}

t = t.baseElemOf();
if (!t.hasPointers())
continue;
if (t.mod & MODimmutable)
continue;

if (!t.hasPointers() || t.mod & MODimmutable)
return PUREstrong;

/* Accept immutable(T)[] and immutable(T)* as being strongly pure
*/
if (t.ty == Tarray || t.ty == Tpointer)
{
Type tn = t.nextOf().toBasetype();
if (tn.mod & MODimmutable)
continue;
return PUREstrong;
if (tn.mod & (MODconst | MODwild))
{
tf.purity = PUREconst;
continue;
}
return PUREconst;
}

/* The rest of this is too strict; fix later.
* For example, the only pointer members of a struct may be immutable,
* which would maintain strong purity.
* (Just like for dynamic arrays and pointers above.)
*/
if (t.mod & (MODconst | MODwild))
{
tf.purity = PUREconst;
return PUREconst;

/* Should catch delegates and function pointers, and fold in their purity
*/
return PUREweak;
}

purity = PUREstrong; // assume strong until something weakens it

/* Evaluate what kind of purity based on the modifiers for the parameters
*/
const dim = Parameter.dim(tf.parameters);
Lloop: foreach (i; 0 .. dim)
{
Parameter fparam = Parameter.getNth(tf.parameters, i);
Type t = fparam.type;
if (!t)
continue;

if (fparam.storageClass & (STClazy | STCout))
{
purity = PUREweak;
break;
}
switch (purityOfType((fparam.storageClass & STCref) != 0, t))
{
case PUREweak:
purity = PUREweak;
break Lloop; // since PUREweak, no need to check further

/* Should catch delegates and function pointers, and fold in their purity
case PUREconst:
purity = PUREconst;
Copy link
Member

@MartinNowak MartinNowak Oct 20, 2016

Choose a reason for hiding this comment

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

This could increase purity again, couldn't it? How about replacing the switch with purity = min(purity, purityOfType(isref, t))?

Copy link
Member Author

@WalterBright WalterBright Oct 21, 2016

Choose a reason for hiding this comment

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

This could increase purity again, couldn't it?

No, because any setting of PUREweak exits the loop.

How about replacing the switch with purity = min(purity, purityOfType(isref, t))?

Because there is no reason to continue with the loop once purity is set to PUREweak.

Copy link
Member

@MartinNowak MartinNowak Oct 23, 2016

Choose a reason for hiding this comment

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

Too clever usage of break/continue in switch nested in a loop IMO.
Hope you agree that break Lloop is better here :).

continue;

case PUREstrong:
continue;

default:
assert(0);
}
}

if (purity > PUREweak && tf.nextOf())
{
/* Adjust purity based on mutability of return type.
* https://issues.dlang.org/show_bug.cgi?id=15862
*/
tf.purity = PUREweak; // err on the side of too strict
break;
const purity2 = purityOfType(tf.isref, tf.nextOf());
if (purity2 < purity)
purity = purity2;
}
tf.purity = purity;
}

/********************************************
Expand Down
66 changes: 66 additions & 0 deletions test/runnable/test15862.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// https://issues.dlang.org/show_bug.cgi?id=15862

/*
PERMUTE_ARGS:
REQUIRED_ARGS: -O -release
*/


int* p() pure nothrow {return new int;}
int[] a() pure nothrow {return [0];}
Object o() pure nothrow {return new Object;}

immutable(int)* pn() pure nothrow {return new int;}
immutable(int)[] an() pure nothrow {return [0];}
immutable(Object) on() pure nothrow {return new Object;}

auto pa() pure nothrow {return new int;}
auto pb() pure nothrow {return cast(immutable(int)*)(new int);}

void main()
{
{
int* p1 = p();
int* p2 = p();

if (p1 is p2) assert(0);
Copy link
Member

Choose a reason for hiding this comment

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

assert(p1 is p2)?

Copy link
Member

Choose a reason for hiding this comment

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

To get those asserts to be ran in release.
It's trying to solve a common problem with assert: https://forum.dlang.org/thread/ed318016-16e9-965f-4d66-c0c94dd2564c@sociomantic.com
I'd say lets revisit when we fix said issue.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. Thanks.

Copy link
Member

@PetarKirov PetarKirov Oct 15, 2016

Choose a reason for hiding this comment

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

@MetaLang I think you meant assert(p1 !is p2).
I would use p1 !is p2 || assert(0) (and IIRC @andralex too according to TDPL).


int[] a1 = a();
int[] a2 = a();

if (a1 is a2) assert(0);

Object o1 = o();
Object o2 = o();

if (o1 is o2) assert(0);
}
{
auto p1 = pn();
auto p2 = pn();

if (p1 !is p2) assert(0);

auto a1 = an();
auto a2 = an();

if (a1 !is a2) assert(0);

auto o1 = on();
auto o2 = on();

if (o1 !is o2) assert(0);
}
{
auto p1 = pa();
auto p2 = pa();

if (p1 is p2) assert(0);
}
{
auto p1 = pb();
auto p2 = pb();

if (p1 !is p2) assert(0);
}
}