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

Test return typeassert on inner constructor is respected #47227

Merged
merged 1 commit into from
Oct 22, 2022

Conversation

jakobnissen
Copy link
Member

@jakobnissen jakobnissen commented Oct 19, 2022

Test that #47209 remains fixed.

@vtjnash
Copy link
Member

vtjnash commented Oct 19, 2022

please make titles and commit messages describe the fix itself; the #s should only go into the body for cross-referencing and auto-closing

@jakobnissen jakobnissen changed the title Add test to make sure #47209 stays fixed Test return typeassert on inner constructor is respected Oct 20, 2022
@jakobnissen
Copy link
Member Author

Alright, done.

@KristofferC KristofferC added test This change adds or pertains to unit tests merge me PR is reviewed. Merge when all tests are passing labels Oct 20, 2022
On Julia 1.8, if the internal constructor's return value was typeasserted,
this assertion could be violated without an error. For example, in this case:

```julia
struct Foo
    x::Int
    Foo()::Nothing = new(1)
end
```

Running `Foo()` would not throw. This was inadvertently fixed in a later PR,
but not tested.
This PR adds a test case similar to the example above.
@jakobnissen
Copy link
Member Author

CI errors appear unrelated

@vtjnash vtjnash merged commit fa9c137 into JuliaLang:master Oct 22, 2022
@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Oct 24, 2022
@jakobnissen jakobnissen deleted the test47209 branch October 31, 2022 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test This change adds or pertains to unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants