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

make Base.@assume_effects :nothrow mark throwing expressions as unreachable #54427

Open
nsajko opened this issue May 10, 2024 · 4 comments
Open
Labels
performance Must go faster

Comments

@nsajko
Copy link
Contributor

nsajko commented May 10, 2024

This would provide a pure-Julia way to implement an assumed-unreachable statement, and thus fix #52851 without introducing any new features. Also xref #51729. If this were implemented, any statement that's annotated with Base.@assume_effects :nothrow, but statically known to throw would be equivalent to unreachable(). E.g., 7::Union{} would be assumed unreachable.

@nsajko nsajko added the performance Must go faster label May 10, 2024
@mikmoore
Copy link
Contributor

mikmoore commented May 10, 2024

This is a clever way to implement unreachable, but I don't like it. It's too hard to use properly. An unreachable() function or macro seems to be a much safer and more ergonomic implementation.

Base.@assume_effects is annotated at the function level. This makes it too coarse-grained to use responsibly for this purpose. EDIT: it can be applied to blocks in upcoming versions (thanks for the correction below). A :nothrow annotation like this would also seem to risk applying recursively to any functions called (or at least to inlined functions), which makes this dangerous in the first place and a maintenance nightmare if any of those called functions ever change.

I think we shouldn't have this for many of the same reasons we don't have global @fastmath (any more).

@nsajko
Copy link
Contributor Author

nsajko commented May 10, 2024

Not true, since v1.11 @assume_effects may be appllied to a code block: https://docs.julialang.org/en/v1.12-dev/base/base/#Base.@assume_effects. For example the nothrow below should only apply to the 7::Union{} AFAIK:

Base.@assume_effects :nothrow let
    7::Union{}
end

Obviously unreachable() is intrinsically unsafe, however, given that we can apply nothrow to a code block, I think my proposal is as safe as possible?

@mikmoore
Copy link
Contributor

mikmoore commented May 10, 2024

Not true, since v1.11

Thanks for the correction. I vaguely recalled seeing something like that but it's hard to keep up with all the upcoming changes.

Really, you're right that (in at least some senses) @assume_effects should have this power. Otherwise it's not really assuming the stated effects. Although taking that line of thinking further, we should allow any provable violation of an effects flag to emit an unreachable.

That said, as far as I understand (though my understanding is poor), @assume_effects has virtually no impact on the internal operation of the function it's defined on. Its impact is actually on call sites of that function. Given that understanding, this proposal would dramatically expand the scope of @assume_effects's operation. So it seems like the wrong place for this functionality.

I also don't relish the idea of encouraging people to impose blatantly false effects to twist the compiler into emitting unreachable. There should be a more direct option (although it could co-exist with the proposed feature).

@nsajko
Copy link
Contributor Author

nsajko commented May 10, 2024

xref #54436

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

No branches or pull requests

2 participants