Skip to content

[LifetimeChecker] Update the debug scope when changing insertion point. #14069

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.

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

Merged
merged 2 commits into from
Jan 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2436,6 +2436,7 @@ handleConditionalDestroys(SILValue ControlVariableAddr) {
auto &Availability = CDElt.Availability;

B.setInsertionPoint(Release);
B.setCurrentDebugScope(Release->getDebugScope());
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes me wonder if we should make it illegal to set the insertion point without also doing something to the scope. Should we replace setInsertionPoint with setInsertionPointAndDebugScope() ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@vedantk For the "let's improve SILBuilder ergonomics" task ^^

Copy link
Contributor

Choose a reason for hiding this comment

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

@dcci: or should these all be fresh SILBuilderWithScopes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably we should do something like that, let me think and let's chat further.

Copy link
Contributor

Choose a reason for hiding this comment

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

I proposed setInsertionPointAndDebugScope() for llvm's IRBuilder. The main sticking point was on how to update all existing uses of setInsertionPoint(). It's worth having a separate discussion about this for SILGen.


// Value types and root classes don't require any fancy handling.
// Just conditionally destroy each memory element, and for classes,
Expand Down Expand Up @@ -2507,10 +2508,12 @@ handleConditionalDestroys(SILValue ControlVariableAddr) {

// If true, self.init or super.init was called and self was consumed.
B.setInsertionPoint(ConsumedBlock->begin());
B.setCurrentDebugScope(ConsumedBlock->begin()->getDebugScope());
processUninitializedRelease(Release, true, B.getInsertionPoint());

// If false, self is uninitialized and must be freed.
B.setInsertionPoint(DeallocBlock->begin());
B.setCurrentDebugScope(DeallocBlock->begin()->getDebugScope());
destroyMemoryElements(Loc, Availability);
processUninitializedRelease(Release, false, B.getInsertionPoint());

Expand Down Expand Up @@ -2541,11 +2544,13 @@ handleConditionalDestroys(SILValue ControlVariableAddr) {

// If true, self was consumed or is fully initialized.
B.setInsertionPoint(LiveBlock->begin());
B.setCurrentDebugScope(LiveBlock->begin()->getDebugScope());
emitReleaseOfSelfWhenNotConsumed(Loc, Release);
isDeadRelease = false;

// If false, self is uninitialized and must be freed.
B.setInsertionPoint(DeallocBlock->begin());
B.setCurrentDebugScope(DeallocBlock->begin()->getDebugScope());
destroyMemoryElements(Loc, Availability);
processUninitializedRelease(Release, false, B.getInsertionPoint());

Expand Down
39 changes: 39 additions & 0 deletions test/SILOptimizer/di-conditional-destroy-scope.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// RUN: %target-swift-frontend -emit-sil %s -Onone -Xllvm \
// RUN: -sil-print-after=definite-init -Xllvm \
// RUN: -sil-print-only-functions=$S2fs36RecursibleDirectoryContentsGeneratorC4path10fileSystemAcA12AbsolutePathV_AA04FileH0_ptKc33_F8B132991B28208F48606E87DC165393Llfc \
// RUN: -Xllvm -sil-print-debuginfo -o /dev/null 2>&1 | %FileCheck %s

// REQUIRES: objc_interop


// CHECK: %49 = ref_element_addr %48 : $RecursibleDirectoryContentsGenerator, #RecursibleDirectoryContentsGenerator.fileSystem, loc {{.*}}:38:5, scope 2
// CHECK: destroy_addr %49 : $*FileSystem, loc {{.*}}:38:5, scope 2


import Foundation

public struct AbsolutePath {
public init(_ absStr: String) {}
}

public protocol FileSystem: class {
func getDirectoryContents(_ path: AbsolutePath) throws -> [String]
}
public extension FileSystem {
}

public var currentWorkingDirectory: AbsolutePath {
let cwdStr = FileManager.default.currentDirectoryPath
return AbsolutePath(cwdStr)
}
public class RecursibleDirectoryContentsGenerator {
private var current: (path: AbsolutePath, iterator: IndexingIterator<[String]>)
private let fileSystem: FileSystem
fileprivate init(
path: AbsolutePath,
fileSystem: FileSystem
) throws {
self.fileSystem = fileSystem
current = (path, try fileSystem.getDirectoryContents(path).makeIterator())
}
}