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

is expression evaluating const expression should be considered constant #6926

Open
333fred opened this issue Jan 28, 2023 Discussed in #6925 · 5 comments
Open

is expression evaluating const expression should be considered constant #6926

333fred opened this issue Jan 28, 2023 Discussed in #6925 · 5 comments
Assignees
Labels
Needs Approved Specification This issue needs an LDM-approved specification Proposal champion Proposal
Milestone

Comments

@333fred
Copy link
Member

333fred commented Jan 28, 2023

Discussed in #6925

Originally posted by Rekkonnect January 28, 2023

Summary

Despite being provided a constant expression, using an is pattern is not considered constant. This can be safely adjusted. As an added bonus, a warning is generated by code flow analysis saying that the expression always evaluates to true or false.

Motivation

Conditional compilation symbols, reliance on runtime constants deriving from conditional statements

Description

You cannot assign an is expression on constant LHS value to a constant field. For example,

public const int A = 4;
public const bool B = A is 4; // illegal, despite the result being available for constant evaluation
public const bool C = A is not 3 and < 1 or 5; // illegal too

In the example above, A is 4 is A == 4, but is not treated as such. A == 4 passes, A is 4 does not. My personal preference is using is wherever available, especially on enum constants, which also allows for extensibility of the pattern.

Since patterns on the RHS of the pattern matching expression are always constant (as of now), the only limitation is the LHS also being a constant expression.

Example

Consider the following repro for determining where to publish a NuGet package.

Design meetings

@Unknown6656
Copy link
Contributor

If code determinism (aka. "constexpr") is implemented, we would get this for free.

list of possible relevant discussions/issues:

@Xyncgas
Copy link

Xyncgas commented Jun 12, 2023

But C++ is a compiled ahead language, C# goes through the JIT which should do const folding on public const bool C = A is not 3 and < 1 or 5;

Should trust JIT compiler on client machine to convert everything undetermined to determined, because it can generate cache and analyse hot path for code

@Rekkonnect
Copy link
Contributor

Rekkonnect commented Jun 13, 2023

A is not 3 and < 1 or 5 is an expression that can be evaluated during compile time, since A is constant and the RHS are all constant expressions, thus making it eligible for a constant expression. Roslyn has some form of constexpr evaluation because of the way constant expressions are evaluated and assigned to the constant fields and locals, including string literals and arithmetic expressions.

The above comment mentioning determinism does not actually interfere with this proposal, aside from being able to "get it for free" if something similar to constexpr is brought into C#.

Constants should never be evaluated during runtime in the JIT, they are fixed values that the program is shipped with, without preserving the expression leading to the result. That's the intent of a constant.

@333fred 333fred added this to the Any Time milestone Oct 9, 2023
@333fred 333fred added the Needs Approved Specification This issue needs an LDM-approved specification label Oct 9, 2023
@333fred
Copy link
Member Author

333fred commented Oct 9, 2023

This was discussed in LDM on https://github.com/dotnet/csharplang/blob/main/meetings/2023/LDM-2023-10-09.md#is-expression-evaluating-const-expression-should-be-considered-constant. It was approved for any time, meaning that it is open for community contribution. The first thing this needs is an approved specification: someone will need to go through the constant expressions part of the specification (https://github.com/dotnet/csharpstandard/blob/draft-v8/standard/expressions.md#1223-constant-expressions) and specify how exactly each type of pattern will work in a constant context (if it will even work at all). Once this is done, the LDM can look at the proposed specification and potentially approve it.

@Rekkonnect
Copy link
Contributor

I'd like to take up on that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Approved Specification This issue needs an LDM-approved specification Proposal champion Proposal
Projects
None yet
Development

No branches or pull requests

5 participants
@333fred @Rekkonnect @Unknown6656 @Xyncgas and others