Skip to content

Conversation

simeonschaub
Copy link
Member

This got a bit messier than I would have liked, since macro expansion
can't simply rename the property without changing the meaning. This
introduces an internal form :renamed which still encodes the original
name.

@simeonschaub simeonschaub added compiler:lowering Syntax lowering (compiler front end, 2nd stage) bugfix This change fixes an existing bug backport 1.8 Change should be backported to release-1.8 backport 1.9 Change should be backported to release-1.9 labels Feb 19, 2023
@simeonschaub simeonschaub added the macros @macros label Feb 19, 2023
@KristofferC KristofferC mentioned this pull request Feb 20, 2023
50 tasks
@KristofferC KristofferC mentioned this pull request Mar 7, 2023
52 tasks
@KristofferC KristofferC mentioned this pull request Apr 9, 2023
26 tasks
@KristofferC
Copy link
Member

Bump. ready to go?

@simeonschaub simeonschaub removed the request for review from JeffBezanson May 4, 2023 11:53
@simeonschaub simeonschaub marked this pull request as draft May 4, 2023 11:53
@KristofferC KristofferC mentioned this pull request May 8, 2023
51 tasks
@KristofferC KristofferC mentioned this pull request Jun 6, 2023
36 tasks
@KristofferC KristofferC mentioned this pull request Jul 11, 2023
35 tasks
KristofferC added a commit that referenced this pull request Aug 18, 2023
Backported PRs:
- [x] #47782 <!-- Generalize Bool parse method to AbstractString -->
- [x] #48634 <!-- Remove unused "deps" mechanism in internal sorting
keywords [NFC] -->
- [x] #49931 <!-- Lock finalizers' lists at exit -->
- [x] #50064 <!-- Fix numbered prompt with input only with comment -->
- [x] #50474 <!-- docs: Fix a `!!! note` which was miscapitalized -->
- [x] #50516 <!-- Fix visibility of assert on GCC12/13 -->
- [x] #50635 <!-- `versioninfo()`: include build info and unofficial
warning -->
- [x] #49915 <!-- Revert "Remove number / vector (#44358)" -->
- [x] #50781 <!-- fix `bit_map!` with aliasing -->
- [x] #50845 <!-- fix #50438, use default pool for at-threads -->
- [x] #49031 <!-- Update inference.md -->
- [x] #50289 <!-- Initialize prev_nold and nold in gc_reset_page -->
- [x] #50559 <!-- Expand kwcall lowering positional default check to
vararg -->
- [x] #49582 <!-- Update HISTORY.md for `DelimitedFiles` -->
- [x] #50341 <!-- invokelatest docs should say not exported before 1.9
-->
- [x] #50525 <!-- only check that values are finite in `generic_lufact`
when `check=true` -->
- [x] #50444 <!-- Optimize getfield lowering to avoid boxing in some
cases -->
- [x] #50523 <!-- Avoid generic call in most cases for getproperty -->
- [x] #50860 <!-- Add `Base.get_extension` to docs/API -->
- [x] #50164 <!-- codegen: handle dead code with unsafe_store of FCA
pointers -->
- [x] #50568 <!-- `Array(::AbstractRange)` should return an `Array` -->
- [x] #50871 <!-- macOS: Don't inspect dead threadtls during exception
handling. -->

Need manual backport:
- [ ] #48542 <!-- Add docs on task-specific buffering using
multithreading -->
- [ ] #50591 <!-- build: fix various makefile bugs -->


Non-merged PRs with backport label:
- [ ] #50842 <!-- Avoid race conditions with recursive rm -->
- [ ] #50823 <!-- Make ranges more robust with unsigned indexes. -->
- [ ] #50663 <!-- Fix Expr(:loopinfo) codegen -->
- [ ] #49716 <!-- Update varinfo() docstring signature -->
- [ ] #49713 <!-- prevent REPL from erroring in numbered mode in some
situations -->
- [ ] #49573 <!-- Implement jl_cpu_pause on PPC64 -->
- [ ] #48726 <!-- fix macro expansion of property destructuring -->
- [ ] #48642 <!-- Use gc alloc instead of alloc typed in lowering -->
- [ ] #48183 <!-- Don't use pkgimage for package if any includes fall in
tracked path for coverage or alloc tracking -->
- [ ] #48050 <!-- improve `--heap-size-hint` arg handling -->
- [ ] #47615 <!-- Allow threadsafe access to buffer of type inference
profiling trees -->
This got a bit messier than I would have liked, since macro expansion
can't simply rename the property without changing the meaning. This
introduces an internal form `:renamed` which still encodes the original
name.
@vtjnash vtjnash force-pushed the sds/prop_destruct_macroexpand branch from 0c6bada to 43fe32b Compare October 2, 2025 15:36
@vtjnash vtjnash closed this Oct 2, 2025
@vtjnash
Copy link
Member

vtjnash commented Oct 2, 2025

I tried rebasing this, but eventually concluded this is probably not the right approach for compatibility. We would probably be better off introducing a new form '(renamed ,sym ,replacement) that we always use to replace symbols, instead of using the naked gensym and trying to handle it later with a special form just for kwargs. But there is also a ton of other bugs in the way function arguments are handled which would be a lot of work to redo correctly (mainly missing a lot of resolve-letlike-assign calls). But this is also going to be completely subsumed by the julia-lowering work, except for users that need to manually call macroexpand, and that is already fixed by the ability disable this pass with legacyscope=false flag or the macroexpand! function.

@simeonschaub
Copy link
Member Author

Thanks for looking into this! I agree that this is more of a band aid, but I do wonder whether it might be fine for now? What's the rough timeline for the JuliaLowering stuff, will it be ready before the 1.13 branch?

@vtjnash
Copy link
Member

vtjnash commented Oct 2, 2025

It is probably 1.14. Im not opposed to merging fixes, but was finding them to be more trouble than they seemed worth

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 1.8 Change should be backported to release-1.8 backport 1.9 Change should be backported to release-1.9 bugfix This change fixes an existing bug compiler:lowering Syntax lowering (compiler front end, 2nd stage) macros @macros

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants