Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

fix Issue 8339 - is(typeof(...)) is reporting true when it's false du…

…e to nested function error

Avoid frame access check and disabled default construction in predicate templates.
  • Loading branch information...
commit 4a0546480078d51a0ad4dbf141c71bf818e034ec 1 parent 230c500
@9rnsr 9rnsr authored
Showing with 17 additions and 8 deletions.
  1. +4 −4 std/format.d
  2. +13 −4 std/traits.d
View
8 std/format.d
@@ -2270,19 +2270,19 @@ template hasToString(T, Char)
// X* does not have toString, even if X is aggregate type has toString.
enum hasToString = 0;
}
- else static if (is(typeof({ T val; FormatSpec!Char f; val.toString((const(char)[] s){}, f); })))
+ else static if (is(typeof({ T val = void; FormatSpec!Char f; val.toString((const(char)[] s){}, f); })))
@klickverbot Collaborator

Hm, this leads to an interesting question: In release mode, DMD would refuse to compile this code (at least it does in similar cases), because the access of uninitialized data is detected during data flow analysis somewhere in the optimizer. Is failing compilation actually legal behavior? The runtime semantics of the code would be completely undefined, and emitting a warning would obviously be a good idea, but can the compiler refuse to accept the code altogether? If so, it should also be legal to perform this check at an earlier stage in the frontend, which would break code like in this commit.

@9rnsr Collaborator
9rnsr added a note

First of all, the adding void initializer means "T is anyway constructible, but doesn't consider the way."
Particular types (struct that disabled default constructor, nested struct that requires enclosing frame pointer, etc.) would run additional checks, but void initializer can avoid them.

... Is failing compilation actually legal behavior?

The optimizer never check the codes inside typeof expressions.Then this static if never fails by the optimizer.

Bug 8339 is a lack of access check for frame pointer of nested struct, but it does not depends on runtime semantics, so I think your question is irrelevant.

@klickverbot Collaborator

dmd-internals is the right place for this discussion, see my message there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
{
enum hasToString = 4;
}
- else static if (is(typeof({ T val; val.toString((const(char)[] s){}, "%s"); })))
+ else static if (is(typeof({ T val = void; val.toString((const(char)[] s){}, "%s"); })))
{
enum hasToString = 3;
}
- else static if (is(typeof({ T val; val.toString((const(char)[] s){}); })))
+ else static if (is(typeof({ T val = void; val.toString((const(char)[] s){}); })))
{
enum hasToString = 2;
}
- else static if (is(typeof({ T val; return val.toString(); }()) S) && isSomeString!S)
+ else static if (is(typeof({ T val = void; return val.toString(); }()) S) && isSomeString!S)
{
enum hasToString = 1;
}
View
17 std/traits.d
@@ -2067,7 +2067,7 @@ template hasElaborateCopyConstructor(S)
else
{
enum hasElaborateCopyConstructor = is(typeof({
- S s;
+ S s = void;
return &s.__postblit;
})) || anySatisfy!(.hasElaborateCopyConstructor, typeof(S.tupleof));
}
@@ -2084,6 +2084,9 @@ unittest
struct S3 { uint num; S1 s; }
static assert(!hasElaborateCopyConstructor!S2);
static assert( hasElaborateCopyConstructor!S3);
+
+ struct S4 { @disable this(); this(int n){} this(this){} }
+ static assert( hasElaborateCopyConstructor!S4);
}
/**
@@ -2101,7 +2104,7 @@ template hasElaborateAssign(S)
else
{
enum hasElaborateAssign = is(typeof(S.init.opAssign(S.init))) ||
- is(typeof(S.init.opAssign({ return S.init; }()))) ||
+ is(typeof(S.init.opAssign({ S s = void; return s; }()))) ||
anySatisfy!(.hasElaborateAssign, typeof(S.tupleof));
}
}
@@ -2126,6 +2129,9 @@ unittest
@disable void opAssign(U)(ref U u);
}
static assert( hasElaborateAssign!S4);
+
+ struct S5 { @disable this(); this(int n){} S s; }
+ static assert( hasElaborateAssign!S5);
}
/**
@@ -2950,8 +2956,8 @@ static assert(!isAssignable!(string, char[]));
template isAssignable(Lhs, Rhs)
{
enum bool isAssignable = is(typeof({
- Lhs l;
- Rhs r;
+ Lhs l = void;
+ Rhs r = void;
l = r;
return l;
}));
@@ -2964,6 +2970,9 @@ unittest
static assert(!isAssignable!(int, long));
static assert(!isAssignable!(string, char[]));
+
+ struct S { @disable this(); this(int n){} }
+ static assert( isAssignable!(S, S));
}
@klickverbot
Collaborator

Hm, this leads to an interesting question: In release mode, DMD would refuse to compile this code (at least it does in similar cases), because the access of uninitialized data is detected during data flow analysis somewhere in the optimizer. Is failing compilation actually legal behavior? The runtime semantics of the code would be completely undefined, and emitting a warning would obviously be a good idea, but can the compiler refuse to accept the code altogether? If so, it should also be legal to perform this check at an earlier stage in the frontend, which would break code like in this commit.

@9rnsr
Collaborator

First of all, the adding void initializer means "T is anyway constructible, but doesn't consider the way."
Particular types (struct that disabled default constructor, nested struct that requires enclosing frame pointer, etc.) would run additional checks, but void initializer can avoid them.

... Is failing compilation actually legal behavior?

The optimizer never check the codes inside typeof expressions.Then this static if never fails by the optimizer.

Bug 8339 is a lack of access check for frame pointer of nested struct, but it does not depends on runtime semantics, so I think your question is irrelevant.

@klickverbot
Collaborator

dmd-internals is the right place for this discussion, see my message there.

Please sign in to comment.
Something went wrong with that request. Please try again.