Propagate purity information for member access to foreign pure variables

[StrongerXi]

Fixes #12927.

[StrongerXi]

Btw, where does the "constant variable evaluation & substitution" step happen? I wonder if this would affect it, since we are allowing member access in rhs of constant variables now.

[StrongerXi]

Looking deeper, we were handling this correctly for file-level constant because of this line. Member access is similar to identifier, so this should give more confidence to this PR.

[cameel]

Btw, where does the "constant variable evaluation & substitution" step happen?

See ConstantEvaluator.h and the places where it's used.

[StrongerXi]

Btw, where does the "constant variable evaluation & substitution" step happen?

See ConstantEvaluator.h and the places where it's used.

Hmm I did check all the usages but none of them is doing what I was looking for.

Then I dumped the IR and things make sense now. At least at the IR level, it seems that each constant (or all?) variable is turned into a function, and the initialization expression is linearized into IR statements in the function body. I assume constant propagation will clean things up further?

Regardless, this PR shouldn't affect things there.

[StrongerXi]

Rebase.

[cameel]

This looks great. Tests need to be a bit stronger (see my suggestions) but other than that it's a nice enhancement that users will probably appreciate!

Also, please add a changelog entry.

[chriseth]

Please also add some tests where the constants are used as array lengths of statically-sized arrays, both as forward and as backward references.

[StrongerXi]

Please also add some tests where the constants are used as array lengths of statically-sized arrays, both as forward and as backward references.

I think this is more involved than expected. The logic chain is as follows:

• To use a constant variable as array lengths, DeclarationTypeChecker::endVisit(ArrayTypeName) requires the variable to either (1) already be a rational number, e.g., X = 42, or (2) can be evaluated to a rational number by ConstantEvaluator.
• To use X = L.INT as array length, we must go with (2).
• ConstantEvaluator currently doesn't evaluate MemberAccess. But to support it, we must check for circular dependencies in constant declarations first, which is currently done in PostTypeChecker.

I think this reveals a larger issue than this PR is concerned -- We probably want to augment ConstantEvaluator to evaluate more things like member access, but that eventually entails moving constant circular dependence check to earlier places in the compilation stack.

Currently the compilation stack is a bit awkward for constant evaluation, i.e.,

...
DeclarationTypeChecker (use of constant variable as array size is checked)
...
TypeChecker (purity information is propagated, e.g., this PR)
...
PostTypeChecker (constant circular dependence is checked)
...

I think the logics in parenthesis should be reversed...?

[StrongerXi]

Rebase.

[chriseth]

Sorry, I should have clarified: Please add tests so that we see that it does not catastrophically fail. I did not mean to implement this feature for the constant evaluator.

And you are right, the constant evaluator is not at the right place, we have a larger issue planned to extend it.

[StrongerXi]

Sorry, I should have clarified: Please add tests so that we see that it does not catastrophically fail. I did not mean to implement this feature for the constant evaluator.

And you are right, the constant evaluator is not at the right place, we have a larger issue planned to extend it.

Got it. Done.

[cameel]

One last thing.

It's minor enough that I'm approving this already but I really think these tests need to be separate.

[cameel]

It's good. I'd merge this already but we have just released 0.8.15 so the changelog entry needs to be moved.

[ekpyron]

Approving, partly based on @cameel's earlier approval.