Skip to content

AST: Don't set source location on cloned parameter lists #31778

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

Conversation

slavapestov
Copy link
Contributor

@slavapestov slavapestov commented May 14, 2020

Note that rdar://problem/63191951 was actually fixed by #31992, and not this change as I expected. However, this change makes sense anyway, and this PR adds the missing test case.

@slavapestov

This comment has been minimized.

@@ -0,0 +1,7 @@
// RUN: %target-swift-frontend -emit-silgen %S/Inputs/partial_apply_debuginfo_other.swift -primary-file %s -module-name partial_apply_debuginfo -g -Xllvm -sil-print-debuginfo | %FileCheck %s
Copy link
Contributor

Choose a reason for hiding this comment

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

The test should ideally be positive. Is there a visible difference in a loc attachment on either a debug_value or an alloc_stack that we could check for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't see one, in fact this test passes without my change, which is why I asked :) I'm not going to merge this in its current state.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you perhaps see a difference with -verbose-sil? This also prints the non-debug-info relevant details of SILLocation.

@slavapestov slavapestov force-pushed the cloned-parameter-list-source-loc branch from bd96d04 to ffd92f9 Compare May 27, 2020 02:59
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test macOS

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test Linux

2 similar comments
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test Linux

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test Linux

@slavapestov
Copy link
Contributor Author

@swift-ci Please clean smoke test Linux

2 similar comments
@slavapestov
Copy link
Contributor Author

@swift-ci Please clean smoke test Linux

@slavapestov
Copy link
Contributor Author

@swift-ci Please clean smoke test Linux

@slavapestov slavapestov merged commit 71b2c50 into swiftlang:master Jun 2, 2020
@compnerd
Copy link
Member

compnerd commented Jun 2, 2020

This seems to have introduced a regression on the windows builds:

https://ci-external.swift.org/job/oss-swift-windows-x86_64-vs2019/1538/changes#detail

Failing Tests (1):
    Swift(windows-x86_64) :: DebugInfo/curry_thunk.swift

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.

3 participants