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

Remove unnecessary pure annotations #40097

Closed
wants to merge 1 commit into from
Closed

Remove unnecessary pure annotations #40097

wants to merge 1 commit into from

Conversation

Keno
Copy link
Member

@Keno Keno commented Mar 18, 2021

These should now be unnecessary. Our effect modeling of Core.sizeof
has improved sufficiently, that it should be able to know right away
that these functions are pure without the additional annotation.

These should now be unnecessary. Our effect modeling of Core.sizeof
has improved sufficiently, that it should be able to know right away
that these functions are pure without the additional annotation.
Copy link
Member

@Sacha0 Sacha0 left a comment

Choose a reason for hiding this comment

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

Nifty! :)

@Keno Keno requested a review from JeffBezanson March 18, 2021 19:39
@Keno
Copy link
Member Author

Keno commented Mar 18, 2021

These were added by @JeffBezanson in 0b80774, which was #32437 where I now realize I already said that this shouldn't be @pure. At least I'm consistent :)

@KristofferC
Copy link
Member

Nice to get rid of the last correct usages of @pure (according to the docs).

@vtjnash
Copy link
Member

vtjnash commented Mar 18, 2021

We explicitly block constant-prop of String, which I think is what made this necessary

@Keno
Copy link
Member Author

Keno commented Mar 18, 2021

Ok, yes, I see (after this PR):

julia> function bar()
       return Base.sizeof("foo")
       end
bar (generic function with 1 method)

julia> bar()
3

julia> @code_typed bar()
CodeInfo(
1 ─ %1 = Core.sizeof::typeof(Core.sizeof)
│   %2 = (%1)("foo")::Int64
└──      return %2
) => Int64

Fortunately, I'm about to fix that too, so we'll just have to hold this PR for a little bit.

@Keno Keno marked this pull request as draft March 18, 2021 20:29
@Keno
Copy link
Member Author

Keno commented Feb 3, 2022

about to fix that too

The "about to fix that" is #43852, so this can go in once that's merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants