Tim,
FWIW, I think that one got incorrectly marked as a starter bug. I don’t think
the bug is fixable as written. You are right about the immediate cause, and I
think that the only thing we _can_ do is mark it conservatively, because that
part of the code isn’t type-checked at all, and probably can’t be, if it is
#if’d out due to it being for some other version of Swift. That variable could
be passed to mutable functions or operators which don’t actually exist in this
version of Swift, so there’s no knowing for certain whether it’s actually read
or written.
- Greg
> On Mar 18, 2016, at 2:18 PM, Timothy Wood via swift-dev <swift-dev@swift.org>
> wrote:
>
> Looking through the ‘StarterBug’ tag, I started looking into
> <https://bugs.swift.org/browse/SR-580
> <https://bugs.swift.org/browse/SR-580>>, in which:
>
> func foo(x: Int) -> Int {
> var result = x + 1
> #if NOT_ENABLED
> _ = result
> #endif
> return result
> }
>
> does not warn that “result" was never written to.
>
> The immediate cause seems to be in lib/Sema/MiscDiagnostics.cpp, in
> VarDeclUsageChecker::handleIfConfig(). This code walks the inactive #if
> blocks and does:
>
> // If we see a bound reference to a decl in an inactive #if block, then
> // conservatively mark it read and written. This will silence "variable
> // unused" and "could be marked let" warnings for it.
> if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
> VDUC.addMark(DRE->getDecl(), RK_Read|RK_Written);
> }
>
> In trying to replace that with something that really walks the Expr (by
> calling VDUC.walkToExprPre() instead or something of the like), I run into a
> bunch of checks like this:
>
> // Sema leaves some subexpressions null, which seems really unfortunate. It
> // should replace them with ErrorExpr.
> if (E == nullptr || !E->getType() || E->getType()->is<ErrorType>()) {
> sawError = true;
> return;
> }
>
> And, if I dump the AST for the test case, it has this section with all sorts
> of nulls:
>
> (#if_stmt
> (#if:
> (unresolved_decl_ref_expr type='<null>' name=NOT_ENABLED
> specialized=no)
> (elements
> (sequence_expr type='<null>'
> (discard_assignment_expr type='<null>')
> (assign_expr
> (**NULL EXPRESSION**)
> (**NULL EXPRESSION**))
> (declref_expr type='<null>' decl=main.(file).func
> decl.qqq@/Users/bungi/Desktop/SR-580.swift:2:7 specialized=yes))))
>
> Presumably VarDeclUsageChecker::handleIfConfig() could pass the right subset
> of RK_Read|RK_Written if the declref_expr had its accessType set, or maybe a
> more complete AST that could be walked.
>
> So, then I started looking into places that look for IfConfig{Stmt,Decl} and
> bail, and found ASTWalker.cpp’s Traversal::visitIfConfigStmt(), and
> TypeCheckStmt.cpp’s visitIfConfigStmt(). I hacked in a walk() call in the
> Traversal case and broke all sorts of stuff (maybe since I didn’t do
> something similar in TypeCheckStmt.cpp). That was about the point that I
> thought I’d ask if I’m digging in the wrong spot =)
>
> Presumably making more work get done for inactive #if branches will slow down
> the compiler some (hopefully the percentage of existing code inside a #if
> block at all is pretty low). But, worse, I can imaging there is a ton of
> other stuff in them that will cause errors if more of the compiler runs on
> them (whole missing types, selectors, …).
>
> Is this direction even worth pursuing? Maybe I should look into whether there
> is a quicker way to get the access type of the declref_expr set when it is
> being created during the inactive #if parsing?
>
> Thanks!
>
> -tim
>
> _______________________________________________
> swift-dev mailing list
> swift-dev@swift.org
> https://lists.swift.org/mailman/listinfo/swift-dev
_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev
