Преглед изворни кода

Cleanup and condense the toolchain configuration. (#436)

There was a lot of redundant noise in the older form. It also had
several issues that made the ordering and mixing together of different
flags much less obvious.

With this change, the user-provided flags are reliably placed in
a useful position, along with fundamental flags like the source file and
output.

All of this is largely in preparation for trying to add the first
sanitizer configurations (as well as enabling them by default).
Chandler Carruth пре 5 година
родитељ
комит
1aeb559a51
1 измењених фајлова са 259 додато и 332 уклоњено
  1. 259 332
      bazel/cc_toolchains/clang_cc_toolchain_config.bzl

+ 259 - 332
bazel/cc_toolchains/clang_cc_toolchain_config.bzl

@@ -99,10 +99,21 @@ def _impl(ctx):
         for name in [ACTION_NAMES.strip]
     ]
 
-    default_compile_flags_feature = feature(
-        name = "default_compile_flags",
+    default_flags_feature = feature(
+        name = "default_flags",
         enabled = True,
         flag_sets = [
+            flag_set(
+                actions = all_compile_actions + all_link_actions,
+                flag_groups = ([
+                    flag_group(
+                        flags = [
+                            "-no-canonical-prefixes",
+                            "-fcolor-diagnostics",
+                        ],
+                    ),
+                ]),
+            ),
             flag_set(
                 actions = all_compile_actions,
                 flag_groups = ([
@@ -117,64 +128,71 @@ def _impl(ctx):
                             # We use partial sets of designated initializers in
                             # test code.
                             "-Wno-missing-field-initializers",
-                            "-fcolor-diagnostics",
+                            # Compile actions shouldn't link anything.
+                            "-c",
                         ],
                     ),
+                    flag_group(
+                        expand_if_available = "output_assembly_file",
+                        flags = ["-S"],
+                    ),
+                    flag_group(
+                        expand_if_available = "output_preprocess_file",
+                        flags = ["-E"],
+                    ),
+                    flag_group(
+                        flags = ["-MD", "-MF", "%{dependency_file}"],
+                        expand_if_available = "dependency_file",
+                    ),
+                    flag_group(
+                        flags = ["-frandom-seed=%{output_file}"],
+                        expand_if_available = "output_file",
+                    ),
                 ]),
             ),
             flag_set(
-                actions = [
-                    ACTION_NAMES.assemble,
-                    ACTION_NAMES.preprocess_assemble,
-                    ACTION_NAMES.c_compile,
-                    ACTION_NAMES.cpp_compile,
-                    ACTION_NAMES.cpp_module_compile,
-                    ACTION_NAMES.cpp_header_parsing,
-                ],
-                flag_groups = [
+                actions = all_cpp_compile_actions + all_link_actions,
+                flag_groups = ([
                     flag_group(
-                        flags = ["-MD", "-MF", "%{dependency_file}"],
-                        expand_if_available = "dependency_file",
+                        flags = [
+                            "-std=c++17",
+                            "-stdlib=libc++",
+                        ],
                     ),
-                ],
+                ]),
             ),
             flag_set(
-                actions = [
-                    ACTION_NAMES.c_compile,
-                    ACTION_NAMES.cpp_compile,
-                    ACTION_NAMES.cpp_module_codegen,
-                    ACTION_NAMES.cpp_module_compile,
-                ],
-                flag_groups = [
+                actions = codegen_compile_actions,
+                flag_groups = ([
                     flag_group(
-                        flags = ["-frandom-seed=%{output_file}"],
-                        expand_if_available = "output_file",
+                        flags = [
+                            "-g0",
+                            "-O3",
+                            "-DNDEBUG",
+                            "-ffunction-sections",
+                            "-fdata-sections",
+                            # Even when optimizing, preserve frame pointers for
+                            # profiling.
+                            "-fno-omit-frame-pointer",
+                            "-mno-omit-leaf-frame-pointer",
+                        ],
                     ),
-                ],
+                ]),
+                with_features = [with_feature_set(features = ["opt"])],
             ),
             flag_set(
-                actions = [
-                    ACTION_NAMES.assemble,
-                    ACTION_NAMES.preprocess_assemble,
-                    ACTION_NAMES.linkstamp_compile,
-                    ACTION_NAMES.c_compile,
-                    ACTION_NAMES.cpp_compile,
-                    ACTION_NAMES.cpp_module_codegen,
-                    ACTION_NAMES.cpp_module_compile,
-                ],
-                flag_groups = [
-                    flag_group(flags = ["-fPIC"], expand_if_available = "pic"),
-                ],
+                actions = codegen_compile_actions,
+                flag_groups = ([
+                    flag_group(
+                        flags = ["-g"],
+                    ),
+                ]),
+                with_features = [with_feature_set(features = ["dbg"])],
             ),
             flag_set(
-                actions = [
-                    ACTION_NAMES.assemble,
-                    ACTION_NAMES.preprocess_assemble,
-                    ACTION_NAMES.c_compile,
-                    ACTION_NAMES.cpp_compile,
-                    ACTION_NAMES.cpp_module_codegen,
-                ],
+                actions = codegen_compile_actions,
                 flag_groups = [
+                    flag_group(flags = ["-fPIC"], expand_if_available = "pic"),
                     flag_group(
                         flags = ["-gsplit-dwarf", "-g"],
                         expand_if_available = "per_object_debug_info_file",
@@ -182,48 +200,27 @@ def _impl(ctx):
                 ],
             ),
             flag_set(
-                actions = [
-                    ACTION_NAMES.preprocess_assemble,
-                    ACTION_NAMES.linkstamp_compile,
-                    ACTION_NAMES.c_compile,
-                    ACTION_NAMES.cpp_compile,
-                    ACTION_NAMES.cpp_header_parsing,
-                    ACTION_NAMES.cpp_module_compile,
-                ],
+                actions = preprocessor_compile_actions,
                 flag_groups = [
+                    flag_group(
+                        flags = [
+                            # Disable a warning and override builtin macros to
+                            # ensure a hermetic build.
+                            "-Wno-builtin-macro-redefined",
+                            "-D__DATE__=\"redacted\"",
+                            "-D__TIMESTAMP__=\"redacted\"",
+                            "-D__TIME__=\"redacted\"",
+                        ],
+                    ),
                     flag_group(
                         flags = ["-D%{preprocessor_defines}"],
                         iterate_over = "preprocessor_defines",
                     ),
-                ],
-            ),
-            flag_set(
-                actions = [
-                    ACTION_NAMES.preprocess_assemble,
-                    ACTION_NAMES.linkstamp_compile,
-                    ACTION_NAMES.c_compile,
-                    ACTION_NAMES.cpp_compile,
-                    ACTION_NAMES.cpp_header_parsing,
-                    ACTION_NAMES.cpp_module_compile,
-                ],
-                flag_groups = [
                     flag_group(
                         flags = ["-include", "%{includes}"],
                         iterate_over = "includes",
                         expand_if_available = "includes",
                     ),
-                ],
-            ),
-            flag_set(
-                actions = [
-                    ACTION_NAMES.preprocess_assemble,
-                    ACTION_NAMES.linkstamp_compile,
-                    ACTION_NAMES.c_compile,
-                    ACTION_NAMES.cpp_compile,
-                    ACTION_NAMES.cpp_header_parsing,
-                    ACTION_NAMES.cpp_module_compile,
-                ],
-                flag_groups = [
                     flag_group(
                         flags = ["-iquote", "%{quote_include_paths}"],
                         iterate_over = "quote_include_paths",
@@ -239,93 +236,157 @@ def _impl(ctx):
                 ],
             ),
             flag_set(
-                actions = all_compile_actions,
-                flag_groups = ([
+                actions = [
+                    ACTION_NAMES.cpp_link_dynamic_library,
+                    ACTION_NAMES.cpp_link_nodeps_dynamic_library,
+                ],
+                flag_groups = [flag_group(flags = ["-shared"])],
+            ),
+            flag_set(
+                actions = [
+                    ACTION_NAMES.cpp_link_executable,
+                ],
+                flag_groups = [
                     flag_group(
-                        flags = ["-g"],
+                        flags = ["-pie"],
+                        expand_if_available = "force_pic",
                     ),
-                ]),
-                with_features = [with_feature_set(features = ["dbg"])],
+                ],
             ),
             flag_set(
-                actions = all_compile_actions,
-                flag_groups = ([
+                actions = all_link_actions,
+                flag_groups = [
                     flag_group(
+                        flags = ["-Wl,--gdb-index"],
+                        expand_if_available = "is_using_fission",
+                    ),
+                    flag_group(
+                        flags = ["-Wl,-S"],
+                        expand_if_available = "strip_debug_symbols",
+                    ),
+                    flag_group(
+                        flags = ["-L%{library_search_directories}"],
+                        iterate_over = "library_search_directories",
+                        expand_if_available = "library_search_directories",
+                    ),
+                    flag_group(
+                        iterate_over = "runtime_library_search_directories",
                         flags = [
-                            "-g0",
-                            "-O3",
-                            "-DNDEBUG",
-                            "-ffunction-sections",
-                            "-fdata-sections",
-                            # Even when optimizing, preserve frame pointers for profiling.
-                            "-fno-omit-frame-pointer",
-                            "-mno-omit-leaf-frame-pointer",
+                            "-Wl,-rpath,$ORIGIN/%{runtime_library_search_directories}",
                         ],
+                        expand_if_available =
+                            "runtime_library_search_directories",
                     ),
-                ]),
-                with_features = [with_feature_set(features = ["opt"])],
+                ],
             ),
+        ],
+    )
+
+    sysroot_feature = feature(
+        name = "sysroot",
+        enabled = True,
+        flag_sets = [
             flag_set(
-                actions = all_cpp_compile_actions,
-                flag_groups = ([
+                actions = all_compile_actions + all_link_actions,
+                flag_groups = [
                     flag_group(
-                        flags = [
-                            "-std=c++17",
-                            "-stdlib=libc++",
-                        ],
+                        flags = ["--sysroot=%{sysroot}"],
+                        expand_if_available = "sysroot",
                     ),
-                ]),
+                ],
             ),
+        ],
+    )
+
+    use_module_maps = feature(
+        name = "use_module_maps",
+        requires = [feature_set(features = ["module_maps"])],
+        flag_sets = [
             flag_set(
-                actions = all_compile_actions,
+                actions = [
+                    ACTION_NAMES.c_compile,
+                    ACTION_NAMES.cpp_compile,
+                    ACTION_NAMES.cpp_header_parsing,
+                    ACTION_NAMES.cpp_module_compile,
+                ],
                 flag_groups = [
+                    # These flag groups are separate so they do not expand to
+                    # the cross product of the variables.
+                    flag_group(flags = ["-fmodule-name=%{module_name}"]),
                     flag_group(
-                        flags = ["%{user_compile_flags}"],
-                        iterate_over = "user_compile_flags",
-                        expand_if_available = "user_compile_flags",
+                        flags = ["-fmodule-map-file=%{module_map_file}"],
                     ),
                 ],
             ),
+        ],
+    )
+
+    # Tell bazel we support module maps in general, so they will be generated
+    # for all c/c++ rules.
+    # Note: not all C++ rules support module maps; thus, do not imply this
+    # feature from other features - instead, require it.
+    module_maps = feature(
+        name = "module_maps",
+        enabled = True,
+        implies = [
+            # "module_map_home_cwd",
+            # "module_map_without_extern_module",
+            # "generate_submodules",
+        ],
+    )
+
+    layering_check = feature(
+        name = "layering_check",
+        implies = ["use_module_maps"],
+        flag_sets = [
             flag_set(
-                actions = all_compile_actions,
-                flag_groups = ([
+                actions = [
+                    ACTION_NAMES.c_compile,
+                    ACTION_NAMES.cpp_compile,
+                    ACTION_NAMES.cpp_header_parsing,
+                    ACTION_NAMES.cpp_module_compile,
+                ],
+                flag_groups = [
+                    flag_group(flags = [
+                        "-fmodules-strict-decluse",
+                        "-Wprivate-header",
+                    ]),
                     flag_group(
+                        iterate_over = "dependent_module_map_files",
                         flags = [
-                            "-no-canonical-prefixes",
-                            "-Wno-builtin-macro-redefined",
-                            "-D__DATE__=\"redacted\"",
-                            "-D__TIMESTAMP__=\"redacted\"",
-                            "-D__TIME__=\"redacted\"",
+                            "-fmodule-map-file=%{dependent_module_map_files}",
                         ],
                     ),
-                ]),
+                ],
             ),
+        ],
+    )
+    fuzzer = feature(
+        name = "fuzzer",
+        flag_sets = [
             flag_set(
-                actions = all_compile_actions,
+                actions = all_compile_actions + all_link_actions,
                 flag_groups = [
                     flag_group(
-                        expand_if_available = "source_file",
-                        flags = ["-c", "%{source_file}"],
-                    ),
-                    flag_group(
-                        expand_if_available = "output_assembly_file",
-                        flags = ["-S"],
-                    ),
-                    flag_group(
-                        expand_if_available = "output_preprocess_file",
-                        flags = ["-E"],
+                        flags = ["-fsanitize=fuzzer,address"],
                     ),
+                ],
+            ),
+            flag_set(
+                actions = all_link_actions,
+                flag_groups = ([
                     flag_group(
-                        expand_if_available = "output_file",
-                        flags = ["-o", "%{output_file}"],
+                        flags = [
+                            "-static-libsan",
+                        ],
                     ),
-                ],
+                ]),
             ),
         ],
     )
 
-    linux_link_flags_feature = feature(
-        name = "linux_link_flags",
+    linux_flags_feature = feature(
+        name = "linux_flags",
         enabled = True,
         flag_sets = [
             flag_set(
@@ -340,8 +401,6 @@ def _impl(ctx):
                             # name, however we have to manually link the ABI
                             # library and libunwind.
                             "-static-libstdc++",
-                            # Link with libc++.
-                            "-stdlib=libc++",
                             # Force static linking with libc++abi as well.
                             "-l:libc++abi.a",
                             # Link with Clang's runtime library. This is always
@@ -361,17 +420,10 @@ def _impl(ctx):
         ],
     )
 
-    default_link_flags_feature = feature(
-        name = "default_link_flags",
+    default_link_libraries_feature = feature(
+        name = "default_link_libraries",
         enabled = True,
         flag_sets = [
-            flag_set(
-                actions = [
-                    ACTION_NAMES.cpp_link_dynamic_library,
-                    ACTION_NAMES.cpp_link_nodeps_dynamic_library,
-                ],
-                flag_groups = [flag_group(flags = ["-shared"])],
-            ),
             flag_set(
                 actions = all_link_actions,
                 flag_groups = [
@@ -380,72 +432,6 @@ def _impl(ctx):
                         iterate_over = "linkstamp_paths",
                         expand_if_available = "linkstamp_paths",
                     ),
-                ],
-            ),
-            flag_set(
-                actions = all_link_actions,
-                flag_groups = [
-                    flag_group(
-                        flags = ["-o", "%{output_execpath}"],
-                        expand_if_available = "output_execpath",
-                    ),
-                ],
-            ),
-            flag_set(
-                actions = [
-                    ACTION_NAMES.cpp_link_executable,
-                ],
-                flag_groups = [
-                    flag_group(
-                        flags = ["-pie"],
-                        expand_if_available = "force_pic",
-                    ),
-                ],
-            ),
-            flag_set(
-                actions = all_link_actions,
-                flag_groups = [
-                    flag_group(
-                        flags = ["-L%{library_search_directories}"],
-                        iterate_over = "library_search_directories",
-                        expand_if_available = "library_search_directories",
-                    ),
-                ],
-            ),
-            flag_set(
-                flag_groups = [
-                    flag_group(
-                        iterate_over = "runtime_library_search_directories",
-                        flags = [
-                            "-Wl,-rpath,$ORIGIN/%{runtime_library_search_directories}",
-                        ],
-                        expand_if_available =
-                            "runtime_library_search_directories",
-                    ),
-                ],
-                actions = all_link_actions,
-            ),
-            flag_set(
-                actions = all_link_actions,
-                flag_groups = [
-                    flag_group(
-                        flags = ["-Wl,--gdb-index"],
-                        expand_if_available = "is_using_fission",
-                    ),
-                ],
-            ),
-            flag_set(
-                actions = all_link_actions,
-                flag_groups = [
-                    flag_group(
-                        flags = ["-Wl,-S"],
-                        expand_if_available = "strip_debug_symbols",
-                    ),
-                ],
-            ),
-            flag_set(
-                actions = all_link_actions,
-                flag_groups = [
                     flag_group(
                         iterate_over = "libraries_to_link",
                         flag_groups = [
@@ -518,23 +504,8 @@ def _impl(ctx):
                         ],
                         expand_if_available = "libraries_to_link",
                     ),
-                ],
-            ),
-            flag_set(
-                actions = all_link_actions,
-                flag_groups = [
-                    flag_group(
-                        flags = ["%{user_link_flags}"],
-                        iterate_over = "user_link_flags",
-                        expand_if_available = "user_link_flags",
-                    ),
-                ],
-            ),
-            flag_set(
-                actions = [
-                    ACTION_NAMES.cpp_link_static_library,
-                ] + all_link_actions,
-                flag_groups = [
+                    # Note that the params file comes at the end, after the
+                    # libraries to link above.
                     flag_group(
                         expand_if_available = "linker_param_file",
                         flags = ["@%{linker_param_file}"],
@@ -544,22 +515,53 @@ def _impl(ctx):
         ],
     )
 
-    sysroot_feature = feature(
-        name = "sysroot",
+    # Place user provided compile flags after all the features so that these
+    # flags can override or customize behavior. The only thing user flags
+    # cannot override is the output file as Bazel depends on that.
+    #
+    # Finally, place the source file (if present) and output file last to make
+    # reading the compile command lines easier for humans.
+    final_flags_feature = feature(
+        name = "final_flags",
         enabled = True,
         flag_sets = [
             flag_set(
-                actions = all_compile_actions + all_link_actions,
+                actions = all_compile_actions,
                 flag_groups = [
                     flag_group(
-                        flags = ["--sysroot=%{sysroot}"],
-                        expand_if_available = "sysroot",
+                        flags = ["%{user_compile_flags}"],
+                        iterate_over = "user_compile_flags",
+                        expand_if_available = "user_compile_flags",
+                    ),
+                    flag_group(
+                        flags = ["%{source_file}"],
+                        expand_if_available = "source_file",
+                    ),
+                    flag_group(
+                        expand_if_available = "output_file",
+                        flags = ["-o", "%{output_file}"],
+                    ),
+                ],
+            ),
+            flag_set(
+                actions = all_link_actions,
+                flag_groups = [
+                    flag_group(
+                        flags = ["%{user_link_flags}"],
+                        iterate_over = "user_link_flags",
+                        expand_if_available = "user_link_flags",
+                    ),
+                    flag_group(
+                        flags = ["-o", "%{output_execpath}"],
+                        expand_if_available = "output_execpath",
                     ),
                 ],
             ),
         ],
     )
 
+    # Archive actions have an entirely independent set of flags and don't
+    # interact with either compiler or link actions.
     default_archiver_flags_feature = feature(
         name = "default_archiver_flags",
         enabled = True,
@@ -572,11 +574,6 @@ def _impl(ctx):
                         flags = ["%{output_execpath}"],
                         expand_if_available = "output_execpath",
                     ),
-                ],
-            ),
-            flag_set(
-                actions = [ACTION_NAMES.cpp_link_static_library],
-                flag_groups = [
                     flag_group(
                         iterate_over = "libraries_to_link",
                         flag_groups = [
@@ -598,130 +595,60 @@ def _impl(ctx):
                         ],
                         expand_if_available = "libraries_to_link",
                     ),
-                ],
-            ),
-        ],
-    )
-
-    use_module_maps = feature(
-        name = "use_module_maps",
-        requires = [feature_set(features = ["module_maps"])],
-        flag_sets = [
-            flag_set(
-                actions = [
-                    ACTION_NAMES.c_compile,
-                    ACTION_NAMES.cpp_compile,
-                    ACTION_NAMES.cpp_header_parsing,
-                    ACTION_NAMES.cpp_module_compile,
-                ],
-                flag_groups = [
-                    # These flag groups are separate so they do not expand to
-                    # the cross product of the variables.
-                    flag_group(flags = ["-fmodule-name=%{module_name}"]),
                     flag_group(
-                        flags = ["-fmodule-map-file=%{module_map_file}"],
+                        expand_if_available = "linker_param_file",
+                        flags = ["@%{linker_param_file}"],
                     ),
                 ],
             ),
         ],
     )
 
-    # Tell bazel we support module maps in general, so they will be generated
-    # for all c/c++ rules.
-    # Note: not all C++ rules support module maps; thus, do not imply this
-    # feature from other features - instead, require it.
-    module_maps = feature(
-        name = "module_maps",
-        enabled = True,
-        implies = [
-            # "module_map_home_cwd",
-            # "module_map_without_extern_module",
-            # "generate_submodules",
-        ],
-    )
-
-    layering_check = feature(
-        name = "layering_check",
-        implies = ["use_module_maps"],
-        flag_sets = [
-            flag_set(
-                actions = [
-                    ACTION_NAMES.c_compile,
-                    ACTION_NAMES.cpp_compile,
-                    ACTION_NAMES.cpp_header_parsing,
-                    ACTION_NAMES.cpp_module_compile,
-                ],
-                flag_groups = [
-                    flag_group(flags = [
-                        "-fmodules-strict-decluse",
-                        "-Wprivate-header",
-                    ]),
-                    flag_group(
-                        iterate_over = "dependent_module_map_files",
-                        flags = [
-                            "-fmodule-map-file=%{dependent_module_map_files}",
-                        ],
-                    ),
-                ],
-            ),
-        ],
-    )
-    fuzzer = feature(
-        name = "fuzzer",
-        flag_sets = [
-            flag_set(
-                actions = all_compile_actions + all_link_actions,
-                flag_groups = [
-                    flag_group(
-                        flags = ["-fsanitize=fuzzer,address"],
-                    ),
-                ],
-            ),
-            flag_set(
-                actions = all_link_actions,
-                flag_groups = ([
-                    flag_group(
-                        flags = [
-                            "-static-libsan",
-                        ],
-                    ),
-                ]),
-            ),
-        ],
-    )
+    # Now that we have built up the constituent feature definitions, compose
+    # them, including configuration based on the target platform. Currently,
+    # the target platform is configured with the "cpu" attribute for legacy
+    # reasons. Further, for legacy reasons the default is a Linux OS target and
+    # the x88-64 CPU name is "k8".
 
-    common_features = [
+    # First, define features that are simply used to configure others.
+    features = [
         feature(name = "no_legacy_features"),
-        feature(name = "supports_pic", enabled = True),
-        default_compile_flags_feature,
-        default_archiver_flags_feature,
-        default_link_flags_feature,
         feature(name = "dbg"),
+        feature(name = "fastbuild"),
         feature(name = "opt"),
+        feature(name = "supports_pic", enabled = True),
+        feature(name = "supports_start_end_lib", enabled = ctx.attr.target_cpu == "k8"),
+        feature(name = "supports_dynamic_linker", enabled = ctx.attr.target_cpu == "k8"),
+    ]
+
+    # The order of the features determines the relative order of flags used.
+    # Start off adding the baseline features.
+    features += [
+        default_flags_feature,
         sysroot_feature,
         fuzzer,
-        module_maps,
         layering_check,
+        module_maps,
         use_module_maps,
+        default_archiver_flags_feature,
     ]
 
-    # Select the features and builtin include directories based on the target
-    # platform. Currently, this is configured with the "cpu" attribute for
-    # legacy reasons. Further, for legacy reasons the default is a Linux OS
-    # target and the x88-64 CPU name is "k8".
+    # Next, add the features based on the target platform. Here too the
+    # features are order sensitive. We also setup the sysroot here.
     if (ctx.attr.target_cpu == "k8"):
-        features = common_features + [
-            linux_link_flags_feature,
-            feature(name = "supports_start_end_lib", enabled = True),
-            feature(name = "supports_dynamic_linker", enabled = True),
-        ]
+        features += [linux_flags_feature]
         sysroot = None
     elif (ctx.attr.target_cpu == "darwin"):
-        features = common_features
         sysroot = sysroot_dir
     else:
         fail("Unsupported target platform!")
 
+    # Finally append the libraries to link and any final flags.
+    features += [
+        default_link_libraries_feature,
+        final_flags_feature,
+    ]
+
     return cc_common.create_cc_toolchain_config_info(
         ctx = ctx,
         features = features,