Fix const-fn check in const_eval

[ouz-a]

We were unable to capture constness of the f and ended up with ICE, this PR checks constness of the callee. I also don't understand why we are not checking constness via calling call_kind and capturing FnCall.

Fixes #117795

[rustbot]

r? @wesleywiser

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

[fmease]

You could use let-chains here :)

[ouz-a]

made it more readable now

[oli-obk]

We should probably just rip out the trait method call logic here and solely rely on effect checks. I don't like making const checks have more special casing logic, but also won't review in detail until January. So I don't have anything useful to say about the code change

cc @fee1-dead

[fee1-dead]

is_const_fn_raw only checks whether the function is const, and not whether it is const-stable. We'd need a test to check whether this would silently allow calling functions that are not const-stable.

[ouz-a]

how do we do that kind of check ?

[fee1-dead]

I'm not sure if other parts of the const checking code already check for const stability, so the first step would be creating an auxiliary ui test where upstream has a const-unstable function and downstream tries to call that function without the feature enabled (following the snippet for this ICE). If it already passes, then great. If it doesn't, we can change this from is_const_fn_raw to is_const_fn

[fee1-dead]

We should probably just rip out the trait method call logic here and solely rely on effect checks.

Yes, that would be the goal. I did leave a comment up there // FIXME(effects) do we need this? suggesting potential removal.

[fee1-dead]

After looking a little bit into this, it looks like the actual ICE isn't because we aren't doing enough checks here, but probably because of something deeper than that. If I recall correctly, we used to remap the callee DefId from a trait method into a method from the actual concrete implementation, but there were problems with that so we stopped doing that. The ICE is then caused by us not thinking Fn::call as a const function thus triggering the ICE later. We still shouldn't allow people who haven't enabled feature(const_trait_impl) to do calls like these. Maybe it should be matched as a builtin impl source somewhere above?

[fee1-dead]

@rustbot author

[bors]

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

[Dylan-DPC]

@ouz-a any updates on this?

[oskgo]

@ouz-a

Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you are going to continue please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive