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

allow_internal_unstable attr only allowed once per function #77088

Closed
tesuji opened this issue Sep 23, 2020 · 6 comments · Fixed by #77183
Closed

allow_internal_unstable attr only allowed once per function #77088

tesuji opened this issue Sep 23, 2020 · 6 comments · Fixed by #77183
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@tesuji
Copy link
Contributor

tesuji commented Sep 23, 2020

Discussed in zulip: https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Using.20const.20unreachable.20in.20libcore/near/210673352

The problem is that compiler doesn't accept code like this:

    #[allow_internal_unstable(const_fn)]
    #[allow_internal_unstable(const_unreachable_unchecked)]
    pub const fn len(&self) -> usize {
        // SAFETY: this is safe because `&[T]` and `FatPtr<T>` have the same layout.
        // Only `std` can make this guarantee.
        let raw_len = unsafe { crate::ptr::Repr { rust: self }.raw.len };

        match mem::size_of::<T>() {
            0 => {}
            // SAFETY: this branch taken if mathematically `isize::MAX < raw_len * N`.
            // But references must point to one allocation with size at most isize::MAX.
            N if (isize::MAX as usize) / N < raw_len => unsafe { hint::unreachable_unchecked() },
            _ => {}
        }

        raw_len
    }

But it accepts code when merging mutiple allow_internal_unstable annotations into one: #77023

Meta

rustc --version --verbose: Version 1.48.0-nightly (0da5800 2020-09-22)

cc #76807 which modified some const stability checks.

@rustbot modify labels: E-mentor

@tesuji tesuji added the C-bug Category: This is a bug. label Sep 23, 2020
@rustbot
Copy link
Collaborator

rustbot commented Sep 23, 2020

Error: Label P-low can only be set by Rust team members

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot rustbot added A-const-eval Area: Constant evaluation (MIR interpretation) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. labels Sep 23, 2020
@ecstatic-morse
Copy link
Contributor

ecstatic-morse commented Sep 23, 2020

#76807 just refactored a few individual calls to rustc_attr::allow_internal_unstable into a single helper function.

pub fn allow_internal_unstable(tcx: TyCtxt<'tcx>, def_id: DefId, feature_gate: Symbol) -> bool {
let attrs = tcx.get_attrs(def_id);
attr::allow_internal_unstable(&tcx.sess, attrs)
.map_or(false, |mut features| features.any(|name| name == feature_gate))
}

rustc_attr::allow_internal_unstable only looks for the first attribute it seems. Shouldn't be hard to fix.

pub fn allow_internal_unstable<'a>(
sess: &'a Session,
attrs: &[Attribute],
) -> Option<impl Iterator<Item = Symbol> + 'a> {
let attr = sess.find_by_name(attrs, sym::allow_internal_unstable)?;
let list = attr.meta_item_list().or_else(|| {
sess.diagnostic()
.span_err(attr.span, "allow_internal_unstable expects list of feature names");
None
})?;
Some(list.into_iter().filter_map(move |it| {
let name = it.ident().map(|ident| ident.name);
if name.is_none() {
sess.diagnostic()
.span_err(it.span(), "`allow_internal_unstable` expects feature names");
}
name
}))
}

@ecstatic-morse ecstatic-morse added A-attributes Area: Attributes (`#[…]`, `#![…]`) E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. and removed A-const-eval Area: Constant evaluation (MIR interpretation) A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. labels Sep 23, 2020
@rustbot rustbot added the E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. label Sep 25, 2020
@tesuji
Copy link
Contributor Author

tesuji commented Sep 25, 2020

@rustbot assign @bugadani

@rustbot
Copy link
Collaborator

rustbot commented Sep 25, 2020

Error: Parsing assign command in comment failed: ...'n bugadani' | error: user should start with @ at >| ''...

Please let @rust-lang/release know if you're having trouble with this bot.

@bors bors closed this as completed in 12b8d89 Sep 25, 2020
@oli-obk oli-obk reopened this Sep 26, 2020
@oli-obk

This comment has been minimized.

@oli-obk oli-obk closed this as completed Sep 26, 2020
@bugadani
Copy link
Contributor

It doesn't hurt to double check, just takes some time :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants