Parameter attributes on Cfunction closures sticks #41827
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vchuravy
reviewed
Aug 8, 2021
troels
force-pushed
the
shift-parameter-attributes-when-adding-more
branch
2 times, most recently
from
cae5325
to
4cd54af
Compare
vtjnash
approved these changes
Aug 9, 2021
|
Welcome, and thanks! Great to have this fixed. |
troels
force-pushed
the
shift-parameter-attributes-when-adding-more
branch
from
4cd54af
to
983d71c
Compare
|
Thanks! I did a check of the LLVM code of AttributeList and discovered I made a miss in the number of AttributeSets preceding the parameter attributes. I had mistakenly believed that only return value (Index 0) precedes parameters, but apparently function attributes do too (they are at index UNSIGNED_INT_MAX, just preceding 0). I have done a new push with the correct code, keeping both function attributes and return value attributes in their proper place. |
troels
force-pushed
the
shift-parameter-attributes-when-adding-more
branch
from
983d71c
to
71d2916
Compare
vtjnash
reviewed
Aug 24, 2021
vtjnash
reviewed
Aug 25, 2021
troels
force-pushed
the
shift-parameter-attributes-when-adding-more
branch
from
70b600b
to
2ef9b36
Compare
When CFunction closures are created an extra argument is added to the function signature for holding the closure. Make sure that the parameter attributes on already existing parameters are not shifted when adding that parameter.
troels
force-pushed
the
shift-parameter-attributes-when-adding-more
branch
from
2ef9b36
to
d837255
Compare
KristofferC
added
backport 1.6
troels
force-pushed
the
shift-parameter-attributes-when-adding-more
branch
from
d837255
to
4adce69
Compare
This was referenced Aug 25, 2021
troels
force-pushed
the
shift-parameter-attributes-when-adding-more
branch
2 times, most recently
from
0d6ed7f
to
6c6c948
Compare
vtjnash
approved these changes
Aug 25, 2021
vtjnash
reviewed
Aug 25, 2021
|
Looks like this constructor requires dropping !hasAttributes ones (or use the get constructor with separate Fn, Ret, and Params attributes, which permits empty attributes) |
troels
force-pushed
the
shift-parameter-attributes-when-adding-more
branch
from
6c6c948
to
6cb4806
Compare
troels
force-pushed
the
shift-parameter-attributes-when-adding-more
branch
from
6cb4806
to
ffb0e29
Compare
KristofferC
pushed a commit
that referenced
this pull request
Aug 27, 2021
When CFunction closures are created an extra argument is added to the function signature for holding the closure. Make sure that the parameter attributes on already existing parameters are not shifted when adding that parameter. (cherry picked from commit 08f3422)
KristofferC
pushed a commit
that referenced
this pull request
Aug 27, 2021
When CFunction closures are created an extra argument is added to the function signature for holding the closure. Make sure that the parameter attributes on already existing parameters are not shifted when adding that parameter. (cherry picked from commit 08f3422)
KristofferC
pushed a commit
that referenced
this pull request
Aug 31, 2021
When CFunction closures are created an extra argument is added to the function signature for holding the closure. Make sure that the parameter attributes on already existing parameters are not shifted when adding that parameter. (cherry picked from commit 08f3422)
KristofferC
pushed a commit
that referenced
this pull request
Sep 3, 2021
When CFunction closures are created an extra argument is added to the function signature for holding the closure. Make sure that the parameter attributes on already existing parameters are not shifted when adding that parameter. (cherry picked from commit 08f3422)
DilumAluthge
removed
the
merge me
KristofferC
removed
backport 1.6
LilithHafner
pushed a commit
to LilithHafner/julia
that referenced
this pull request
Feb 22, 2022
…#41827) When CFunction closures are created an extra argument is added to the function signature for holding the closure. Make sure that the parameter attributes on already existing parameters are not shifted when adding that parameter.
LilithHafner
pushed a commit
to LilithHafner/julia
that referenced
this pull request
Mar 8, 2022
…#41827) When CFunction closures are created an extra argument is added to the function signature for holding the closure. Make sure that the parameter attributes on already existing parameters are not shifted when adding that parameter.
Closed
staticfloat
pushed a commit
that referenced
this pull request
Dec 23, 2022
When CFunction closures are created an extra argument is added to the function signature for holding the closure. Make sure that the parameter attributes on already existing parameters are not shifted when adding that parameter. (cherry picked from commit 08f3422)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When CFunction closures are created an extra parameter is added to the
function signature for holding the closure.
Make sure that the parameter attributes on already existing parameters
are not shifted when adding that parameter.
Fixes #40164