From 87940a649e3b4f757905015de95225d57ec2f27c Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Fri, 24 Jul 2020 18:12:41 -0700 Subject: Fix bugs related to mutating implementations of interface methods (#1461) There are two main bug fixes here: * We were failing to diagnose when code calls a `[mutating]` method on a value that doesn't support mutation (that is an r-value instead of an l-value). * We had a bug in the synthesis logic for interface requirements where we used the *result* type of the requirement in place of each of the *parameter* types. The second bug made synthesis often produce incorrect signatures with `void` parameters. The first bug meant that even though a `[mutating]` method should not be able to satisfy a non-`[mutating]` method (and we had code to enforce this for the "exact match" case), when we go on to try and synthesize a non-`[mutating]` method that satisfies the requirement by delegating to the user-written one, it would end up succeeding, because nothing was stopping a non-`[mutating]` method from calling a `[mutating]` one. In each case this code adds a fix and a test case to confirm it. --- source/slang/slang-check-overload.cpp | 49 ++++++++++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 3 deletions(-) (limited to 'source/slang/slang-check-overload.cpp') diff --git a/source/slang/slang-check-overload.cpp b/source/slang/slang-check-overload.cpp index 69baf9f75..0a1294965 100644 --- a/source/slang/slang-check-overload.cpp +++ b/source/slang/slang-check-overload.cpp @@ -358,11 +358,54 @@ namespace Slang return true; } + bool isEffectivelyMutating(CallableDecl* decl) + { + if(decl->hasModifier()) + return true; + + if(decl->hasModifier()) + return false; + + if(as(decl)) + return true; + + return false; + } + bool SemanticsVisitor::TryCheckOverloadCandidateDirections( - OverloadResolveContext& /*context*/, - OverloadCandidate const& /*candidate*/) + OverloadResolveContext& context, + OverloadCandidate const& candidate) { - // TODO(tfoley): check `in` and `out` markers, as needed. + if(candidate.flavor != OverloadCandidate::Flavor::Func) + return true; + + auto funcDeclRef = candidate.item.declRef.as(); + SLANG_ASSERT(funcDeclRef); + + // Note: This operation was originally introduced as + // a place to add checking around l-value-ness of arguments + // and parameters, but currently that checking is being + // done in other places. + // + // For now we will only use this step to check the + // mutability of the `this` parameter where necessary. + // + if(!isEffectivelyStatic(funcDeclRef.getDecl())) + { + if(isEffectivelyMutating(funcDeclRef.getDecl())) + { + if(context.baseExpr && !context.baseExpr->type.isLeftValue) + { + if(context.mode == OverloadResolveContext::Mode::ForReal) + { + getSink()->diagnose(context.loc, Diagnostics::mutatingMethodOnImmutableValue, funcDeclRef.getName()); + maybeDiagnoseThisNotLValue(context.baseExpr); + } + return false; + } + } + } + return true; } -- cgit v1.2.3