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

C# Runtime check fails on compilation #3759

Closed
DanielMSchmidt opened this issue Sep 21, 2022 · 2 comments · Fixed by #3760
Closed

C# Runtime check fails on compilation #3759

DanielMSchmidt opened this issue Sep 21, 2022 · 2 comments · Fixed by #3760
Assignees
Labels
bug This issue is a bug. module/pacmak Issues affecting the `jsii-pacmak` module p1

Comments

@DanielMSchmidt
Copy link
Contributor

Describe the bug

The CDKTF library does not build with 1.67.0 after upgrading from 1.65.0 in C#. There is an error thrown when running dotnet build --force --configuration Release as part of jsii-pacmak as part of jsii-srcmak.

Expected Behavior

The unchanged code should still build.

Current Behavior

[jsii-pacmak] [WARN] Exception occurred, not cleaning up /var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-packP6yknl
[jsii-pacmak] [WARN] dotnet failed
Error: All attempts failed. Last error: Command (dotnet build --force --configuration Release) failed with status 1:

/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp/Cdktf/ListTerraformIterator.cs(33,26): error CS8120: The switch case is unreachable. It has already been handled by a previous case or it is impossible to match. [/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp.Cdktf.csproj]
/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp/Cdktf/ListTerraformIterator.cs(35,26): error CS8120: The switch case is unreachable. It has already been handled by a previous case or it is impossible to match. [/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp.Cdktf.csproj]
/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp/Cdktf/ListTerraformIterator.cs(37,26): error CS8120: The switch case is unreachable. It has already been handled by a previous case or it is impossible to match. [/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp.Cdktf.csproj]
/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp/Cdktf/ListTerraformIterator.cs(39,26): error CS8120: The switch case is unreachable. It has already been handled by a previous case or it is impossible to match. [/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp.Cdktf.csproj]
/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp/Cdktf/ListTerraformIterator.cs(41,26): error CS8120: The switch case is unreachable. It has already been handled by a previous case or it is impossible to match. [/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp.Cdktf.csproj]
/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp/Cdktf/TerraformIterator.cs(57,26): error CS8120: The switch case is unreachable. It has already been handled by a previous case or it is impossible to match. [/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp.Cdktf.csproj]
/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp/Cdktf/TerraformIterator.cs(59,26): error CS8120: The switch case is unreachable. It has already been handled by a previous case or it is impossible to match. [/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp.Cdktf.csproj]
/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp/Cdktf/TerraformIterator.cs(61,26): error CS8120: The switch case is unreachable. It has already been handled by a previous case or it is impossible to match. [/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp.Cdktf.csproj]
/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp/Cdktf/TerraformIterator.cs(63,26): error CS8120: The switch case is unreachable. It has already been handled by a previous case or it is impossible to match. [/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp.Cdktf.csproj]
/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp/Cdktf/TerraformIterator.cs(65,26): error CS8120: The switch case is unreachable. It has already been handled by a previous case or it is impossible to match. [/private/var/folders/z_/v03l33d55fb57nrr3b1q03ch0000gq/T/npm-pack9gX9b6/HashiCorp.Cdktf/HashiCorp.Cdktf.csproj]

The code in question looks like this in C#:

public class ListTerraformIterator : HashiCorp.Cdktf.TerraformIterator
    {
        /// <remarks>
        /// <strong>Stability</strong>: Experimental
        /// </remarks>
        public ListTerraformIterator(object list): base(_MakeDeputyProps(list))
        {
        }

        [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.AggressiveInlining)]
        private static DeputyProps _MakeDeputyProps(object list)
        {
            if (Amazon.JSII.Runtime.Configuration.RuntimeTypeChecking)
            {
                switch (list)
                {
                    case string[] cast_a33039:
                        break;
                    case HashiCorp.Cdktf.IResolvable cast_a33039:
                        break;
                    case double[] cast_a33039:
                        break;
                    case HashiCorp.Cdktf.ComplexList cast_a33039:
                        break;
                    case HashiCorp.Cdktf.StringMapList cast_a33039:
                        break;
                    case HashiCorp.Cdktf.NumberMapList cast_a33039:
                        break;
                    case HashiCorp.Cdktf.BooleanMapList cast_a33039:
                        break;
                    case HashiCorp.Cdktf.AnyMapList cast_a33039:
                        break;
                    case object[] cast_a33039:
                        for (var __idx_14fecf = 0 ; __idx_14fecf < cast_a33039.Length ; __idx_14fecf++)
                        {
                            switch (cast_a33039[__idx_14fecf])
                            {
                                case bool cast_30b700:
                                    break;
                                case HashiCorp.Cdktf.IResolvable cast_30b700:
                                    break;
                                case Amazon.JSII.Runtime.Deputy.AnonymousObject cast_30b700:
                                    // Not enough information to type-check...
                                    break;
                                case null:
                                    throw new System.ArgumentException($"Expected argument {nameof(list)}[{__idx_14fecf}] to be one of: bool, {typeof(HashiCorp.Cdktf.IResolvable).FullName}; received null", nameof(list));
                                default:
                                    throw new System.ArgumentException($"Expected argument {nameof(list)}[{__idx_14fecf}] to be one of: bool, {typeof(HashiCorp.Cdktf.IResolvable).FullName}; received {cast_a33039[__idx_14fecf].GetType().FullName}", nameof(list));
                            }
                        }
                        break;
                    case Amazon.JSII.Runtime.Deputy.AnonymousObject cast_a33039:
                        // Not enough information to type-check...
                        break;
                    case null:
                        throw new System.ArgumentException($"Expected argument {nameof(list)} to be one of: string[], {typeof(HashiCorp.Cdktf.IResolvable).FullName}, double[], {typeof(HashiCorp.Cdktf.ComplexList).FullName}, {typeof(HashiCorp.Cdktf.StringMapList).FullName}, {typeof(HashiCorp.Cdktf.NumberMapList).FullName}, {typeof(HashiCorp.Cdktf.BooleanMapList).FullName}, {typeof(HashiCorp.Cdktf.AnyMapList).FullName}, object[]; received null", nameof(list));
                    default:
                        throw new System.ArgumentException($"Expected argument {nameof(list)} to be one of: string[], {typeof(HashiCorp.Cdktf.IResolvable).FullName}, double[], {typeof(HashiCorp.Cdktf.ComplexList).FullName}, {typeof(HashiCorp.Cdktf.StringMapList).FullName}, {typeof(HashiCorp.Cdktf.NumberMapList).FullName}, {typeof(HashiCorp.Cdktf.BooleanMapList).FullName}, {typeof(HashiCorp.Cdktf.AnyMapList).FullName}, object[]; received {list.GetType().FullName}", nameof(list));
                }
            }
            return new DeputyProps(new object?[]{list});
        }

The errors are thrown from case HashiCorp.Cdktf.ComplexList cast_a33039: to case HashiCorp.Cdktf.AnyMapList cast_a33039:.

Reproduction Steps

git clone git@github.com:hashicorp/terraform-cdk.git
cd terraform-cdk
git checkout automation/yarn-upgrade
yarn
yarn build
yarn package 

Possible Solution

No response

Additional Information/Context

No response

SDK version used

1.57.0

Environment details (OS name and version, etc.)

OSX 12.6

@DanielMSchmidt DanielMSchmidt added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 21, 2022
@RomainMuller RomainMuller added p1 module/pacmak Issues affecting the `jsii-pacmak` module and removed needs-triage This issue or PR still needs to be triaged. labels Sep 21, 2022
@RomainMuller RomainMuller self-assigned this Sep 21, 2022
@RomainMuller
Copy link
Contributor

Thanks for reporting. I can confirm that, contrary to the unwritten assumption, types in a union aren't necessarily unrelated... and this may cause us to emit unnecessary checks.

Should not be all too hard to either remove checks that are implied by others, or to re-order the clauses so that the implied clauses come first (I lean towards removing redundant clauses but am wondering if there'd be any associated risk/tradeoff)

RomainMuller added a commit that referenced this issue Sep 21, 2022
If a type union includes several candidates that are related to each
other (A extends B or A implements B), `jsii-pacmak` may generate type
checking clauses in a pattern matching `switch` statement in an order
such that the compiler identifies dead clauses, which is an error in C#.

This adds a provision to NOT emit such a clause so as to not cause the
error. It is worth mentioning that the error cannot be "opted out" of
via a `#pragma` directive like a warning would be, which is unfortunate.

Fixes #3759
@mergify mergify bot closed this as completed in #3760 Sep 22, 2022
mergify bot pushed a commit that referenced this issue Sep 22, 2022
If a type union includes several candidates that are related to each other (A extends B or A implements B), `jsii-pacmak` may generate type checking clauses in a pattern matching `switch` statement in an order such that the compiler identifies dead clauses, which is an error in C#.

This adds a provision to NOT emit such a clause so as to not cause the error. It is worth mentioning that the error cannot be "opted out" of via a `#pragma` directive like a warning would be, which is unfortunate.

Fixes #3759



---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. module/pacmak Issues affecting the `jsii-pacmak` module p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants