Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Issue 9230 - Incorrect implicit immutable conversion occurs in pure function #1418

Merged
merged 2 commits into from

3 participants

@9rnsr
Collaborator

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

hasMutableIndirectionParams() is based on the old TypeFunction::purityLevel() (before fixing issue 8408). So its result is consistent with TypeFunction::purityLevel() != PUREstrong in 2.060. Then it fixes the regression.

@9rnsr 9rnsr fix Issue 9230 - Incorrect implicit immutable conversion occurs in pu…
…re function

`hasMutableIndirectionParams()` is based on the old `TypeFunction::purityLevel()` (before fixing issue 8408). So its result is consistent with `TypeFunction::purityLevel() != PUREstrong` in 2.060. Then it *fixes* the regression.
afa7a54
src/mtype.c
@@ -5828,6 +5828,67 @@ void TypeFunction::purityLevel()
}
}
+/********************************************
+ * FIXME: This function is a workaround for fixing Bugzilla 9210.
+ * In 2.061, TypeFunction::purityLevel() improved to make more functions
+ * strong purity, but immutable conversion on return statemet had broken by that.
+ * Because, it is essentially unrelated to PUREstring. This function is
@yebblies Collaborator

s/PUREstring/PUREstrong ?

@9rnsr Collaborator
9rnsr added a note

Will fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@yebblies yebblies commented on the diff
src/statement.c
@@ -3954,7 +3954,9 @@ Statement *ReturnStatement::semantic(Scope *sc)
}
else if (tbret->ty != Tvoid)
{
- if (fd->isPureBypassingInference() == PUREstrong &&
@yebblies Collaborator

The function still has to be pure, no? Was this check moved somewhere else?

@9rnsr Collaborator
9rnsr added a note

Ouch, it's my mistake. We should keep the check whether fd is really a pure function or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@9rnsr 9rnsr Fixed the bug pointed out by Daniel Murphy.
Also updated test cases. By a recent change in 2.061 (4b2767e), direct returning of NewExp sometimes avoids immutable implicit conversion. So, test5081 should return mutable array values through local variables.
000f02e
@9rnsr
Collaborator

Updated.

@WalterBright WalterBright merged commit c42d35b into D-Programming-Language:master

1 check passed

Details default Pass: 10
@9rnsr 9rnsr referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@9rnsr 9rnsr referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@9rnsr 9rnsr referenced this pull request from a commit in 9rnsr/dmd
@9rnsr 9rnsr Revert 9rnsr/fix9230 and 9rnsr/fix8408
- Revert "Merge pull request #1418 from 9rnsr/fix9230"

  This reverts commit c42d35b, reversing
  changes made to 9f2d9ea.

- Revert "Merge pull request #1110 from 9rnsr/fix8408"

  This reverts commit b6a8093, reversing
  changes made to 3b06132.
b65d12a
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 28, 2012
  1. @9rnsr

    fix Issue 9230 - Incorrect implicit immutable conversion occurs in pu…

    9rnsr authored
    …re function
    
    `hasMutableIndirectionParams()` is based on the old `TypeFunction::purityLevel()` (before fixing issue 8408). So its result is consistent with `TypeFunction::purityLevel() != PUREstrong` in 2.060. Then it *fixes* the regression.
  2. @9rnsr

    Fixed the bug pointed out by Daniel Murphy.

    9rnsr authored
    Also updated test cases. By a recent change in 2.061 (4b2767e), direct returning of NewExp sometimes avoids immutable implicit conversion. So, test5081 should return mutable array values through local variables.
This page is out of date. Refresh to see the latest.
View
61 src/mtype.c
@@ -5828,6 +5828,67 @@ void TypeFunction::purityLevel()
}
}
+/********************************************
+ * FIXME: This function is a workaround for fixing Bugzilla 9210.
+ * In 2.061, TypeFunction::purityLevel() improved to make more functions
+ * strong purity, but immutable conversion on return statemet had broken by that.
+ * Because, it is essentially unrelated to PUREstrong. This function is
+ * necessary to check the convertibility.
+ */
+bool TypeFunction::hasMutableIndirectionParams()
+{
+ TypeFunction *tf = this;
+ size_t dim = Parameter::dim(tf->parameters);
+ for (size_t i = 0; i < dim; i++)
+ {
+ Parameter *fparam = Parameter::getNth(tf->parameters, i);
+ if (fparam->storageClass & STClazy)
+ {
+ return true;
+ }
+ if (fparam->storageClass & STCout)
+ {
+ return true;
+ }
+ if (!fparam->type)
+ continue;
+ if (fparam->storageClass & STCref)
+ {
+ if (!(fparam->type->mod & (MODconst | MODimmutable | MODwild)))
+ return true;
+ if (fparam->type->mod & MODconst)
+ return true;
+ }
+ Type *t = fparam->type->toBasetype();
+ if (!t->hasPointers())
+ continue;
+ if (t->mod & (MODimmutable | MODwild))
+ continue;
+ /* 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.
+ */
+ if (t->mod & MODconst)
+ return true;
+ Type *tn = t->nextOf();
+ if (tn)
+ { tn = tn->toBasetype();
+ if (tn->ty == Tpointer || tn->ty == Tarray)
+ { /* Accept immutable(T)* and immutable(T)[] as being strongly pure
+ */
+ if (tn->mod & (MODimmutable | MODwild))
+ continue;
+ if (tn->mod & MODconst)
+ return true;
+ }
+ }
+ /* Should catch delegates and function pointers, and fold in their purity
+ */
+ return true;
+ }
+ return false;
+}
+
/********************************
* 'args' are being matched to function 'this'
View
1  src/mtype.h
@@ -637,6 +637,7 @@ struct TypeFunction : TypeNext
Type *syntaxCopy();
Type *semantic(Loc loc, Scope *sc);
void purityLevel();
+ bool hasMutableIndirectionParams();
void toDecoBuffer(OutBuffer *buf, int flag);
void toCBuffer(OutBuffer *buf, Identifier *ident, HdrGenState *hgs);
void toCBufferWithAttributes(OutBuffer *buf, Identifier *ident, HdrGenState* hgs, TypeFunction *attrs, TemplateDeclaration *td);
View
5 src/statement.c
@@ -3954,7 +3954,10 @@ Statement *ReturnStatement::semantic(Scope *sc)
}
else if (tbret->ty != Tvoid)
{
- if (fd->isPureBypassingInference() == PUREstrong &&
@yebblies Collaborator

The function still has to be pure, no? Was this check moved somewhere else?

@9rnsr Collaborator
9rnsr added a note

Ouch, it's my mistake. We should keep the check whether fd is really a pure function or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ assert(fd->type->ty == Tfunction);
+ TypeFunction *tf = (TypeFunction *)fd->type;
+ if (fd->isPureBypassingInference() != PUREimpure &&
+ !tf->hasMutableIndirectionParams() &&
!exp->type->implicitConvTo(tret) &&
exp->type->invariantOf()->implicitConvTo(tret))
{
View
29 test/fail_compilation/test9230.d
@@ -0,0 +1,29 @@
+/*
+TEST_OUTPUT:
+---
+fail_compilation/test9230.d(12): Error: cannot implicitly convert expression (s) of type const(char[]) to string
+fail_compilation/test9230.d(18): Error: cannot implicitly convert expression (a) of type int[] to immutable(int[])
+fail_compilation/test9230.d(23): Error: cannot implicitly convert expression (a) of type int[] to immutable(int[])
+fail_compilation/test9230.d(28): Error: cannot implicitly convert expression (a) of type int[] to immutable(int[])
+---
+*/
+
+string foo(in char[] s) pure {
+ return s; //
+}
+
+/*pure*/ immutable(int[]) x1()
+{
+ int[] a = new int[](10);
+ return a;
+}
+/*pure */immutable(int[]) x2(int len)
+{
+ int[] a = new int[](len);
+ return a;
+}
+/*pure */immutable(int[]) x3(immutable(int[]) org)
+{
+ int[] a = new int[](org.length);
+ return a;
+}
View
25 test/runnable/xtest46.d
@@ -1963,21 +1963,36 @@ void test99()
}
/***************************************************/
+// 5081
void test5081()
{
- static pure immutable(int[]) x()
+ static pure immutable(int[]) x1()
{
- return new int[](10);
+ int[] a = new int[](10);
+ return a;
+ }
+ static pure immutable(int[]) x2(int len)
+ {
+ int[] a = new int[](len);
+ return a;
}
+ static pure immutable(int[]) x3(immutable(int[]) org)
+ {
+ int[] a = new int[](org.length);
+ return a;
+ }
+
+ immutable a1 = x1();
+ immutable a2 = x2(10);
+ immutable a3 = x3([1,2]);
- static pure int[] y()
+ static pure int[] y1()
{
return new int[](10);
}
- immutable a = x();
- immutable b = y();
+ immutable b1 = y1();
}
/***************************************************/
Something went wrong with that request. Please try again.