Add some aliasing warnings to docstrings for mutating functions in Base#50824
Add some aliasing warnings to docstrings for mutating functions in Base#50824LilithHafner merged 17 commits intoJuliaLang:masterfrom
Conversation
|
Should |
|
I would even change should to must. |
Looked for it in the code base but the search didn't yield anything. Perhaps it doesn't exist 🤯 |
done |
|
Is "alias with" a self-explanatory phrase? Not a native speaker, but I would understand this better if it said "share memory with" or something similar. |
I took the formulation from |
|
Replacement done, I think this is ready for review |
|
I agree with adding warnings to the docstrings. However, I'm not sure what to do with julia> x = [1, 2];
julia> map!(x -> 2x, x, x); x
2-element Vector{Int64}:
2
4In fact, there is a unit test for that use case. |
|
I just took a cue from #50814 but it's easy to remove. Should I leave |
|
Good catch. julia> x = ones(Int,4); @views map!(+, x[begin+1:end], x[begin+1:end], x[begin:end-1]); x
4-element Vector{Int64}:
1
2
3
4We need to specify (or non-specify) an evaluation order for Either of these would mean that We should also consider the behavior of |
|
I actually had a PR (#47012) for the
|
Yeah, that might be the safest thing to do for now. |
|
The "...intended to operate without making any allocations" clause seems redundant and buries the key point. How about sticking to |
|
I removed the redundancy and fixed the |
|
Gonna use @jakobnissen's suggestion and run filter(map(String, names(Base))) do i
endswith(i, '!')
endto see if I missed a few spots. |
|
OK just went through the whole list, see the first message. If anyone objects to my selection of "aliasing-vulnerable" mutating functions, let them speak now or forever hold their peace |
|
Can this be backported to 1.10? If 1.10 becomes an LTS release, it might be nice to have these changes in the docs. |
Co-authored-by: Cameron Bieganek <8310743+CameronBieganek@users.noreply.github.com>
Don't ask me, I'm just visiting around these parts 🤣 |
Backported PRs: - [x] #48625 <!-- add replace(io, str, patterns...) --> - [x] #48387 <!-- Improve documentation of sort-related functions --> - [x] #48363 <!-- Revise sort.md and docstrings in sort.jl --> - [x] #48977 <!-- Update SparseArrays.jl stdlib for SuiteSparse 7 --> - [x] #50719 <!-- fix `CyclePadding(::DataType)` --> - [x] #50694 <!-- inference: permit recursive type traits --> - [x] #50860 <!-- Add `Base.get_extension` to docs/API --> - [x] #50594 <!-- Disallow non-index Integer types in isassigned --> - [x] #50802 <!-- Makes IntrusiveLinkedListSynchronized mutable to avoid allocation on poptask --> - [x] #50858 <!-- Add a `threadpool` parameter to `Channel` constructor --> - [x] #50874 <!-- Restrict COFF to a single thread when symbol count is high --> - [x] #50822 <!-- Add default method for setmodifiers! --> - [x] #50730 <!-- Fix integer overflow in `isapprox` --> - [x] #50850 <!-- Remove weird Rational dispatch and add pi functions to list --> - [x] #50809 <!-- Limit type-printing in MethodError --> - [x] #50915 <!-- Add note the `Task` about sticky bit --> - [x] #50929 <!-- when widening tuple types in tmerge, only widen the complex parts --> - [x] #50928 <!-- Bump JuliaSyntax to 0.4.6 --> - [x] #50959 <!-- Update libssh2 patches --> - [x] #50823 <!-- Make ranges more robust with unsigned indexes. --> - [x] #48542 <!-- Add docs on task-specific buffering using multithreading --> - [x] #50912 <!-- Separate foreign threads into a :foreign threadpool --> - [x] #51010 <!-- Add ORIGIN to SuiteSparse rpath on Linux/FreeBSD --> - [x] #50753 <!-- cat: remove unused promote_eltype methods that confuse inference --> - [x] #51027 <!-- Implement realloc accounting correctly --> - [x] #51019 <!-- fix a case of potentially use of undefined variable when handling error in distributed message processing --> - [x] #51039 <!-- Revert "Optimize findall(f, ::AbstractArray{Bool}) (#42202)" --> - [x] #51036 <!-- add missing invoke edge for nospecialize targets --> - [x] #51042 <!-- inference: fix return_type_tfunc modeling of concrete functions --> - [x] #51114 <!-- Workaround upstream FreeBSD issue #272992 --> - [x] #50892 <!-- Add `JL_DLLIMPORT` to `small_typeof` declaration --> - [x] #51154 <!-- broadcast: use recursion rather than ntuple to map over a tuple --> - [x] #51153 <!-- fix debug typo in "add missing invoke edge for nospecialize targets (#51036)" --> - [x] #51222 <!-- Check again if the tty is open inside the IO lock --> - [x] #51236 <!-- Add lock around uv_unref during init --> - [x] #51243 <!-- GMP: Gracefully handle more overflows. --> - [x] #51254 <!-- Ryu: make sure adding zeros does not overwrite trailing dot --> - [x] #51175 <!-- shorten stale_age for cachefile lock --> - [x] #51300 <!-- fix method definition error for bad vararg --> - [x] #51307 <!-- fix force-throw ctrl-C on Windows --> - [x] #51303 <!-- ensure revising structs is safe --> - [x] #51393 - [x] #51403 Need manual backport: - [x] #51009 <!-- fix #50562, regression in `in` of tuple of Symbols --> - [x] #51053 <!-- Bump Statistics stdlib --> - [x] #51013 <!-- fix #50709, type bound error due to free typevar in sparam env --> - [x] #51305 <!-- reduce test time for rounding and floatfuncs --> Contains multiple commits, manual intervention needed: - [ ] #50663 <!-- Fix Expr(:loopinfo) codegen --> - [ ] #51035 <!-- refactor GC scanning code to reflect jl_binding_t are now first class --> - [ ] #51092 <!-- inference: fix bad effects for recursion --> - [x] #51247 <!-- Check if malloc has succeeded before incrementing gc stats --> - [x] #51294 <!-- use LibGit2_jll for LibGit2 library --> Non-merged PRs with backport label: - [ ] #51132 <!-- Handle `AbstractQ` in concatenation --> - [x] #51029 <!-- testdefs: make sure that if a test set changes the active project, they change it back when they're done --> - [ ] #50919 <!-- Code loading: do the "skipping mtime check for stdlib" check regardless of the value of `ispath(f)` --> - [ ] #50824 <!-- Add some aliasing warnings to docstrings for mutating functions --> - [x] #50385 <!-- Precompile pidlocks: add to NEWS and docs --> - [ ] #49805 <!-- Limit TimeType subtraction to AbstractDateTime -->
|
@LilithHafner did this come up in triage? Honestly at this point I'm happy to revert to whatever state of the PR is deemed more mergeable |
|
No. Last Traige we only talked about the |
Yes |
|
|
Triage says merge! |
LilithHafner
left a comment
There was a problem hiding this comment.
Some usages of _DOCS_ALIASING_WARNING need to be qualified for Julia to build.
|
Thanks @gdalle! Sorry I made this process so long. |
|
Don't apologize, thorough reviews are better than no reviews :) |
Backported PRs: - [x] #50932 <!-- types: fix hash values of Vararg --> - [x] #50975 <!-- Use rr-safe `nopl; rdtsc` sequence --> - [x] #50989 <!-- fix incorrect results in `expm1(::Union{Float16, Float32})` --> - [x] #51284 <!-- Avoid infinite loop when doing SIGTRAP in arm64-apple --> - [x] #51332 <!-- Add s4 field to Xoshiro --> - [x] #51397 <!-- call Pkg precompile hook in latest world --> - [x] #51405 <!-- Remove fallback that assigns a module to inlined frames. --> - [x] #51491 <!-- Throw clearer ArgumentError for strip with two string args --> - [x] #51531 <!-- fix `_tryonce_download_from_cache` (busybox.exe download error) --> - [x] #51541 <!-- Fix string index error in tab completion code --> - [x] #51530 <!-- Don't mark nonlocal symbols as hidden --> - [x] #51557 <!-- Fix last startup & shutdown precompiles --> - [x] #51512 <!-- avoid limiting Type{Any} to Type --> - [x] #51595 <!-- reset `maxprobe` on `empty!` --> - [x] #51582 <!-- Aggressive constprop in LinearAlgebra.wrap --> - [x] #51592 <!-- correctly track element pointer in heap snapshot --> - [x] #51326 <!-- complete false & true more generally as vals --> - [x] #51376 <!-- make `hash(::Xoshiro)` compatible with `==` --> - [x] #51557 <!-- Fix last startup & shutdown precompiles --> - [x] #51845 - [x] #51840 - [x] #50663 <!-- Fix Expr(:loopinfo) codegen --> - [x] #51863 <!-- LLVM 15.0.7-9 --> Contains multiple commits, manual intervention needed: - [ ] #51035 <!-- refactor GC scanning code to reflect jl_binding_t are now first class --> - [ ] #51092 <!-- inference: fix bad effects for recursion --> Non-merged PRs with backport label: - [ ] #51479 <!-- prevent code loading from lookin in the versioned environment when building Julia --> - [ ] #51414 <!-- improvements on GC scheduler shutdown --> - [ ] #51366 <!-- Handle infix operators in REPL completion --> - [ ] #50919 <!-- Code loading: do the "skipping mtime check for stdlib" check regardless of the value of `ispath(f)` --> - [ ] #50824 <!-- Add some aliasing warnings to docstrings for mutating functions in Base --> - [ ] #49805 <!-- Limit TimeType subtraction to AbstractDateTime -->
Backported PRs: - [x] #51213 <!-- Wait for other threads to finish compiling before exiting --> - [x] #51520 <!-- Make allocopt respect the GC verifier rules with non usual address spaces --> - [x] #51598 <!-- Use a simple error when reporting sysimg load failures. --> - [x] #51757 <!-- fix parallel peakflop usage --> - [x] #51781 <!-- Don't make pkgimages global editable --> - [x] #51848 <!-- allow finalizers to take any locks and yield during exit --> - [x] #51847 <!-- add missing wait during Timer and AsyncCondition close --> - [x] #50824 <!-- Add some aliasing warnings to docstrings for mutating functions in Base --> - [x] #51885 <!-- remove chmodding the pkgimages --> - [x] #50207 <!-- [devdocs] Improve documentation about building external forks of LLVM --> - [x] #51967 <!-- further fix to the new promoting method for AbstractDateTime subtraction --> - [x] #51980 <!-- macroexpand: handle const/atomic struct fields correctly --> - [x] #51995 <!-- [Artifacts] Pass artifacts dictionary to `ensure_artifact_installed` dispatch --> - [x] #52098 <!-- Fix errors in `sort` docstring --> - [x] #52136 <!-- Bump JuliaSyntax to 0.4.7 --> - [x] #52140 <!-- Make c func `abspath` consistent on Windows. Fix tracking path conversion. --> - [x] #52009 <!-- fix completion that resulted in startpos of 0 for `\\ --> - [x] #52192 <!-- cap the number of GC threads to number of cpu cores --> - [x] #52206 <!-- Make have_fma consistent between interpreter and compiled --> - [x] #52027 <!-- fix Unicode.julia_chartransform for Julia 1.10 --> - [x] #52217 <!-- More helpful error message for empty `cpu_target` in `Base.julia_cmd` --> - [x] #51371 <!-- Memoize `cwstring` when used for env lookup / modification on Windows --> - [x] #52214 <!-- Turn Method Overwritten Error into a PrecompileError -- turning off caching --> - [x] #51895 <!-- Devdocs on fixing precompile hangs, take 2 --> - [x] #51596 <!-- Reland "Don't mark nonlocal symbols as hidden"" --> - [x] #51834 <!-- [REPLCompletions] allow symbol completions within incomplete macrocall expression --> - [x] #52010 <!-- Revert "Support sorting iterators (#46104)" --> - [x] #51430 <!-- add support for async backtraces of Tasks on any thread --> - [x] #51471 <!-- Fix segfault if root task is NULL --> - [x] #52194 <!-- Fix multiversioning issues caused by the parallel llvm work --> - [x] #51035 <!-- refactor GC scanning code to reflect jl_binding_t are now first class --> - [x] #52030 <!-- Bump Statistics --> - [x] #52189 <!-- codegen: ensure i1 bool is widened to i8 before storing --> - [x] #52228 <!-- Widen diagonal var during `Type` unwrapping in `instanceof_tfunc` --> - [x] #52182 <!-- jitlayers: replace sharedbytes intern pool with one that respects alignment --> Contains multiple commits, manual intervention needed: - [ ] #51092 <!-- inference: fix bad effects for recursion --> Non-merged PRs with backport label: - [ ] #52196 <!-- Fix creating custom log level macros --> - [ ] #52170 <!-- fix invalidations related to `ismutable` --> - [ ] #51479 <!-- prevent code loading from lookin in the versioned environment when building Julia -->
Functions like
sum!(B, A)have undefined behavior whenAandBshare memory. We might fix that in the long run, but in the short run, doc warnings are better than nothing.Related issues:
map!#50814See also: https://discourse.julialang.org/t/did-julia-community-do-something-to-improve-its-correctness/102515/
accumulate!all!any!append!(unsure)asyncmap!broadcast!circcopy!circshift!clamp!conj!copy!copyto!count!cumprod!cumsum!delete!deleteat!digits!empty!extrema!fill!filter!findmax!findmin!get!hex2bytes!insert!intersect!invpermute!keepat!kron!map!maximum!merge!(unsure)mergewith!(unsure)minimum!modifyproperty!partialsort!partialsortperm!permute!permutedims!pop!popat!popfirst!prepend!(unsure)prod!push!pushfirst!put!read!readbytes!replace!replaceproperty!resize!reverse!setdiff!setindex!setproperty!sizehint!sort!sortperm!splice!sum!swapproperty!symdiff!take!union!unique!unsafe_copyto!unsafe_store!