Skip to content

Add test case for rethrows functions with defaulted throwable argument functions #36455

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 1 commit into from
Mar 18, 2021

Conversation

kthchew
Copy link
Contributor

@kthchew kthchew commented Mar 16, 2021

Two test cases were added, both of which with an expected error "call can throw but is not marked with 'try'" and expected note "call is to 'rethrows' function, but a defaulted argument function can throw". These help someone to understand when the note should be emitted.

The first test case relates to SR-1534. Ideally it has no expected errors or notes, but because of that bug, it currently emits one. The second test case should always have the error and note regardless of whether SR-1534 is fixed.

Resolves SR-14270.

Copy link
Contributor

@LucianoPAlmeida LucianoPAlmeida left a comment

Choose a reason for hiding this comment

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

Left a couple of comments that maybe make sense, but let me know WDYT @varungandhi-apple :)

Copy link
Contributor

@varungandhi-apple varungandhi-apple left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the patch. One minor thing: I think we generally try to have commit messages wrap to 80 columns (similar to code).

@varungandhi-apple
Copy link
Contributor

@swift-ci smoke test

@kthchew
Copy link
Contributor Author

kthchew commented Mar 17, 2021

LGTM. Thanks for the patch. One minor thing: I think we generally try to have commit messages wrap to 80 columns (similar to code).

Alright! I'll fix that, thanks.

The first test case added ideally shouldn't have any expected error or diagnostic.
However, due to SR-1534, there is an error emitted here. Thus, an expected error is
used here to show the compiler's behavior today.

If SR-1534 is fixed in the future, the first test case should no longer have any
expected error nor an expected note. However, the second test case should still be
left alone.

Addresses SR-14270
@varungandhi-apple
Copy link
Contributor

Oh, I wasn't expecting you'd fix it, since it wasn't a big deal, but thanks for doing that. 😄

@swift-ci smoke test

@LucianoPAlmeida
Copy link
Contributor

@swift-ci Please smoke test Linux Platform

@LucianoPAlmeida
Copy link
Contributor

@swift-ci Please test Windows Platform

@varungandhi-apple varungandhi-apple merged commit 26831c6 into swiftlang:main Mar 18, 2021
@varungandhi-apple
Copy link
Contributor

Merged! :shipit:

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