Use of option_env Macro Causing the RLS to Panic?

[DSpeckhals]

I am attempting to debug the RLS not properly executing when trying to load save analysis, and caught something here that made me question. This is the issue brought up in rust-lang/rls#544.

I am seeing that the error is occurring in rls-analysis. It looks like this code was refactored within the last several weeks in 37508ae.

https://github.com/nrc/rls-analysis/blob/4cb03bacb2ade979855f0103d264d74e75ca532d/src/loader.rs#L115-L131

First, is option_env! giving us unexpected behavior. It says in the standard library documentation that this macro inspects env variables at compile time, when they should probably be inspected at run time here.

If this env variable is being found at compile time, this could pose problematic for RLS binaries that are compiled in the Rust CI. The RUSTSRC env variable may exist there, but not on the end-user's systems. I'm thinking that the previous version of this code would fall into the second or_else, and just work from there. The new version doesn't have the second or_else, which would likely cause that invalid RUSTSRC path to be used, and the command would fail.

[nrc]

First, is option_env! giving us unexpected behavior. It says in the standard library documentation that this macro inspects env variables at compile time, when they should probably be inspected at run time here.

Yeah, this is correct, we should be using std::env::var rather than option_env

[akappel]

Although I concur that the documentation states it's a compile-time inspection and should probably be changed, the actual semantics that were there previously don't seem to have changed. What I mean is before that change, the logic was still using option_env!.

[DSpeckhals]

I've illustrated the problem is this Playground example: https://play.rust-lang.org/?gist=23af1a1c4e3e6d6e9ba7835264a781bb&version=nightly

@akappel I think that the actual semantics did change here. I didn't think so either (it's not obvious with a quick glance). However, I think this Playground snippet reproduces the issue.

[akappel]

@DSpeckhals gotcha! That makes sense. Thanks for the code snippet.