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

Numeric literals are not handled properly #35

Open
svick opened this issue Nov 27, 2017 · 6 comments
Open

Numeric literals are not handled properly #35

svick opened this issue Nov 27, 2017 · 6 comments

Comments

@svick
Copy link
Contributor

svick commented Nov 27, 2017

RoslynQuoter doesn't generate the correct code for numeric literals. For example, consider this expression:

new object[] { 42, 42.0, 42f, 42m, 42.0m, 42.00m }

Those six values all have different syntax and are also different values. But the code generated by RQ is:

ArrayCreationExpression(
        ArrayType(PredefinedType(Token(SyntaxKind.ObjectKeyword))).WithRankSpecifiers(
            SingletonList<ArrayRankSpecifierSyntax>(
                ArrayRankSpecifier(
                    SingletonSeparatedList<ExpressionSyntax>(OmittedArraySizeExpression())))))
    .WithInitializer(
        InitializerExpression(
            SyntaxKind.ArrayInitializerExpression,
            SeparatedList<ExpressionSyntax>(
                new SyntaxNodeOrToken[]
                {
                    LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(42)),
                    Token(SyntaxKind.CommaToken),
                    LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(42)),
                    Token(SyntaxKind.CommaToken),
                    LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(42)),
                    Token(SyntaxKind.CommaToken),
                    LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(42)),
                    Token(SyntaxKind.CommaToken),
                    LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(42.0)),
                    Token(SyntaxKind.CommaToken),
                    LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(42.00))
                }))).NormalizeWhitespace()

Evaluating this results in:

new object[]{42, 42, 42, 42, 42, 42}

I think that RQ should generate code that at least preserves semantics. (Ideally, it should preserve the exact syntax used, but I think that's less important.)

@svick
Copy link
Contributor Author

svick commented Nov 27, 2017

Somewhat related issue: dotnet/roslyn#23403.

@KirillOsenkov
Copy link
Owner

To solve this, we need to look at the actual syntax trees for these cases and see how the trees are different.

Then make sure that we emit those differences as API calls. There may be another hole where it's impossible to construct the syntax trees using the public API and only the parser is able to construct such trees internally, because it bypasses the public API and uses the internal APIs directly.

We've had such cases in the past a few times.

@svick
Copy link
Contributor Author

svick commented Nov 27, 2017

I suspect that using the SyntaxFactory.Literal(string text, SomeType value) overloads is going to work.

@KirillOsenkov
Copy link
Owner

Aha, so we probably just need to hint it to choose the right overload...

@KirillOsenkov
Copy link
Owner

Could you please check if this is still a problem?

@svick
Copy link
Contributor Author

svick commented May 30, 2021

@KirillOsenkov It's much better, but still not fully fixed. The generated code now is:

ArrayCreationExpression(
    ArrayType(
        PredefinedType(
            Token(SyntaxKind.ObjectKeyword)))
    .WithRankSpecifiers(
        SingletonList<ArrayRankSpecifierSyntax>(
            ArrayRankSpecifier(
                SingletonSeparatedList<ExpressionSyntax>(
                    OmittedArraySizeExpression())))))
.WithInitializer(
    InitializerExpression(
        SyntaxKind.ArrayInitializerExpression,
        SeparatedList<ExpressionSyntax>(
            new SyntaxNodeOrToken[]{
                LiteralExpression(
                    SyntaxKind.NumericLiteralExpression,
                    Literal(42)),
                Token(SyntaxKind.CommaToken),
                LiteralExpression(
                    SyntaxKind.NumericLiteralExpression,
                    Literal(42.0)),
                Token(SyntaxKind.CommaToken),
                LiteralExpression(
                    SyntaxKind.NumericLiteralExpression,
                    Literal(42f)),
                Token(SyntaxKind.CommaToken),
                LiteralExpression(
                    SyntaxKind.NumericLiteralExpression,
                    Literal(42m)),
                Token(SyntaxKind.CommaToken),
                LiteralExpression(
                    SyntaxKind.NumericLiteralExpression,
                    Literal(42.0m)),
                Token(SyntaxKind.CommaToken),
                LiteralExpression(
                    SyntaxKind.NumericLiteralExpression,
                    Literal(42.00m))})))
.NormalizeWhitespace()

This produces:

new object[]{42, 42, 42F, 42M, 42.0M, 42.00M}

So the only remaining problematic case is double, which should be fixed if dotnet/roslyn#23403 is fixed. I guess this means you can now either close this issue as a duplicate of the Roslyn issue, or keep it open if you think workaround in RQ would be worth it.

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

No branches or pull requests

2 participants