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

fix code coverage bug in tail position and else #53354

Merged
merged 1 commit into from
Feb 20, 2024
Merged

Conversation

JeffBezanson
Copy link
Sponsor Member

@JeffBezanson JeffBezanson commented Feb 15, 2024

This was due to lowering keeping the same location info for the inserted return or goto statement, even though the last seen location might not have executed.

Also fixes inliner handling of the sentinel 0 value for code locations.

@JeffBezanson JeffBezanson added the backport 1.10 Change should be backported to the 1.10 release label Feb 15, 2024
@JeffBezanson JeffBezanson changed the title fix code coverage bug in tail position fix code coverage bug in tail position and else Feb 15, 2024
@KristofferC KristofferC added the backport 1.11 Change should be backported to release-1.11 label Feb 16, 2024
@oscardssmith oscardssmith added compiler:lowering Syntax lowering (compiler front end, 2nd stage) kind:bugfix This change fixes an existing bug labels Feb 16, 2024
@JeffBezanson
Copy link
Sponsor Member Author

This seems to still be subtly wrong. Now I'm thinking the right way to fix this might be to put location nodes at the end of if blocks like we do for for and while.

@JeffBezanson
Copy link
Sponsor Member Author

Ok I fixed the fix, now this might be good enough on its own.

Copy link
Sponsor Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

SGTM

This was due to lowering keeping the same location info for the inserted
`return` or `goto` statement, even though the last seen location might not have
executed.
@KristofferC KristofferC mentioned this pull request Feb 20, 2024
19 tasks
@JeffBezanson JeffBezanson merged commit 61fc907 into master Feb 20, 2024
7 checks passed
@JeffBezanson JeffBezanson deleted the jb/tailposcodecov branch February 20, 2024 21:42
JeffBezanson added a commit that referenced this pull request Feb 21, 2024
This was due to lowering keeping the same location info for the inserted
`return` or `goto` statement, even though the last seen location might
not have executed.

Also fixes inliner handling of the sentinel `0` value for code
locations.

(cherry picked from commit 61fc907)
JeffBezanson added a commit that referenced this pull request Feb 22, 2024
This was due to lowering keeping the same location info for the inserted
`return` or `goto` statement, even though the last seen location might
not have executed.

Also fixes inliner handling of the sentinel `0` value for code
locations.

(cherry picked from commit 61fc907)
KristofferC pushed a commit that referenced this pull request Feb 26, 2024
This was due to lowering keeping the same location info for the inserted
`return` or `goto` statement, even though the last seen location might
not have executed.

Also fixes inliner handling of the sentinel `0` value for code
locations.

(cherry picked from commit 61fc907)
@KristofferC KristofferC mentioned this pull request Feb 26, 2024
28 tasks
KristofferC pushed a commit that referenced this pull request Feb 26, 2024
This was due to lowering keeping the same location info for the inserted
`return` or `goto` statement, even though the last seen location might
not have executed.

Also fixes inliner handling of the sentinel `0` value for code
locations.

(cherry picked from commit 61fc907)
KristofferC added a commit that referenced this pull request Feb 27, 2024
Backported PRs:
- [x] #53205 <!-- Profile: add notes to `print()` docs -->
- [x] #53170 <!-- Remove outdated discussion about externally changing
module bindings -->
- [x] #53228 <!-- SubArray: avoid invalid elimination of singleton
indices -->
- [x] #51361 <!-- code_warntype docs: more neutral reference to
@code_warntype -->
- [x] #50480 <!-- Document --heap-size-hint in Command-line Interface
-->
- [x] #53301 <!-- Fix typo in `Sys.total_memory` docstring. -->
- [x] #53354 <!-- fix code coverage bug in tail position and `else` -->
- [x] #53388 <!-- Fix documentation: thread pool of main thread -->
- [x] #53429 <!-- Subtype: skip slow-path in `local_∀_∃_subtype` if
inputs contain no ∃ typevar. -->
- [x] #53437 <!-- Add debug variant of loader_trampolines.o -->

Need manual backport:
- [ ] #52505 <!-- fix alignment of emit_unbox_store copy -->
- [ ] #53373 <!-- fix sysimage-native-code=no option with pkgimages -->
- [ ] #53439 <!-- staticdata: fix assert from partially disabled native
code -->

Contains multiple commits, manual intervention needed:
- [ ] #52913 <!-- Added docstring for Artifacts.jl -->
- [ ] #53218 <!-- Fix interpreter_exec.jl test -->

Non-merged PRs with backport label:
- [ ] #53424 <!-- yet more atomics & cache-line fixes on work-stealing
queue -->
- [ ] #53125 <!-- coverage: count coverage where explicitly requested by
inference only -->
- [ ] #52694 <!-- Reinstate similar for AbstractQ for backward
compatibility -->
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
KristofferC added a commit that referenced this pull request Mar 1, 2024
Backported PRs:
- [x] #53361 <!-- 🤖 [master] Bump the SparseArrays stdlib from c9f7293
to cb602d7 -->
- [x] #53300 <!-- allow external absint to hold custom data in
`codeinst.inferred` -->
- [x] #53342 <!-- Add `Base.wrap` to docs -->
- [x] #53372 <!-- Silence warnings in `test/file.jl` -->
- [x] #53357 <!-- 🤖 [master] Bump the Pkg stdlib from 6dd0e7c9e to
76070d295 -->
- [x] #53373 <!-- fix sysimage-native-code=no option with pkgimages -->
- [x] #53333 <!-- More consistent return value for annotations, take 2
-->
- [x] #53354 <!-- fix code coverage bug in tail position and `else` -->
- [x] #53407 <!-- fix sysimage-native-code=yes option -->
- [x] #53388 <!-- Fix documentation: thread pool of main thread -->
- [x] #53355 <!-- Fix synchronization issues on the GC scheduler. -->
- [x] #53429 <!-- Subtype: skip slow-path in `local_∀_∃_subtype` if
inputs contain no ∃ typevar. -->
- [x] #53437 <!-- Add debug variant of loader_trampolines.o -->
- [x] #53284 <!-- Add annotate! method for AnnotatedIOBuffer -->
- [x] #53466 <!-- [MozillaCACerts_jll] Update to v2023-12-12 -->
- [x] #53467 <!-- [LibGit2_jll] Update to v1.7.2 -->
- [x] #53326 <!-- RFC: when loading code for internal purposes, load
stdlib files directly, bypassing DEPOT_PATH, LOAD_PATH, and stale checks
-->
- [x] #53332
- [x] #53320 <!-- Add `Sys.isreadable, Sys.iswriteable`, update `ispath`
-->
- [x] #53476

Contains multiple commits, manual intervention needed:
- [ ] #53285 <!-- Add update mechanism for Terminfo, and common
user-alias data -->

Non-merged PRs with backport label:
- [ ] #53424 <!-- yet more atomics & cache-line fixes on work-stealing
queue -->
- [ ] #53408 <!-- task splitting: change additive accumulation to
multiplicative -->
- [ ] #53403 <!-- Move parallel precompilation to Base -->
- [ ] #53402 <!-- Add `jl_getaffinity` and `jl_setaffinity` -->
- [ ] #53391 <!-- Default to the medium code model in x86 linux -->
- [ ] #53125 <!-- coverage: count coverage where explicitly requested by
inference only -->
- [ ] #52694 <!-- Reinstate similar for AbstractQ for backward
compatibility -->
@KristofferC KristofferC removed the backport 1.11 Change should be backported to release-1.11 label Mar 1, 2024
tecosaur pushed a commit to tecosaur/julia that referenced this pull request Mar 4, 2024
This was due to lowering keeping the same location info for the inserted
`return` or `goto` statement, even though the last seen location might
not have executed.

Also fixes inliner handling of the sentinel `0` value for code
locations.
mkitti pushed a commit to mkitti/julia that referenced this pull request Mar 7, 2024
This was due to lowering keeping the same location info for the inserted
`return` or `goto` statement, even though the last seen location might
not have executed.

Also fixes inliner handling of the sentinel `0` value for code
locations.
@KristofferC KristofferC removed the backport 1.10 Change should be backported to the 1.10 release label Mar 12, 2024
Drvi pushed a commit to RelationalAI/julia that referenced this pull request Jun 7, 2024
This was due to lowering keeping the same location info for the inserted
`return` or `goto` statement, even though the last seen location might
not have executed.

Also fixes inliner handling of the sentinel `0` value for code
locations.

(cherry picked from commit 61fc907)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:lowering Syntax lowering (compiler front end, 2nd stage) kind:bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants