bazel: Make generator configurable, minor improvements

This change merges the internal version of `sapi.bzl` with the external version again:
- Add more docstrings to the various macros
- Skip creation of `.isystem` file, get info from toolchain instead

PiperOrigin-RevId: 437730588
Change-Id: I6f670d32e3d7177a6a160fd24cbee6f8f3ca9503

sandboxed_api/bazel/sapi.bzl

@@ -12,9 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""Macros that simplifies header and library generation for Sandboxed API."""
+"""Starlark rules for projects using Sandboxed API."""
 
 load("//sandboxed_api/bazel:embed_data.bzl", "sapi_cc_embed_data")
+load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain")
 
 # Helper functions
 def append_arg(arguments, name, value):
@@ -31,6 +32,16 @@ def get_embed_dir():
     return native.package_name()
 
 def sort_deps(deps):
+    """Sorts a list of dependencies.
+
+    This does not convert absolute references targeting the current package
+    into relative ones.
+
+    Args:
+      deps: List of labels to be sorted
+    Returns:
+      A sorted list of dependencies, with local deps (starting with ":") first.
+    """
     deps = depset(deps).to_list()
     colon_deps = [x for x in deps if x.startswith(":")]
     other_deps = [x for x in deps if not x.startswith(":")]
@@ -39,6 +50,9 @@ def sort_deps(deps):
 def sapi_interface_impl(ctx):
     """Implementation of build rule that generates SAPI interface."""
 
+    cpp_toolchain = find_cpp_toolchain(ctx)
+    use_clang_generator = ctx.executable.generator.basename == "generator_tool"
+
     # TODO(szwl): warn if input_files is not set and we didn't find anything
     input_files_paths = []
     input_files = []
@@ -50,10 +64,6 @@ def sapi_interface_impl(ctx):
     append_arg(args, "--sapi_embed_name", ctx.attr.embed_name)
     append_arg(args, "--sapi_functions", ",".join(ctx.attr.functions))
     append_arg(args, "--sapi_ns", ctx.attr.namespace)
-    if ctx.attr.isystem:
-        isystem = ctx.attr.isystem.files.to_list()[0]
-        append_arg(args, "--sapi_isystem", isystem.path)
-        input_files += [isystem]
 
     if ctx.attr.limit_scan_depth:
         args.append("--sapi_limit_scan_depth")
@@ -67,7 +77,7 @@ def sapi_interface_impl(ctx):
     # package path. Including extra headers is harmless except that
     # we may hit Bazel's file-count limit, so be conservative and
     # pass a lot through that we don't strictly need.
-
+    #
     extra_flags = []
     cc_ctx = ctx.attr.lib[CcInfo].compilation_context
 
@@ -75,20 +85,31 @@ def sapi_interface_impl(ctx):
     input_files += cc_ctx.headers.to_list()
 
     quote_includes = cc_ctx.quote_includes.to_list()
-    append_all(extra_flags, "-D", cc_ctx.defines.to_list())
+
-    append_all(extra_flags, "-isystem", cc_ctx.system_includes.to_list())
+    if use_clang_generator:
-    append_all(extra_flags, "-iquote", quote_includes)
+        input_files += cpp_toolchain.all_files.to_list()
+
+        # TODO(cblichmann): Get language standard from the toolchain
+        extra_flags.append("--extra-arg=-std=c++17")
+
+        # Disable warnings in parsed code
+        extra_flags.append("--extra-arg=-Wno-everything")
+        extra_flags += ["--extra-arg=-isystem{}".format(d) for d in cpp_toolchain.built_in_include_directories]
+        extra_flags += ["--extra-arg=-D{}".format(d) for d in cc_ctx.defines.to_list()]
+        extra_flags += ["--extra-arg=-isystem{}".format(i) for i in cc_ctx.system_includes.to_list()]
+        extra_flags += ["--extra-arg=-iquote{}".format(i) for i in quote_includes]
+    else:
+        append_all(extra_flags, "-D", cc_ctx.defines.to_list())
+        append_all(extra_flags, "-isystem", cc_ctx.system_includes.to_list())
+        append_all(extra_flags, "-iquote", quote_includes)
 
     if ctx.attr.input_files:
-        for target in ctx.attr.input_files:
+        for f in ctx.files.input_files:
-            if target.files:
+            input_files.append(f)
-                for f in target.files.to_list():
+            input_files_paths.append(f.path)
-                    input_files_paths.append(f.path)
-                    input_files.append(f)
     else:
         # Try to find files automatically
         for h in cc_ctx.direct_headers:
-            # Collect all headers as dependency.
             if h.extension != "h" or "/PROTECTED/" in h.path:
                 continue
 
@@ -97,8 +118,11 @@ def sapi_interface_impl(ctx):
             if (h.owner.package == ctx.attr.lib.label.package):
                 input_files_paths.append(h.path)
 
-    append_arg(args, "--sapi_in", ",".join(input_files_paths))
+    if use_clang_generator:
-    args += ["--"] + extra_flags
+        args += extra_flags + input_files_paths
+    else:
+        append_arg(args, "--sapi_in", ",".join(input_files_paths))
+        args += ["--"] + extra_flags
 
     progress_msg = ("Generating {} from {} header files." +
                     "").format(ctx.outputs.out.short_path, len(input_files_paths))
@@ -107,7 +131,7 @@ def sapi_interface_impl(ctx):
         outputs = [ctx.outputs.out],
         arguments = args,
         progress_message = progress_msg,
-        executable = ctx.executable._sapi_generator,
+        executable = ctx.executable.generator,
     )
 
 # Build rule that generates SAPI interface.
@@ -118,22 +142,23 @@ sapi_interface = rule(
         "embed_dir": attr.string(),
         "embed_name": attr.string(),
         "functions": attr.string_list(allow_empty = True, default = []),
-        "include_prefix": attr.string(),
         "input_files": attr.label_list(allow_files = True),
         "lib": attr.label(providers = [CcInfo], mandatory = True),
         "lib_name": attr.string(mandatory = True),
         "namespace": attr.string(),
-        "isystem": attr.label(),
         "limit_scan_depth": attr.bool(default = False),
-        "_sapi_generator": attr.label(
+        "generator": attr.label(
             executable = True,
             cfg = "host",
             allow_files = True,
-            default = Label("@com_google_sandboxed_api//sandboxed_api/" +
+            default = Label("//sandboxed_api/tools/generator2:sapi_generator"),
-                            "tools/generator2:sapi_generator"),
+        ),
+        "_cc_toolchain": attr.label(
+            default = Label("@bazel_tools//tools/cpp:current_cc_toolchain"),
         ),
     },
     output_to_genfiles = True,
+    toolchains = ["@bazel_tools//tools/cpp:toolchain_type"],
 )
 
 def sapi_library(
@@ -143,6 +168,7 @@ def sapi_library(
         namespace = "",
         embed = True,
         add_default_deps = True,
+        limit_scan_depth = False,
         srcs = [],
         hdrs = [],
         functions = [],
@@ -150,10 +176,35 @@ def sapi_library(
         input_files = [],
         deps = [],
         tags = [],
+        generator_executable = "//sandboxed_api/tools/generator2:sapi_generator",
         visibility = None):
-    """Provides the implementation of a Sandboxed API library."""
+    """Provides the implementation of a Sandboxed API library.
+
+    Args:
+      name: Name of the sandboxed library
+      lib: Label of the library target to sandbox
+      lib_name: Name of the class which will proxy the library functions from
+        the functions list
+      malloc: Override the default dependency on malloc
+      namespace: A C++ namespace identifier to place the API class into
+      embed: Whether the SAPI library should be embedded inside the host code
+      add_default_deps: Add SAPI dependencies to target (deprecated)
+      limit_scan_depth: Limit include depth for header generator (deprecated)
+      srcs: Any additional sources to include with the sandboxed library
+      hdrs: Like srcs, any additional headers to include with the sandboxed
+        library
+      functions: A list for function to use from host code
+      header: If set, do not generate a header, but use the specified one
+        (deprecated).
+      input_files: List of source files which the SAPI interface generator
+        should scan for function declaration
+      deps: Extra dependencies to add to the SAPI library
+      tags: Extra tags to associate with the target
+      generator_executable: Label of the SAPI interface generator to use
+        (experimental).
+      visibility: Target visibility
+    """
 
-    rprefix = "@com_google_sandboxed_api"
     common = {
         "tags": tags,
     }
@@ -164,7 +215,7 @@ def sapi_library(
 
     # Reference (pull into the archive) required functions only. If the functions'
     # array is empty, pull in the whole archive (may not compile with MSAN).
-    exported_funcs = ["-Wl,--export-dynamic-symbol," + s for s in functions]
+    exported_funcs = ["-Wl,-u," + s for s in functions]
     if (not exported_funcs):
         exported_funcs = [
             "-Wl,--whole-archive",
@@ -177,7 +228,7 @@ def sapi_library(
     else:
         lib_hdrs += [generated_header]
 
-    default_deps = [rprefix + "//sandboxed_api/sandbox2"]
+    default_deps = ["//sandboxed_api/sandbox2"]
 
     # Library that contains generated interface and sandboxed binary as a data
     # dependency. Add this as a dependency instead of original library.
@@ -188,11 +239,12 @@ def sapi_library(
         data = [":" + name + ".bin"],
         deps = sort_deps(
             [
+                "@com_google_absl//absl/base:core_headers",
                 "@com_google_absl//absl/status",
                 "@com_google_absl//absl/status:statusor",
-                rprefix + "//sandboxed_api:sapi",
+                "//sandboxed_api:sapi",
-                rprefix + "//sandboxed_api/util:status",
+                "//sandboxed_api/util:status",
-                rprefix + "//sandboxed_api:vars",
+                "//sandboxed_api:vars",
             ] + deps +
             ([":" + name + "_embed"] if embed else []) +
             (default_deps if add_default_deps else []),
@@ -204,14 +256,12 @@ def sapi_library(
         name = name + ".bin",
         linkopts = [
             "-ldl",  # For dlopen(), dlsym()
-            # The sandboxing client must have access to all symbols used in
+            # The sandboxing client must have access to all
-            # the sandboxed library, so these must be both referenced, and
+            "-Wl,-E",  # symbols used in the sandboxed library, so these
-            # exported
+        ] + exported_funcs,  # must be both referenced, and exported
-            "-Wl,-E",
-        ] + exported_funcs,
         deps = [
             ":" + name + ".lib",
-            rprefix + "//sandboxed_api:client",
+            "//sandboxed_api:client",
         ],
         **common
     )
@@ -246,15 +296,7 @@ def sapi_library(
         embed_name = embed_name,
         embed_dir = embed_dir,
         namespace = namespace,
-        isystem = ":" + name + ".isystem",
+        generator = generator_executable,
+        limit_scan_depth = limit_scan_depth,
         **common
     )
-
-    native.genrule(
-        name = name + ".isystem",
-        outs = [name + ".isystem.list"],
-        cmd = """$(CC) -E -x c++ -v /dev/null 2>&1 |
-                 awk '/> search starts here:/{f=1;next}/^End of search/{f=0}f{print $$1}' > $@
-              """,
-        toolchains = ["@bazel_tools//tools/cpp:current_cc_toolchain"],
-    )