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

don't go through llvm for const indexing #25370

Closed
wants to merge 3 commits into from

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented May 13, 2015

closes #3170

@rust-highfive
Copy link
Collaborator

r? @alexcrichton

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

_ => signal!(e, IndexedNonVec),
}
}
ast::ExprVec(ref arr) => const_val::Array(arr.clone()), // FIXME: eval elements?
Copy link
Member

Choose a reason for hiding this comment

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

... I would imagine you do need to eval the elements ...

(... well, maybe not ... i guess you are effectively doing some sort of lazy-evaluation in the constant evaluator, which may be okay. But what happens if no one ever indexes the element in a constant expression?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But what happens if no one ever indexes the element in a constant expression?

Then they get evaluated in https://github.com/oli-obk/rust/blob/const_indexing/src/librustc_trans/trans/consts.rs#L694-717

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated this to simply memorize the ast::NodeId like tuples and structs do. Note: tuples and structs also do lazy evaluation for the field accesses.

@oli-obk
Copy link
Contributor Author

oli-obk commented May 13, 2015

hmmm... getting quite a few test failures. I guess I broke creating references to const arrays and their elements. I'm evaluating this.

    [run-pass] run-pass/check-static-slice.rs
    [run-pass] run-pass/const-autoderef.rs
    [run-pass] run-pass/const-enum-vec-index.rs
    [run-pass] run-pass/const-fields-and-indexing.rs
    [run-pass] run-pass/const-str-ptr.rs
    [run-pass] run-pass/issue-17233.rs
    [run-pass] run-pass/syntax-extension-source-utils.rs

@alexcrichton
Copy link
Member

r? @pnkfelix or @nrc

Oliver Schneider added 2 commits May 13, 2015 18:13
@oli-obk
Copy link
Contributor Author

oli-obk commented May 13, 2015

this isn't as trivial as I thought. I'm closing this until I've got a working version. sorry about that.

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.

Report const index out-of-bound condition earlier
4 participants