Remove fail("Unreachable") from features' and action_configs' declaration statements

If toolchain A and B need feature f, but toolchain C doesn't, the generated declaration statement for feature f would be:
if ctx.attr.cpu == 'A.cpu':
   f = feature(...)
elif ctx.attr.cpu == "B.cpu" and ctx.attr.compiler == "B.compiler":
   f = feature(...)
else:
   fail("Unreachable")

This will break the rule implementation in the case of toolchain C because it will reach the  fail("Unreachable") although it doesn't need to declare feature f

This cl fixes that

Issue #5380

PiperOrigin-RevId: 232683639
This commit is contained in:
rosica 2019-02-06 08:55:55 -08:00 committed by Copybara-Service
parent de27916485
commit e02152f86e
2 changed files with 85 additions and 11 deletions

View File

@ -347,7 +347,7 @@ func getStringStatement(crosstool *crosstoolpb.CrosstoolRelease,
mappedValuesToIds := getMappedStringValuesToIdentifiers(identifiers, fieldValues) mappedValuesToIds := getMappedStringValuesToIdentifiers(identifiers, fieldValues)
return getAssignmentStatement(field, mappedValuesToIds, crosstool, return getAssignmentStatement(field, mappedValuesToIds, crosstool,
cToolchainIdentifiers, depth /* isPlainString= */, true) cToolchainIdentifiers, depth /* isPlainString= */, true, /* shouldFail= */ true)
} }
func getFeatures(crosstool *crosstoolpb.CrosstoolRelease) ( func getFeatures(crosstool *crosstoolpb.CrosstoolRelease) (
@ -392,7 +392,8 @@ func getFeaturesDeclaration(crosstool *crosstoolpb.CrosstoolRelease,
crosstool, crosstool,
cToolchainIdentifiers, cToolchainIdentifiers,
depth, depth,
/* isPlainString= */ false)) /* isPlainString= */ false,
/* shouldFail= */ false))
} }
return strings.Join(res, "") return strings.Join(res, "")
} }
@ -467,7 +468,8 @@ func getActionConfigsDeclaration(
crosstool, crosstool,
cToolchainIdentifiers, cToolchainIdentifiers,
depth, depth,
/* isPlainString= */ false)) /* isPlainString= */ false,
/* shouldFail= */ false))
} }
return strings.Join(res, "") return strings.Join(res, "")
} }
@ -650,7 +652,8 @@ func getArtifactNamePatterns(crosstool *crosstoolpb.CrosstoolRelease,
crosstool, crosstool,
cToolchainIdentifiers, cToolchainIdentifiers,
depth, depth,
/* isPlainString= */ false)) /* isPlainString= */ false,
/* shouldFail= */ true))
return strings.Join(res, "\n") return strings.Join(res, "\n")
} }
@ -826,7 +829,8 @@ func getToolPaths(crosstool *crosstoolpb.CrosstoolRelease,
crosstool, crosstool,
cToolchainIdentifiers, cToolchainIdentifiers,
depth, depth,
/* isPlainString= */ false)) /* isPlainString= */ false,
/* shouldFail= */ true))
return strings.Join(res, "\n") return strings.Join(res, "\n")
} }
@ -865,7 +869,8 @@ func getMakeVariables(crosstool *crosstoolpb.CrosstoolRelease,
crosstool, crosstool,
cToolchainIdentifiers, cToolchainIdentifiers,
depth, depth,
/* isPlainString= */ false)) /* isPlainString= */ false,
/* shouldFail= */ true))
return strings.Join(res, "\n") return strings.Join(res, "\n")
} }
@ -1034,7 +1039,7 @@ func makeStringArr(arr []string, depth int, isPlainString bool) string {
func getAssignmentStatement(field string, valToIds map[string][]string, func getAssignmentStatement(field string, valToIds map[string][]string,
crosstool *crosstoolpb.CrosstoolRelease, crosstool *crosstoolpb.CrosstoolRelease,
toCToolchainIdentifier map[string]CToolchainIdentifier, toCToolchainIdentifier map[string]CToolchainIdentifier,
depth int, isPlainString bool) string { depth int, isPlainString, shouldFail bool) string {
var b bytes.Buffer var b bytes.Buffer
if len(valToIds) <= 1 { if len(valToIds) <= 1 {
// if there is only one possible value for this field, we don't need if statements // if there is only one possible value for this field, we don't need if statements
@ -1063,10 +1068,12 @@ func getAssignmentStatement(field string, valToIds map[string][]string,
toCToolchainIdentifier, depth, isPlainString)) toCToolchainIdentifier, depth, isPlainString))
first = false first = false
} }
b.WriteString( if shouldFail {
fmt.Sprintf( b.WriteString(
"%selse:\n%sfail(\"Unreachable\")\n", fmt.Sprintf(
getTabs(depth), getTabs(depth+1))) "%selse:\n%sfail(\"Unreachable\")\n",
getTabs(depth), getTabs(depth+1)))
}
} }
b.WriteString("\n") b.WriteString("\n")
return b.String() return b.String()

View File

@ -1080,6 +1080,73 @@ func TestAllAndNoneAvailableErrorsWhenMoreThanOneElement(t *testing.T) {
} }
} }
func TestNoFailUnreachableInFeaturesAndActionConfigsDeclaration(t *testing.T) {
toolchainFeatureAEnabled := getCToolchain("1", "cpuA", "compilerA",
[]string{getFeature([]string{"name: 'A'", "enabled: true"})},
)
toolchainFeatureADisabled := getCToolchain("2", "cpuA", "compilerB",
[]string{getFeature([]string{"name: 'A'", "enabled: false"})},
)
toolchainWithoutFeatureA := getCToolchain("3", "cpuA", "compilerC", []string{})
toolchainActionConfigAEnabled := getCToolchain("4", "cpuA", "compilerD",
[]string{getActionConfig([]string{
"config_name: 'A'",
"action_name: 'A'",
"enabled: true",
})})
toolchainActionConfigADisabled := getCToolchain("5", "cpuA", "compilerE",
[]string{getActionConfig([]string{
"config_name: 'A'",
"action_name: 'A'",
})})
toolchainWithoutActionConfigA := getCToolchain("6", "cpuA", "compilerF", []string{})
testCases := []struct {
field string
toolchains []string
expectedText string
}{
{field: "features",
toolchains: []string{
toolchainFeatureAEnabled, toolchainFeatureADisabled, toolchainWithoutFeatureA},
expectedText: `
if (ctx.attr.cpu == "cpuA" and ctx.attr.compiler == "compilerB"):
a_feature = feature(name = "A")
elif (ctx.attr.cpu == "cpuA" and ctx.attr.compiler == "compilerA"):
a_feature = feature(name = "A", enabled = True)
` /* empty line after the elif means there's no else statement */},
{field: "action_config",
toolchains: []string{
toolchainActionConfigAEnabled, toolchainActionConfigADisabled, toolchainWithoutActionConfigA},
expectedText: `
if (ctx.attr.cpu == "cpuA" and ctx.attr.compiler == "compilerE"):
a_action = action_config(action_name = "A")
elif (ctx.attr.cpu == "cpuA" and ctx.attr.compiler == "compilerD"):
a_action = action_config(action_name = "A", enabled = True)
` /* empty line after the elif means there's no else statement */ },
}
for _, tc := range testCases {
crosstool := makeCrosstool(tc.toolchains)
got, err := Transform(crosstool)
if err != nil {
t.Fatalf("CROSSTOOL conversion failed: %v", err)
}
if !strings.Contains(got, tc.expectedText) {
t.Errorf("Failed to correctly convert '%s' field, expected to contain:\n%v\n",
tc.field, tc.expectedText)
t.Fatalf("Tested CROSSTOOL:\n%v\n\nGenerated rule:\n%v\n",
strings.Join(tc.toolchains, "\n"), got)
}
}
}
func TestActionConfigDeclaration(t *testing.T) { func TestActionConfigDeclaration(t *testing.T) {
toolchainEmpty1 := getCToolchain("1", "cpuA", "compilerA", []string{}) toolchainEmpty1 := getCToolchain("1", "cpuA", "compilerA", []string{})
toolchainEmpty2 := getCToolchain("2", "cpuB", "compilerA", []string{}) toolchainEmpty2 := getCToolchain("2", "cpuB", "compilerA", []string{})