Merge "Split asm and c flags and srcs in bp2build output"

android/bazel.go

@@ -220,15 +220,7 @@ var (
 	// Per-module denylist to opt modules out of mixed builds. Such modules will
 	// still be generated via bp2build.
 	mixedBuildsDisabledList = []string{
-		"libc_common",                      // cparsons@ cc_library_static, depends on //bionic/libc:libc_nopthread
+		"libc_common_shared", // cparsons@ cc_library_static, version script assignment of 'LIBC' to symbol '__cxa_atexit' failed: symbol not defined
-		"libc_common_static",               // cparsons@ cc_library_static, depends on //bionic/libc:libc_common
-		"libc_common_shared",               // cparsons@ cc_library_static, depends on //bionic/libc:libc_common
-		"libc_netbsd",                      // lberki@, cc_library_static, version script assignment of 'LIBC_PRIVATE' to symbol 'SHA1Final' failed: symbol not defined
-		"libc_nopthread",                   // cparsons@ cc_library_static, version script assignment of 'LIBC' to symbol 'memcmp' failed: symbol not defined
-		"libc_openbsd",                     // ruperts@, cc_library_static, OK for bp2build but error: duplicate symbol: strcpy for mixed builds
-		"libarm-optimized-routines-string", // jingwen@, cc_library_static, OK for bp2build but b/186615213 (asflags not handled in  bp2build), version script assignment of 'LIBC' to symbol 'memcmp' failed: symbol not defined (also for memrchr, strnlen)
-		"fmtlib_ndk",                       // http://b/187040371, cc_library_static, OK for bp2build but format-inl.h:11:10: fatal error: 'cassert' file not found for mixed builds
-		"libc_nomalloc",                    // cc_library_static, OK for bp2build but ld.lld: error: undefined symbol: pthread_mutex_lock (and others)
 	}
 
 	// Used for quicker lookups

bazel/properties.go

@@ -137,6 +137,54 @@ func SubtractStrings(haystack []string, needle []string) []string {
 	return strings
 }
 
+// Return all needles in a given haystack, where needleFn is true for needles.
+func FilterLabelList(haystack LabelList, needleFn func(string) bool) LabelList {
+	var includes []Label
+
+	for _, inc := range haystack.Includes {
+		if needleFn(inc.Label) {
+			includes = append(includes, inc)
+		}
+	}
+	return LabelList{Includes: includes, Excludes: haystack.Excludes}
+}
+
+// Return all needles in a given haystack, where needleFn is true for needles.
+func FilterLabelListAttribute(haystack LabelListAttribute, needleFn func(string) bool) LabelListAttribute {
+	var result LabelListAttribute
+
+	result.Value = FilterLabelList(haystack.Value, needleFn)
+
+	for arch := range PlatformArchMap {
+		result.SetValueForArch(arch, FilterLabelList(haystack.GetValueForArch(arch), needleFn))
+	}
+
+	for os := range PlatformOsMap {
+		result.SetValueForOS(os, FilterLabelList(haystack.GetValueForOS(os), needleFn))
+	}
+
+	return result
+}
+
+// Subtract needle from haystack
+func SubtractBazelLabelListAttribute(haystack LabelListAttribute, needle LabelListAttribute) LabelListAttribute {
+	var result LabelListAttribute
+
+	for arch := range PlatformArchMap {
+		result.SetValueForArch(arch,
+			SubtractBazelLabelList(haystack.GetValueForArch(arch), needle.GetValueForArch(arch)))
+	}
+
+	for os := range PlatformOsMap {
+		result.SetValueForOS(os,
+			SubtractBazelLabelList(haystack.GetValueForOS(os), needle.GetValueForOS(os)))
+	}
+
+	result.Value = SubtractBazelLabelList(haystack.Value, needle.Value)
+
+	return result
+}
+
 // Subtract needle from haystack
 func SubtractBazelLabels(haystack []Label, needle []Label) []Label {
 	// This is really a set

bp2build/cc_library_conversion_test.go

@@ -311,7 +311,7 @@ cc_library {
         "//build/bazel/platforms/arch:arm64": ["-DHAVE_FAST_FMA=1"],
         "//conditions:default": [],
     }),
-    srcs = ["math/cosf.c"],
+    srcs_c = ["math/cosf.c"],
 )`},
 	})
 }
@@ -618,7 +618,7 @@ cc_library {
 
 func TestCcLibraryCppFlagsGoesIntoCopts(t *testing.T) {
 	runCcLibraryTestCase(t, bp2buildTestCase{
-		description:                        "cc_library cppflags goes into copts",
+		description:                        "cc_library cppflags usage",
 		moduleTypeUnderTest:                "cc_library",
 		moduleTypeUnderTestFactory:         cc.LibraryFactory,
 		moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryBp2Build,
@@ -654,10 +654,12 @@ func TestCcLibraryCppFlagsGoesIntoCopts(t *testing.T) {
     name = "a",
     copts = [
         "-Wall",
-        "-fsigned-char",
-        "-pedantic",
         "-Ifoo/bar",
         "-I$(BINDIR)/foo/bar",
+    ],
+    cppflags = [
+        "-fsigned-char",
+        "-pedantic",
     ] + select({
         "//build/bazel/platforms/arch:arm64": ["-DARM64=1"],
         "//conditions:default": [],

bp2build/cc_library_static_conversion_test.go

@@ -650,7 +650,7 @@ cc_library_static {
         "-I$(BINDIR)/.",
     ],
     linkstatic = True,
-    srcs = [
+    srcs_c = [
         "common.c",
         "foo-a.c",
     ],
@@ -682,7 +682,7 @@ cc_library_static {
         "-I$(BINDIR)/.",
     ],
     linkstatic = True,
-    srcs = ["common.c"] + select({
+    srcs_c = ["common.c"] + select({
         "//build/bazel/platforms/arch:arm": ["foo-arm.c"],
         "//conditions:default": [],
     }),
@@ -719,7 +719,7 @@ cc_library_static {
         "-I$(BINDIR)/.",
     ],
     linkstatic = True,
-    srcs = ["common.c"] + select({
+    srcs_c = ["common.c"] + select({
         "//build/bazel/platforms/arch:arm": ["for-arm.c"],
         "//conditions:default": ["not-for-arm.c"],
     }),
@@ -758,7 +758,7 @@ cc_library_static {
         "-I$(BINDIR)/.",
     ],
     linkstatic = True,
-    srcs = ["common.c"] + select({
+    srcs_c = ["common.c"] + select({
         "//build/bazel/platforms/arch:arm": [
             "for-arm.c",
             "not-for-x86.c",
@@ -813,7 +813,7 @@ cc_library_static {
         "-I$(BINDIR)/.",
     ],
     linkstatic = True,
-    srcs = ["common.c"] + select({
+    srcs_c = ["common.c"] + select({
         "//build/bazel/platforms/arch:arm": [
             "for-arm.c",
             "not-for-arm64.c",
@@ -909,7 +909,7 @@ cc_library_static {
         "-I$(BINDIR)/.",
     ],
     linkstatic = True,
-    srcs = ["common.c"] + select({
+    srcs_c = ["common.c"] + select({
         "//build/bazel/platforms/arch:arm": ["for-lib32.c"],
         "//build/bazel/platforms/arch:x86": ["for-lib32.c"],
         "//conditions:default": ["not-for-lib32.c"],
@@ -948,7 +948,7 @@ cc_library_static {
         "-I$(BINDIR)/.",
     ],
     linkstatic = True,
-    srcs = ["common.c"] + select({
+    srcs_c = ["common.c"] + select({
         "//build/bazel/platforms/arch:arm": [
             "for-lib32.c",
             "not-for-lib64.c",
@@ -1020,7 +1020,7 @@ cc_library_static {
         "-I$(BINDIR)/.",
     ],
     linkstatic = True,
-    srcs = ["common.c"] + select({
+    srcs_c = ["common.c"] + select({
         "//build/bazel/platforms/arch:arm": [
             "for-arm.c",
             "for-lib32.c",
@@ -1074,10 +1074,10 @@ func TestCcLibraryStaticArchSrcsExcludeSrcsGeneratedFiles(t *testing.T) {
 		moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryStaticBp2Build,
 		depsMutators:                       []android.RegisterMutatorFunc{cc.RegisterDepsBp2Build},
 		filesystem: map[string]string{
-			"common.c":             "",
+			"common.cpp":             "",
-			"for-x86.c":            "",
+			"for-x86.cpp":            "",
-			"not-for-x86.c":        "",
+			"not-for-x86.cpp":        "",
-			"not-for-everything.c": "",
+			"not-for-everything.cpp": "",
 			"dep/Android.bp": `
 genrule {
 	name: "generated_src_other_pkg",
@@ -1118,14 +1118,14 @@ genrule {
 
 cc_library_static {
    name: "foo_static3",
-   srcs: ["common.c", "not-for-*.c"],
+   srcs: ["common.cpp", "not-for-*.cpp"],
-   exclude_srcs: ["not-for-everything.c"],
+   exclude_srcs: ["not-for-everything.cpp"],
    generated_sources: ["generated_src", "generated_src_other_pkg"],
    generated_headers: ["generated_hdr", "generated_hdr_other_pkg"],
    arch: {
        x86: {
-           srcs: ["for-x86.c"],
+           srcs: ["for-x86.cpp"],
-           exclude_srcs: ["not-for-x86.c"],
+           exclude_srcs: ["not-for-x86.cpp"],
            generated_sources: ["generated_src_x86"],
            generated_headers: ["generated_hdr_other_pkg_x86"],
        },
@@ -1144,14 +1144,14 @@ cc_library_static {
         "//dep:generated_src_other_pkg",
         ":generated_hdr",
         ":generated_src",
-        "common.c",
+        "common.cpp",
     ] + select({
         "//build/bazel/platforms/arch:x86": [
             "//dep:generated_hdr_other_pkg_x86",
             ":generated_src_x86",
-            "for-x86.c",
+            "for-x86.cpp",
         ],
-        "//conditions:default": ["not-for-x86.c"],
+        "//conditions:default": ["not-for-x86.cpp"],
     }),
 )`},
 	})
@@ -1269,7 +1269,7 @@ cc_library_static {
         "//conditions:default": [],
     }),
     linkstatic = True,
-    srcs = ["common.c"],
+    srcs_c = ["common.c"],
 )`},
 	})
 }

cc/bp2build.go

@@ -183,15 +183,26 @@ func bp2BuildParseStaticProps(ctx android.TopDownMutatorContext, module *Module)
 
 // Convenience struct to hold all attributes parsed from compiler properties.
 type compilerAttributes struct {
-	copts    bazel.StringListAttribute
+	// Options for all languages
+	copts bazel.StringListAttribute
+	// Assembly options and sources
+	asFlags bazel.StringListAttribute
+	asSrcs  bazel.LabelListAttribute
+	// C options and sources
+	conlyFlags bazel.StringListAttribute
+	cSrcs      bazel.LabelListAttribute
+	// C++ options and sources
+	cppFlags bazel.StringListAttribute
 	srcs     bazel.LabelListAttribute
-	includes bazel.StringListAttribute
 }
 
 // bp2BuildParseCompilerProps returns copts, srcs and hdrs and other attributes.
 func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Module) compilerAttributes {
 	var srcs bazel.LabelListAttribute
 	var copts bazel.StringListAttribute
+	var asFlags bazel.StringListAttribute
+	var conlyFlags bazel.StringListAttribute
+	var cppFlags bazel.StringListAttribute
 
 	// Creates the -I flags for a directory, while making the directory relative
 	// to the exec root for Bazel to work.
@@ -215,15 +226,21 @@ func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Modul
 		return append(includeDirs, baseCompilerProps.Local_include_dirs...)
 	}
 
-	// Parse the list of copts.
+	parseCommandLineFlags := func(soongFlags []string) []string {
-	parseCopts := func(baseCompilerProps *BaseCompilerProperties) []string {
+		var result []string
-		var copts []string
+		for _, flag := range soongFlags {
-		for _, flag := range append(baseCompilerProps.Cflags, baseCompilerProps.Cppflags...) {
 			// Soong's cflags can contain spaces, like `-include header.h`. For
 			// Bazel's copts, split them up to be compatible with the
 			// no_copts_tokenization feature.
-			copts = append(copts, strings.Split(flag, " ")...)
+			result = append(result, strings.Split(flag, " ")...)
 		}
+		return result
+	}
+
+	// Parse the list of copts.
+	parseCopts := func(baseCompilerProps *BaseCompilerProperties) []string {
+		var copts []string
+		copts = append(copts, parseCommandLineFlags(baseCompilerProps.Cflags)...)
 		for _, dir := range parseLocalIncludeDirs(baseCompilerProps) {
 			copts = append(copts, includeFlags(dir)...)
 		}
@@ -260,6 +277,9 @@ func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Modul
 		if baseCompilerProps, ok := props.(*BaseCompilerProperties); ok {
 			srcs.Value = parseSrcs(baseCompilerProps)
 			copts.Value = parseCopts(baseCompilerProps)
+			asFlags.Value = parseCommandLineFlags(baseCompilerProps.Asflags)
+			conlyFlags.Value = parseCommandLineFlags(baseCompilerProps.Conlyflags)
+			cppFlags.Value = parseCommandLineFlags(baseCompilerProps.Cppflags)
 
 			// Used for arch-specific srcs later.
 			baseSrcs = baseCompilerProps.Srcs
@@ -290,6 +310,9 @@ func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Modul
 			}
 
 			copts.SetValueForArch(arch.Name, parseCopts(baseCompilerProps))
+			asFlags.SetValueForArch(arch.Name, parseCommandLineFlags(baseCompilerProps.Asflags))
+			conlyFlags.SetValueForArch(arch.Name, parseCommandLineFlags(baseCompilerProps.Conlyflags))
+			cppFlags.SetValueForArch(arch.Name, parseCommandLineFlags(baseCompilerProps.Cppflags))
 		}
 	}
 
@@ -315,6 +338,9 @@ func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Modul
 			// TODO(b/186153868): add support for os-specific srcs and exclude_srcs
 			srcs.SetValueForOS(os.Name, bazel.SubtractBazelLabelList(srcsList, baseSrcsLabelList))
 			copts.SetValueForOS(os.Name, parseCopts(baseCompilerProps))
+			asFlags.SetValueForOS(os.Name, parseCommandLineFlags(baseCompilerProps.Asflags))
+			conlyFlags.SetValueForOS(os.Name, parseCommandLineFlags(baseCompilerProps.Conlyflags))
+			cppFlags.SetValueForOS(os.Name, parseCommandLineFlags(baseCompilerProps.Cppflags))
 		}
 	}
 
@@ -333,9 +359,28 @@ func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Modul
 		}
 	}
 
+	// Branch srcs into three language-specific groups.
+	// C++ is the "catch-all" group, and comprises generated sources because we don't
+	// know the language of these sources until the genrule is executed.
+	// TODO(b/): Handle language detection of sources in a Bazel rule.
+	isCSrc := func(s string) bool {
+		return strings.HasSuffix(s, ".c")
+	}
+	isAsmSrc := func(s string) bool {
+		return strings.HasSuffix(s, ".S") || strings.HasSuffix(s, ".s")
+	}
+	cSrcs := bazel.FilterLabelListAttribute(srcs, isCSrc)
+	asSrcs := bazel.FilterLabelListAttribute(srcs, isAsmSrc)
+	srcs = bazel.SubtractBazelLabelListAttribute(srcs, cSrcs)
+	srcs = bazel.SubtractBazelLabelListAttribute(srcs, asSrcs)
 	return compilerAttributes{
-		srcs:  srcs,
+		copts:      copts,
-		copts: copts,
+		srcs:       srcs,
+		asFlags:    asFlags,
+		asSrcs:     asSrcs,
+		cSrcs:      cSrcs,
+		conlyFlags: conlyFlags,
+		cppFlags:   cppFlags,
 	}
 }
 

cc/library.go

@@ -230,6 +230,13 @@ type bazelCcLibraryAttributes struct {
 	Copts               bazel.StringListAttribute
 	Includes            bazel.StringListAttribute
 	Linkopts            bazel.StringListAttribute
+
+	Cppflags   bazel.StringListAttribute
+	Srcs_c     bazel.LabelListAttribute
+	Conlyflags bazel.StringListAttribute
+	Srcs_as    bazel.LabelListAttribute
+	Asflags    bazel.StringListAttribute
+
 	// Attributes pertaining to shared variant.
 	Shared_copts                  bazel.StringListAttribute
 	Shared_srcs                   bazel.LabelListAttribute
@@ -239,6 +246,7 @@ type bazelCcLibraryAttributes struct {
 	Whole_archive_deps_for_shared bazel.LabelListAttribute
 	User_link_flags               bazel.StringListAttribute
 	Version_script                bazel.LabelAttribute
+
 	// Attributes pertaining to static variant.
 	Static_copts                  bazel.StringListAttribute
 	Static_srcs                   bazel.LabelListAttribute
@@ -294,20 +302,27 @@ func CcLibraryBp2Build(ctx android.TopDownMutatorContext) {
 	srcs.Append(compilerAttrs.srcs)
 
 	attrs := &bazelCcLibraryAttributes{
-		Srcs:                          srcs,
+		Srcs:                srcs,
-		Implementation_deps:           linkerAttrs.deps,
+		Implementation_deps: linkerAttrs.deps,
-		Deps:                          linkerAttrs.exportedDeps,
+		Deps:                linkerAttrs.exportedDeps,
-		Dynamic_deps:                  linkerAttrs.dynamicDeps,
+		Dynamic_deps:        linkerAttrs.dynamicDeps,
-		Whole_archive_deps:            linkerAttrs.wholeArchiveDeps,
+		Whole_archive_deps:  linkerAttrs.wholeArchiveDeps,
-		Copts:                         compilerAttrs.copts,
+		Copts:               compilerAttrs.copts,
-		Includes:                      exportedIncludes,
+		Includes:            exportedIncludes,
-		Linkopts:                      linkerAttrs.linkopts,
+		Linkopts:            linkerAttrs.linkopts,
+		Cppflags:            compilerAttrs.cppFlags,
+		Srcs_c:              compilerAttrs.cSrcs,
+		Conlyflags:          compilerAttrs.conlyFlags,
+		Srcs_as:             compilerAttrs.asSrcs,
+		Asflags:             compilerAttrs.asFlags,
+
 		Shared_copts:                  sharedAttrs.copts,
 		Shared_srcs:                   sharedAttrs.srcs,
 		Static_deps_for_shared:        sharedAttrs.staticDeps,
 		Whole_archive_deps_for_shared: sharedAttrs.wholeArchiveDeps,
 		Dynamic_deps_for_shared:       sharedAttrs.dynamicDeps,
 		Version_script:                linkerAttrs.versionScript,
+
 		Static_copts:                  staticAttrs.copts,
 		Static_srcs:                   staticAttrs.srcs,
 		Static_deps_for_static:        staticAttrs.staticDeps,
@@ -2230,6 +2245,12 @@ type bazelCcLibraryStaticAttributes struct {
 	Linkstatic          bool
 	Includes            bazel.StringListAttribute
 	Hdrs                bazel.LabelListAttribute
+
+	Cppflags   bazel.StringListAttribute
+	Srcs_c     bazel.LabelListAttribute
+	Conlyflags bazel.StringListAttribute
+	Srcs_as    bazel.LabelListAttribute
+	Asflags    bazel.StringListAttribute
 }
 
 type bazelCcLibraryStatic struct {
@@ -2259,6 +2280,12 @@ func ccLibraryStaticBp2BuildInternal(ctx android.TopDownMutatorContext, module *
 		Linkopts:   linkerAttrs.linkopts,
 		Linkstatic: true,
 		Includes:   exportedIncludes,
+
+		Cppflags:   compilerAttrs.cppFlags,
+		Srcs_c:     compilerAttrs.cSrcs,
+		Conlyflags: compilerAttrs.conlyFlags,
+		Srcs_as:    compilerAttrs.asSrcs,
+		Asflags:    compilerAttrs.asFlags,
 	}
 
 	props := bazel.BazelTargetModuleProperties{

cc/object.go

@@ -185,8 +185,14 @@ func ObjectBp2Build(ctx android.TopDownMutatorContext) {
 	}
 	// TODO(b/183595872) warn/error if we're not handling product variables
 
+	// Don't split cc_object srcs across languages. Doing so would add complexity,
+	// and this isn't typically done for cc_object.
+	srcs := compilerAttrs.srcs
+	srcs.Append(compilerAttrs.cSrcs)
+	srcs.Append(compilerAttrs.asSrcs)
+
 	attrs := &bazelObjectAttributes{
-		Srcs:    compilerAttrs.srcs,
+		Srcs:    srcs,
 		Deps:    deps,
 		Copts:   compilerAttrs.copts,
 		Asflags: asFlags,