Skip to content

likely/unlikely intrinsics don't need unsafe #45445

Closed
@gnzlbg

Description

@gnzlbg
Contributor

Minor paper-cut, but these intrinsics are safe.

Activity

gnzlbg

gnzlbg commented on Oct 22, 2017

@gnzlbg
ContributorAuthor

Relates to #45440. If we can get them to propagate properly through inline(always) functions, we would be able to write a "safe" wrapper around them that we can stabilize.

hanna-kruppe

hanna-kruppe commented on Oct 22, 2017

@hanna-kruppe
Contributor

These are far from the only intrinsics that are safe, but there's currently no way to make intrinsics safe (short of a safe wrapper). I think I saw an issue about this at some point but I can't find it now.

est31

est31 commented on Oct 22, 2017

@est31
Member

For safe wrappers for likely/unlikely intrinsics, one can also do macros with allow_internal_unsafe: see it live:

#[allow_internal_unsafe]
macro_rules! unlikely {
  ($e:expr) => {
    unsafe {
      unlikely($e)
    }
  };
}

You can pair it with allow_internal_unstable to expose stable unlikely/likely macros!

added
C-feature-requestCategory: A feature request, i.e: not implemented / a PR.
T-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.
on Oct 22, 2017
jbayardo

jbayardo commented on Oct 23, 2017

@jbayardo

The snippet by @est31 should be:

#[allow_internal_unsafe]
macro_rules! unlikely {
  ($e:expr) => {
    #[allow(unused_unsafe)]
    unsafe {
      std::intrinsics::unlikely($e)
    }
  };
}

And requires

#![feature(core_intrinsics)]
#![feature(allow_internal_unsafe)]
#![feature(stmt_expr_attributes)]

In order to work seamlessly as of today's nightly (in the case of likely/unlikely used within unsafe blocks).

est31

est31 commented on Oct 23, 2017

@est31
Member

@jbayardo AFAIK you can also attach the #[allow(unused_unsafe)] to the macro declaration, it should work too. The entire macro can theoretically be exposed by the standard library.

jbayardo

jbayardo commented on Oct 23, 2017

@jbayardo

That did not work for me. I believe this shouldn't be a macro though, the likely/unlikely intrinsics should just be safe; the #allow(unused_unsafe) won't warn you in cases like

unsafe {
  ...
  if likely!(unsafe { ... }) {
    ...
  } else {
    ...
  }
}
hanna-kruppe

hanna-kruppe commented on Oct 23, 2017

@hanna-kruppe
Contributor

You can't just mark intrinsics safe, because all intrinsics are extern "rust-intrinsic" functions and extern functions are always unsafe to call. Changing that would require some new language feature, e.g., as proposed by #1248.

dtolnay

dtolnay commented on Nov 14, 2017

@dtolnay
Member

As mentioned, there are many other intrinsics that should not require unsafe. This is going to need an RFC around how to expose intrinsics that are safe to call. One previous RFC on this topic was closed rust-lang/rfcs#1248 (comment) but the lang team plans to follow up by merging the idea of "intrinsic" and "lang item", at which point likely and unlikely would be exposed as safe to call.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-feature-requestCategory: A feature request, i.e: not implemented / a PR.T-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @gnzlbg@jbayardo@TimNN@dtolnay@hanna-kruppe

        Issue actions

          likely/unlikely intrinsics don't need unsafe · Issue #45445 · rust-lang/rust