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.

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

[Coverage] Consolidate visitation logic for functions and nominal types #16012

Merged
merged 1 commit into from
Apr 18, 2018

Conversation

vedantk
Copy link
Member

@vedantk vedantk commented Apr 18, 2018

This should address an ASAN failure which arose due to the PGOMapping
ASTWalker not being updated in sync with the other profiling-related
walkers.

rdar://39534066

This should address an ASAN failure which arose due to the PGOMapping
ASTWalker not being updated in sync with the other profiling-related
walkers.

rdar://39534066
@vedantk
Copy link
Member Author

vedantk commented Apr 18, 2018

@swift-ci please smoke test and merge

} else if (auto *TLCD = dyn_cast<TopLevelCodeDecl>(D)) {
assignCounter(TLCD->getBody());
ImplicitTopLevelBody = TLCD->getBody();
} else if (auto *NTD = dyn_cast<NominalTypeDecl>(D)) {
bool continueWalk = Parent.isNull();
if (continueWalk) {
return visitNominalTypeDecl(*this, NTD, [&] {
ParentNominalType = NTD;
Copy link
Member

Choose a reason for hiding this comment

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

What happens when you have nested types?

Copy link
Member Author

Choose a reason for hiding this comment

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

When a constructor for a type is emitted in SILGen, we lazily create a SILProfiler instance for the nominal type associated with the constructor. The same holds for nested types.

Copy link
Member Author

Choose a reason for hiding this comment

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

Er, that could've been clearer, sorry. We'd create a new ASTWalker for the nested type, where the nested nominal-type-decl would be the root of the walk.

@vedantk
Copy link
Member Author

vedantk commented Apr 18, 2018

@shahmishal I'm seeing a strange failure on Linux:

https://ci.swift.org/job/swift-PR-Linux-smoke-test/6339
[xUnit] [ERROR] - Test reports were found but not all of them are new. Did all the tests run?

  • /home/buildnode/jenkins/workspace/swift-PR-Linux-smoke-test@2/branch-master/buildbot_linux/swift-linux-x86_64/swift-test-results/x86_64-unknown-linux-gnu/lit-tests.xml is 1 mo 26 days old

Any idea what this is about?

@vedantk
Copy link
Member Author

vedantk commented Apr 18, 2018

@swift-ci Please clean smoke test Linux platform

@shahmishal
Copy link
Member

real failure:

12:09:40 CMake Error at CMakeLists.txt:103 (message):
12:09:40   LLDB test compilers not specified.  Tests will not run
12:09:40 
12:09:40 
12:09:40 -- Configuring incomplete, errors occurred!

@vedantk
Copy link
Member Author

vedantk commented Apr 18, 2018

@shahmishal Thanks, I'm tracking a fix in rdar://39537655. I think I've gotten to the bottom of it..

@vedantk
Copy link
Member Author

vedantk commented Apr 18, 2018

@shahmishal Heh, today's just not my day. The Linux job failed with a Jenkins exception this time around:

https://ci.swift.org/job/swift-PR-Linux-smoke-test/6344
15:38:11 [177/334] CompileC: CoreFoundation/String.subproj/CFCharacterSet.c
15:45:44 FATAL: command execution failed
15:45:44
java.io.EOFException
15:45:44 at java.io.ObjectInputStream$PeekInputStream.readFully(ObjectInputStream.java:2624)

I've got a fix for the lldb cmake issue, though: apple/swift-lldb#541

@vedantk
Copy link
Member Author

vedantk commented Apr 18, 2018

@swift-ci Please clean smoke test Linux platform

@vedantk vedantk merged commit 80a707d into apple:master Apr 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants