[swift-dev] CSGen/LinkedExprAnalyzer questions

[Jacob Bandes-Storch]

CS.getType(FVE) already gets rid of the optionality, so the
lookThrough/getAnyOptionalObjectType isn't necessary at all.

I'm still not sure whether this is the right solution. Are there no other
expression types that need similar treatment? Is ASTWalker really the right
tool for this job? These are questions for someone who knows CSGen better
than I...

Jacob

On Sat, Dec 31, 2016 at 1:31 PM, Robert Widmann <devteam.codafi at gmail.com>
wrote:

> Whoops, should only look through one level of optionality.  You get the
> gist.
>
> ~Robert Widmann
>
> On Dec 31, 2016, at 2:31 PM, Robert Widmann <devteam.codafi at gmail.com>
> wrote:
>
> I taught LinkedExprAnalyzer about ForceValueExpr
>
>       if (auto FVE = dyn_cast<ForceValueExpr>(expr)) {
>         LTI.collectedTypes.insert(CS.getType(FVE)->lookThroughAllAny
> OptionalTypes().getPointer());
>         return { false, expr };
>       }
>
> This seems to get it - didn’t check whether this regresses things, but
> this seems to be the right fix at a high level.
>
> ~Robert Widmann
>
>
> On Dec 31, 2016, at 11:22 AM, Mark Lacey via swift-dev <
> swift-dev at swift.org> wrote:
>
>
> On Dec 30, 2016, at 2:07 PM, Jacob Bandes-Storch via swift-dev <
> swift-dev at swift.org> wrote:
>
> This is a pretty great bug: https://bugs.swift.org/browse/SR-3483
>
>     let x: Double? = 1
>
>     // error: ambiguous reference to member '+'
>     let sum = x! + x! + x!
>
>     //  error: value of optional type 'Double?' not unwrapped; did you
> mean to use '!' or '?'?
>     let sum: Double = x! + x! + x!
>
> I've been poking around and I think the problem might be in
> LinkedExprAnalyzer.
>
>
> Yeah I think you’re onto something there. Just looking at the output of
> the constraint solver and where bindings are happening in matchTypes(), it
> looks like the constraint optimizer is trying to force the result type of
> both adds to ‘Double?’ rather than ‘Double’, so we only ever try to solve
> for that type.
>
> I haven’t looked at the constraint optimizer code in a while, so I don’t
> have a lot of insight into the best fix, but it’s clearly the problematic
> piece here.
>
> Mark
>
>
> This class collects type info about elements in a chain of binary
> operators to speed up constraint solving. It does so in this case by
> grabbing the DeclRefExpr's type: https://github.com/apple/swift/tree/
> 474096b9cbd6ff7ac998d7cea41d629512e25570#L230-L239
>
> However, since this is an ASTWalker, (I think) it automatically traverses
> into the ForceValueExpr without remembering that the type it finds inside
> (from the DeclRefExpr) should have one level of optionality removed when
> added to the constraint system.
>
> This theory sort of makes sense to me, but it doesn't explain why the
> simpler "let sum = x! + x!" typechecks correctly, because that goes
> through the same code path.
>
> Am I correct that the LinkedExprAnalyzer probably needs to make sure it
> doesn't keep the Optional when adding the type of a ForceValueExpr? Why
> wouldn't this also cause problems for a single binop application?
>
> Would it be more appropriate for LinkedExprAnalyzer to be an ASTVisitor
> (ExprVisitor) so that instead of just saying "yes, continue traversing
> downwards" (by returning {true, expr}), its ForceValueExpr case could
> recursively call visit() and then getAnyOptionalObjectType on the result?
>
>
> Semi-eptly,
> Jacob
> _______________________________________________
> swift-dev mailing list
> swift-dev at swift.org
> https://lists.swift.org/mailman/listinfo/swift-dev
>
>
> _______________________________________________
> swift-dev mailing list
> swift-dev at swift.org
> https://lists.swift.org/mailman/listinfo/swift-dev
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-dev/attachments/20161231/1131049e/attachment.html>