Skip to content
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

Jump to bottom

fix issue 20138 - is expression not evaluating correctly? #10317

Merged
merged 4 commits into from Aug 20, 2019
Merged

fix issue 20138 - is expression not evaluating correctly? #10317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5b5219e
fix issue 20138 - is expression not evaluating correctly?
aG0aep6G Aug 18, 2019
6522b0a
fix cases where const/inout/inout(const) are stripped
aG0aep6G Aug 18, 2019
98d27f2
remove stray newline
aG0aep6G Aug 19, 2019
cc9b34c
add changelog entry for issue 20138
aG0aep6G Aug 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 22 additions & 0 deletions changelog/isexpression-const-inout-shared.dd
View file
Open in desktop
@@ -0,0 +1,22 @@
$(I IsExpression)s now correctly match combinations of `const`, `inout`, and `shared`.

With $(BUGZILLA 20138) fixed,
$(LINK2 $(ROOT_DIR)spec/expression.html#is_expression, $(I IsExpression)s) now
correctly match combinations of the qualifiers `const`, `inout`, and `shared`.

Examples that failed previously but pass now:

----
static assert(is(shared(const int) U == shared U) && is(U == const int));
static assert(is(shared(inout int) U == inout U) && is(U == shared int));
----

Note that the behavior of code like the following changes:

----
static if (is(T U == const U) { ...}
else static if (is(T U == shared const U)) { ... }
----

The second case will never be reached, because the first one now matches when
`T` is `shared const`. To get the old behavior, switch the order.
23 changes: 15 additions & 8 deletions src/dmd/dtemplate.d
View file
Open in desktop
Expand Up @@ -3248,25 +3248,29 @@ private MATCH deduceTypeHelper(Type t, Type* at, Type tparam)
*at = t.mutableOf().unSharedOf();
return MATCH.exact;
}
case X(MODFlags.const_, MODFlags.shared_ | MODFlags.const_):
case X(MODFlags.wild, MODFlags.shared_ | MODFlags.wild):
case X(MODFlags.wildconst, MODFlags.shared_ | MODFlags.wildconst):
// foo(const(U)) shared(const(T)) => shared(T)
// foo(inout(U)) shared(inout(T)) => shared(T)
// foo(inout(const(U))) shared(inout(const(T))) => shared(T)
{
*at = t.mutableOf();
return MATCH.exact;
}
case X(MODFlags.const_, 0):
case X(MODFlags.const_, MODFlags.wild):
case X(MODFlags.const_, MODFlags.wildconst):
case X(MODFlags.const_, MODFlags.shared_ | MODFlags.const_):
case X(MODFlags.const_, MODFlags.shared_ | MODFlags.wild):
case X(MODFlags.const_, MODFlags.shared_ | MODFlags.wildconst):
case X(MODFlags.const_, MODFlags.immutable_):
case X(MODFlags.wild, MODFlags.shared_ | MODFlags.wild):
case X(MODFlags.wildconst, MODFlags.shared_ | MODFlags.wildconst):
case X(MODFlags.shared_ | MODFlags.const_, MODFlags.immutable_):
// foo(const(U)) T => T
// foo(const(U)) inout(T) => T
// foo(const(U)) inout(const(T)) => T
// foo(const(U)) shared(const(T)) => shared(T)
// foo(const(U)) shared(inout(T)) => shared(T)
// foo(const(U)) shared(inout(const(T))) => shared(T)
// foo(const(U)) immutable(T) => T
// foo(inout(U)) shared(inout(T)) => shared(T)
// foo(inout(const(U))) shared(inout(const(T))) => shared(T)
// foo(shared(const(U))) immutable(T) => T
{
*at = t.mutableOf();
Expand All @@ -3281,10 +3285,14 @@ private MATCH deduceTypeHelper(Type t, Type* at, Type tparam)
case X(MODFlags.shared_, MODFlags.shared_ | MODFlags.const_):
case X(MODFlags.shared_, MODFlags.shared_ | MODFlags.wild):
case X(MODFlags.shared_, MODFlags.shared_ | MODFlags.wildconst):
case X(MODFlags.shared_ | MODFlags.const_, MODFlags.shared_):
// foo(shared(U)) shared(const(T)) => const(T)
// foo(shared(U)) shared(inout(T)) => inout(T)
// foo(shared(U)) shared(inout(const(T))) => inout(const(T))
{
*at = t.unSharedOf();
return MATCH.exact;
}
case X(MODFlags.shared_ | MODFlags.const_, MODFlags.shared_):
// foo(shared(const(U))) shared(T) => T
{
*at = t.unSharedOf();
Expand Down Expand Up @@ -3545,7 +3553,6 @@ MATCH deduceType(RootObject o, Scope* sc, Type tparam, TemplateParameters* param
// Found the corresponding parameter tp
if (!tp.isTemplateTypeParameter())
goto Lnomatch;

Type at = cast(Type)(*dedtypes)[i];
Type tt;
if (ubyte wx = wm ? deduceWildHelper(t, &tt, tparam) : 0)
Expand Down
16 changes: 16 additions & 0 deletions test/compilable/test20138.d
View file
Open in desktop
@@ -0,0 +1,16 @@
alias C = const int;
static assert(is(shared(C) U == shared U) && is(U == C));
static assert(is(shared(C) == shared U, U) && is(U == C));

alias I = inout int;
static assert(is(shared(I) U == shared U) && is(U == I));
static assert(is(shared(I) == shared U, U) && is(U == I));

alias IC = inout const int;
static assert(is(shared(IC) U == shared U) && is(U == IC));
static assert(is(shared(IC) == shared U, U) && is(U == IC));

alias S = shared int;
static assert(is(const(S) U == const U) && is(U == shared int));
static assert(is(inout(S) U == inout U) && is(U == shared int));
static assert(is(inout(const S) U == inout(const U)) && is(U == shared int));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest adding static assert(is(inout const S U == inout U) && is(U == const shared S));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggest adding static assert(is(inout const S U == inout U) && is(U == const shared S));

I'd rather leave that out for now, because it's less obvious how to handle it.

For the others, I just changed MATCH.constant to MATCH.exact. For this combination, deduceTypeHelper currently returns MATCH.nomatch. But it works in IFTI, so there must be something else going on. My guess is that it's handled by deduceTypeHelper, which isn't even called for is expressions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My guess is that it's handled by deduceTypeHelper, which isn't even called for is expressions.

That was supposed to be: My guess is that it's handled by deduceWildHelper

Copy link
Contributor

Choose a reason for hiding this comment

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

I was gonna say we could do something like this:

    *at = t.mutableOf().unSharedOf();
    if ((t.mod & MODFlags.const_) && !(tParam.mod & MODFlags.const_))
        *at = (*at).constOf();
    if ((t.mod & MODFlags.shared_) && !(tParam.mod & MODFlags.shared_))
        *at = (*at).sharedOf();
    if ((t.mod & MODFlags.immutable_) && !(tParam.mod & MODFlags.immutable_))
        *at = (*at).immutableOf();
    if ((t.mod & MODFlags.wild) && !(tParam.mod & MODFlags.wild))
        *at = (*at).wildOf();

However, apparently calling sharedOf() on a const type doesn't return a const shared type, and likewise for the other *Of()s. This somewhat disappoints me.