Section of switch statement is excluded from translation

[dbh1997]

C# function

        static bool IsValidRoofProperty(PropID propID)
        {
            bool ok = true;

            switch (propID)
            {
                case PropID.RidgeHeight:
                case PropID.BarrelHeight:
                case PropID.RidgeHeader:
                //case PropID.PeakHeader: not currently used
                case PropID.RidgeLeft:
                case PropID.RidgeRight:
                case PropID.RidgeLength:
                case PropID.HipLength:
                case PropID.SegmCount:
                case PropID.EdgeLength:
                case PropID.PeakHeights:
                case PropID.Radius:
                case PropID.RoofSpan:
                case PropID.SlopeSingle:
                case PropID.Slope:
                case PropID.SlopeLeft:
                case PropID.SlopeRight:
                case PropID.SlopeUpper:
                case PropID.SlopeLower:
                case PropID.SlopesAll:
                case PropID.HipEndSlopeC:
                case PropID.HipEndSlopeD:
                case PropID.RafterLength:
                case PropID.RafterLeft:
                case PropID.RafterRight:
                case PropID.SlopeSame:
                case PropID.RafterLengthUpper:
                case PropID.RafterLengthLower:
                //case PropID.Measure:
                case PropID.EaveOverhang:
                case PropID.RakeOverhang:
                case PropID.RoofType:
                case PropID.EaveHeightSingle:
                case PropID.EaveHeightHeader:
                case PropID.EaveHeightAll:
                case PropID.EaveHeightLeft:
                case PropID.EaveHeightRight:
                case PropID.EaveHeightHipC:
                case PropID.EaveHeightHipD:
                //case PropID.HalfHip:
                case PropID.DutchGableHeight:
                case PropID.HipEnds:
                case PropID.HipWidth:
                case PropID.LowerRidgeDistance:
                case PropID.UpperRidgeHeight:
                case PropID.LowerRidgeHeight:
                case PropID.RoofFramingCenters:
                case PropID.RoofFramingCustomCenters:
                case PropID.RoofFramingTrussPlies:
                case PropID.RoofFramingCommonRafter:
                case PropID.RoofFramingRidgeBeam:
                case PropID.RoofFramingRidgeBeamCount:
                case PropID.RoofFramingHipRafter:
                case PropID.RoofFramingRafterHipCount:
                case PropID.RoofFramingCollarTie:
                case PropID.RoofFramingCollarTiePos:
                case PropID.RoofFramingSubFacia:
                case PropID.RoofFramingType:
                case PropID.WallConstruction:
                case PropID.RoofSheathing:
                case PropID.ShowLabel:
                case PropID.RoofColor:
                case PropID.RoofTexture:
                case PropID.ExtendRoofWalls:
                case PropID.RoofWallHeight:
                    break;
                default:
                    ok = false;
                    break;
            }

            return ok;
        }

Function as shown in ILSPY. It added the outer switch statement, probably these two values were disjointed enough in value from the others to warrant this optimization.

private static bool IsValidRoofProperty(PropID propID)
{
    bool ok = true;
    if (propID != PropID.ShowLabel && propID != PropID.WallConstruction)
    {
        switch (propID)
        {
        case PropID.RoofType:
        case PropID.EaveHeightSingle:
        case PropID.EaveHeightHeader:
        case PropID.EaveHeightAll:
        case PropID.EaveHeightLeft:
        case PropID.EaveHeightRight:
        case PropID.EaveHeightHipC:
        case PropID.EaveHeightHipD:
        case PropID.EaveOverhang:
        case PropID.RakeOverhang:
        case PropID.SlopeSingle:
        case PropID.Slope:
        case PropID.SlopeLeft:
        case PropID.SlopeRight:
        case PropID.SlopesAll:
        case PropID.SlopeUpper:
        case PropID.SlopeLower:
        case PropID.HipEndSlopeC:
        case PropID.HipEndSlopeD:
        case PropID.SlopeSame:
        case PropID.RafterLength:
        case PropID.RafterLeft:
        case PropID.RafterRight:
        case PropID.RafterLengthUpper:
        case PropID.RafterLengthLower:
        case PropID.HipEnds:
        case PropID.HipWidth:
        case PropID.DutchGableHeight:
        case PropID.BarrelHeight:
        case PropID.RidgeHeader:
        case PropID.RidgeHeight:
        case PropID.RidgeLeft:
        case PropID.RidgeRight:
        case PropID.RidgeLength:
        case PropID.HipLength:
        case PropID.SegmCount:
        case PropID.EdgeLength:
        case PropID.Radius:
        case PropID.PeakHeights:
        case PropID.RoofSpan:
        case PropID.UpperRidgeHeight:
        case PropID.LowerRidgeHeight:
        case PropID.LowerRidgeDistance:
        case PropID.RoofFramingCenters:
        case PropID.RoofFramingCommonRafter:
        case PropID.RoofFramingRidgeBeam:
        case PropID.RoofFramingRidgeBeamCount:
        case PropID.RoofFramingHipRafter:
        case PropID.RoofFramingRafterHipCount:
        case PropID.RoofFramingCollarTie:
        case PropID.RoofFramingCollarTiePos:
        case PropID.RoofFramingSubFacia:
        case PropID.RoofFramingType:
        case PropID.RoofSheathing:
        case PropID.RoofColor:
        case PropID.RoofTexture:
        case PropID.ExtendRoofWalls:
        case PropID.RoofWallHeight:
        case PropID.RoofFramingCustomCenters:
        case PropID.RoofFramingTrussPlies:
            return ok;
        case PropID.Measure:
        case PropID.SlopeHeader:
        case PropID.HalfHip:
        case PropID.PeakHeader:
            IL_120:
            ok = false;
            return ok;
        }
        goto IL_120;
    }
    return ok;
}

Translation to JS

function StdRoof2_IsValidRoofProperty (propID) {
    var ok = true;
    if (!((propID === $T21().ShowLabel) || (propID === $T21().WallConstruction))) {
      switch (propID.valueOf()) {
        default: 
        case 182: 
        case 183: 
        case 200: 
        case 205: 
          ok = false;
          break;

      }
    }
    return ok;
  }; 

Most of the cases are excluded. Perhaps the optimizer got a little aggressive and thought it was unnecessary pass through code. If it weren't for the "default" switch statement, then this would absolutely make for a good optimization.

The fix was easy enough. Change the first set of case statement to return true, and return false at the bottom of the function. No reason to bother with temp variable.

Translation after change:

 function StdRoof2_IsValidRoofProperty (propID) {
    if (!((propID === $T21().ShowLabel) || (propID === $T21().WallConstruction))) {
      switch (propID.valueOf()) {
        case 172: 
        case 173: 
        case 174: 
        case 175: 
        case 176: 
        case 177: 
        case 178: 
        case 179: 
        case 180: 
        case 181: 
        case 184: 
        case 185: 
        case 186: 
        case 187: 
        case 188: 
        case 189: 
        case 190: 
        case 191: 
        case 192: 
        case 193: 
        case 194: 
        case 195: 
        case 196: 
        case 197: 
        case 198: 
        case 199: 
        case 201: 
        case 202: 
        case 203: 
        case 204: 
        case 206: 
        case 207: 
        case 208: 
        case 209: 
        case 210: 
        case 211: 
        case 212: 
        case 213: 
        case 214: 
        case 215: 
        case 216: 
        case 217: 
        case 218: 
        case 219: 
        case 220: 
        case 221: 
        case 222: 
        case 223: 
        case 224: 
        case 225: 
        case 226: 
        case 227: 
        case 228: 
        case 229: 
        case 230: 
        case 231: 
        case 232: 
        case 233: 
        case 234: 
        case 235: 
          return true;

      }
      return false;
    }
    return true;

[kg]

That goto inside the switch statement looks suspicious, I'm not sure it would even compile. Not sure what happened to all the cases in the switch statement - I don't run any code to prune cases...

[dbh1997]

Here’s a reproducible C# example that’s pared way back. Not sure it’s the minimum case, but probably small enough for what you need. In any event, you MUST turn the optimize flag on in project settings for this to reproduce, otherwise it works fine.

Should print true, but prints false.

    enum TestSwitchEnum
    {
        A,
        B,
        C,
        D,
        E,
        F,
        G,
        G2,
        H = 350,
        I = 351,
    }

    public static void SwitchDefect_899()
    {
        TestSwitch(TestSwitchEnum.C);
    }

    private static void TestSwitch(TestSwitchEnum e)
    {
        bool ok = true;
        switch (e)
        {
            case TestSwitchEnum.B:
            case TestSwitchEnum.C:
            case TestSwitchEnum.E:
            case TestSwitchEnum.F:
            case TestSwitchEnum.G:
                break;
            default:
                ok = false;
                break;
        }
        Console.WriteLine(ok);
    }

---

Join us for Xactware User Conference 2016 on Feb. 9-10 in Salt Lake City. Stay February 11 for an extra day of free training. Visit xactwareuserconference.com for more information.

---

This email is intended solely for the recipient. It may contain privileged, proprietary or confidential information or material. If you are not the intended recipient, please delete this email and any attachments and notify the sender of the error.

[kg]

The opt flag is an important detail, thanks for calling it out. Does the large one fail without the opt flag?

[dbh1997]

It works fine with the flag off as well.

From: Katelyn Gadd [mailto:notifications@github.com] Sent: Thursday, November 05, 2015 12:35 PM To: sq/JSIL Cc: Harris, Dave Subject: Re: [JSIL] Section of switch statement is excluded from translation (#899)

The opt flag is an important detail, thanks for calling it out. Does the large one fail without the opt flag?

— Reply to this email directly or view it on GitHubhttps://github.com/sq/JSIL/issues/899#issuecomment-154166840.

---

Join us for Xactware User Conference 2016 on Feb. 9-10 in Salt Lake City. Stay February 11 for an extra day of free training. Visit xactwareuserconference.com for more information.

---

This email is intended solely for the recipient. It may contain privileged, proprietary or confidential information or material. If you are not the intended recipient, please delete this email and any attachments and notify the sender of the error.