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

Add lint for large const items #21632

Closed
wants to merge 1 commit into from

Conversation

petrochenkov
Copy link
Contributor

const-ants are defined to be inline rvalues and this definition sometimes leads to quite non-intuitive behavior.
For example, in this code the array ARR is (correctly) copied to the stack 3 times (see the unoptimized IR/asm http://is.gd/UMQz1B):

const ARR: [u8; 100] = [33; 100];

fn main() {
    let _a = ARR[4] + ARR[7] + ARR[47];
}

The lint prevents such mistakes by checking the size of a const-ant and warning if it's larger than MAX_CONST_SIZE.

In this PR the value of MAX_CONST_SIZE is selected to be 64 bytes, so nothing in Rust itself breaks (except for one place in libstd/thread_local), but it can (should?) be smaller.
Obviously, the value should be larger than size of fat reference, because the code like

const MY_STR: &'static str = "Hello!";

shouldn't warn.
Any ideas on what should the value of MAX_CONST_SIZE ideally be?

@rust-highfive
Copy link
Contributor

r? @pcwalton

(rust_highfive has picked a reviewer for you, use r? to override)

@petrochenkov
Copy link
Contributor Author

r? @eddyb
Will the warning still be relevant when #21744 lands?

@rust-highfive rust-highfive assigned eddyb and unassigned pcwalton Jan 29, 2015
@eddyb
Copy link
Member

eddyb commented Jan 29, 2015

I made non-constant evaluation descend into expression that have [T; N] in their interior, at that is the only expression that has a constant evaluation cost that doesn't scale linearly with source size, and it's always more efficient to initialize in-place than copy it from somewhere.
This means const ARR: [u8; 100] = [33; 100]; will behave as a non-const, when ARR is copied somewhere. Only if it's borrowed it will be placed in static memory. We could also optimize that to generate zeroinitializer in the common case of [all_zero_bytes; N] instead of using O(N) memory and time.

@bors
Copy link
Collaborator

bors commented Feb 28, 2015

☔ The latest upstream changes (presumably #22801) made this pull request unmergeable. Please resolve the merge conflicts.

@petrochenkov
Copy link
Contributor Author

I'll gladly close this PR myself if large const items are never duplicated or copied when used
by immutable or by mutable reference, including inter-crate usage.
cc rust-lang/rfcs#885

@steveklabnik
Copy link
Member

So, what's the status on this?

@petrochenkov
Copy link
Contributor Author

@steveklabnik
Needs a decision from someone familiar with the implementation of constants.
rust-lang/rfcs#885 needs a decision too, because it proposes a backward-incompatible change.

@petrochenkov petrochenkov deleted the const_lint branch May 9, 2015 12:01
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.

6 participants