Handle features and target_features in cfg

Author: dzbarsky

rs/extensions.bzl

@@ -31,6 +31,15 @@ def _platform(triple, use_experimental_platforms):
 def _select(items):
     return {k: sorted(v) for k, v in items.items()}
 
+def _cfg_match_info_for_target(target, platform_cfg_attrs, cfg_match_cache):
+    match_info = cfg_match_cache.get(target)
+    if match_info:
+        return match_info
+
+    match_info = cfg_matches_expr_for_cfg_attrs(target, platform_cfg_attrs)
+    cfg_match_cache[target] = match_info
+    return match_info
+
 def _add_to_dict(d, k, v):
     existing = d.get(k, [])
     if not existing:
@@ -248,14 +257,17 @@ def _generate_hub_and_spokes(
         pkg["local_path"] = local_path
         packages.append(pkg)
 
-    platform_cfg_attrs = [triple_to_cfg_attrs(triple, [], []) for triple in platform_triples]
+    platform_cfg_attrs = [triple_to_cfg_attrs(triple) for triple in platform_triples]
+    platform_cfg_attrs_by_triple = {}
+    for cfg_attr in platform_cfg_attrs:
+        platform_cfg_attrs_by_triple[cfg_attr["_triple"]] = cfg_attr
 
     mctx.report_progress("Computing dependencies and features")
 
     feature_resolutions_by_fq_crate = dict()
 
     # TODO(zbarsky): Would be nice to resolve for _ALL_PLATFORMS instead of per-triple, but it's complicated.
-    cfg_match_cache = {None: platform_triples}
+    cfg_match_cache = {None: struct(matches = platform_triples, uses_feature_cfg = False)}
 
     versions_by_name = dict()
     for package_index in range(len(packages)):
@@ -451,15 +463,13 @@ def _generate_hub_and_spokes(
             dep["feature_resolutions"] = feature_resolutions_by_fq_crate[dep_fq]
 
             target = dep.get("target")
-            match = cfg_match_cache.get(target)
-            if not match:
-                match = cfg_matches_expr_for_cfg_attrs(target, platform_cfg_attrs)
-
-                # TODO(zbarsky): Figure out how to do this optimization safely.
-                #if len(match) == len(platform_cfg_attrs):
-                #    match = match_all
-                cfg_match_cache[target] = match
-            dep["target"] = set(match)
+            match_info = _cfg_match_info_for_target(target, platform_cfg_attrs, cfg_match_cache)
+            if match_info.uses_feature_cfg:
+                dep["target_expr"] = target
+                dep["feature_sensitive"] = True
+                dep["target"] = set(platform_triples)
+            else:
+                dep["target"] = set(match_info.matches)
 
     _date(mctx, "set up resolutions")
 
@@ -521,16 +531,9 @@ def _generate_hub_and_spokes(
             versions.add(dep_fq)
 
             target = dep.get("target")
-            match = cfg_match_cache.get(target)
-            if not match:
-                match = cfg_matches_expr_for_cfg_attrs(target, platform_cfg_attrs)
-
-                # TODO(zbarsky): Figure out how to do this optimization safely.
-                #if len(match) == len(platform_cfg_attrs):
-                #    match = match_all
-                cfg_match_cache[target] = match
+            match_info = _cfg_match_info_for_target(target, platform_cfg_attrs, cfg_match_cache)
 
-            for triple in match:
+            for triple in match_info.matches:
                 workspace_dep_labels_by_triple[triple].add(":" + dep_name)
                 feature_resolutions.features_enabled[triple].update(features)
 
@@ -555,7 +558,7 @@ def _generate_hub_and_spokes(
 
     _date(mctx, "set up initial deps!")
 
-    resolve(mctx, packages, feature_resolutions_by_fq_crate, debug)
+    resolve(mctx, packages, feature_resolutions_by_fq_crate, platform_cfg_attrs_by_triple, debug)
 
     # Validate that we aren't trying to enable any `dep:foo` features that were not even in the lockfile.
     for package in packages:
@@ -839,14 +842,8 @@ RESOLVED_PLATFORMS = select({{
                 bazel_target = "//" + paths.join(workspace_package, _normalize_path(dep["path"]).removeprefix(repo_root + "/"))
 
             target = dep.get("target")
-            match = cfg_match_cache.get(target)
-            if not match:
-                match = cfg_matches_expr_for_cfg_attrs(target, platform_cfg_attrs)
-
-                # TODO(zbarsky): Figure out how to do this optimization safely.
-                #if len(match) == len(platform_cfg_attrs):
-                #    match = match_all
-                cfg_match_cache[target] = match
+            match_info = _cfg_match_info_for_target(target, platform_cfg_attrs, cfg_match_cache)
+            match = match_info.matches
 
             kind = dep["kind"]
             if kind == "dev":

rs/private/cfg_parser.bzl

@@ -71,6 +71,7 @@ def cfg_parse(expr):
     frames = [{"fn": "__ROOT__", "args": []}]
     pending_ident = None
     pending_eq_key = None
+    uses_feature_cfg = False
 
     for t in tokens:
         kind = t["t"]
@@ -90,7 +91,7 @@ def cfg_parse(expr):
             if not pending_eq_key:
                 fail("cfg parse error: string literal not expected here.")
             if pending_eq_key == "feature":
-                fail("Feature evaluation in cfg is unsupported!")
+                uses_feature_cfg = True
             frames[-1]["args"].append({
                 "kind": "eq",
                 "key": pending_eq_key,
@@ -138,7 +139,7 @@ def cfg_parse(expr):
             fail("cfg parse error: empty expression.")
         fail("cfg parse error: multiple top-level expressions; wrap with all(...)/any(...).")
 
-    return root_args[0]
+    return root_args[0], uses_feature_cfg
 
 ############################################
 # Triple → cfg attribute derivation
@@ -192,7 +193,18 @@ def _abi_from_env(env):
             return abi_piece
     return ""
 
-def triple_to_cfg_attrs(triple, features, target_features):
+def _target_has_feature(ctx, feature):
+    # x86_64 baseline implies SSE2.
+    if feature == "sse2":
+        return ctx["target_arch"] == "x86_64"
+
+    # AArch64 baseline implies NEON.
+    if feature == "neon":
+        return ctx["target_arch"] == "aarch64"
+
+    return False
+
+def triple_to_cfg_attrs(triple):
     parts = triple.split("-")
     arch_part = _get(parts, 0, "")
     vendor_part = _get(parts, 1, "unknown")
@@ -221,22 +233,17 @@ def triple_to_cfg_attrs(triple, features, target_features):
         "false": False,
         "unix": fam == "unix",
         "windows": fam == "windows",
-
-        # feature sets
-        #"__features__": dict(((f, True) for f in features)),
-        #"__tfeatures__": dict(((tf, True) for tf in target_features)),
     }
 
 ############################################
 # Evaluator (non-recursive; explicit stack)
 ############################################
 
-def _eval_eq(ctx, key, value):
+def _eval_eq(ctx, key, value, features):
     if key == "feature":
-        return ctx.get("__features__", {}).get(value, False)
+        return value in features
     if key == "target_feature":
-        return ctx.get("__tfeatures__",
-                       {}).get(value, False)
+        return _target_has_feature(ctx, value)
     known = [
         "target_os","target_family","target_arch","target_env",
         "target_vendor","target_endian","target_pointer_width","target_abi",
@@ -251,7 +258,7 @@ def _eval_pred(ctx, name):
     return ctx.get(name, False)
 
 
-def _cfg_eval(ast, ctx):
+def _cfg_eval(ast, ctx, features=[]):
     todo = [{"op": "VISIT", "node": ast}]
     results = []
     for _ in range(200000):
@@ -265,7 +272,7 @@ def _cfg_eval(ast, ctx):
             if kind == "pred":
                 results.append(_eval_pred(ctx, node["name"]))
             elif kind == "eq":
-                results.append(_eval_eq(ctx, node["key"], node["value"]))
+                results.append(_eval_eq(ctx, node["key"], node["value"], features))
             else:
                 children = node["args"]
                 n = len(children)
@@ -299,20 +306,28 @@ def _cfg_eval(ast, ctx):
         fail("cfg eval error: unexpected result stack size.")
     return results[0]
 
-def cfg_matches(expr, triple, features=[], target_features=[]):
-    ast = cfg_parse(expr)
-    ctx = triple_to_cfg_attrs(triple, features, target_features)
-    return _cfg_eval(ast, ctx)
+def cfg_matches(expr, triple, features=[]):
+    ast, _ = cfg_parse(expr)
+    ctx = triple_to_cfg_attrs(triple)
+    return _cfg_eval(ast, ctx, features)
 
-def cfg_matches_expr_for_triples(expr, triples, features=[], target_features=[]):
-    cfg_attrs = [triple_to_cfg_attrs(triple, features, target_features) for triple in triples]
-    return cfg_matches_expr_for_cfg_attrs(expr, cfg_attrs, features, target_features)
+def cfg_matches_expr_for_triples(expr, triples, features=[]):
+    cfg_attrs = [triple_to_cfg_attrs(triple) for triple in triples]
+    return cfg_matches_expr_for_cfg_attrs(expr, cfg_attrs, features)
 
-def cfg_matches_expr_for_cfg_attrs(expr, cfg_attrs, features=[], target_features=[]):
+def cfg_matches_expr_for_cfg_attrs(expr, cfg_attrs, features=[]):
     if expr.startswith("cfg("):
-        return cfg_matches_ast_for_triples(cfg_parse(expr), cfg_attrs)
-    # Cargo target table keys that aren't cfg(...) are literal triples.
-    return [cfg_attr["_triple"] for cfg_attr in cfg_attrs if cfg_attr["_triple"] == expr]
-
-def cfg_matches_ast_for_triples(ast, cfg_attrs):
-    return [cfg_attr["_triple"] for cfg_attr in cfg_attrs if _cfg_eval(ast, cfg_attr)]
+        ast, uses_feature_cfg = cfg_parse(expr)
+        return struct(
+            matches = cfg_matches_ast_for_triples(ast, cfg_attrs, features),
+            uses_feature_cfg = uses_feature_cfg,
+        )
+    else:
+        # Cargo target table keys that aren't cfg(...) are literal triples.
+        return struct(
+            matches = [cfg_attr["_triple"] for cfg_attr in cfg_attrs if cfg_attr["_triple"] == expr],
+            uses_feature_cfg = False,
+        )
+
+def cfg_matches_ast_for_triples(ast, cfg_attrs, features=[]):
+    return [cfg_attr["_triple"] for cfg_attr in cfg_attrs if _cfg_eval(ast, cfg_attr, features)]

rs/private/cfg_parser_test.bzl

@@ -1,5 +1,5 @@
 load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest")
-load(":cfg_parser.bzl", "cfg_matches", "cfg_matches_expr_for_triples")
+load(":cfg_parser.bzl", "cfg_matches", "cfg_matches_expr_for_triples", "triple_to_cfg_attrs", "cfg_matches_expr_for_cfg_attrs")
 
 def _cfg(expr):
     return "cfg(%s)" % expr
@@ -48,23 +48,51 @@ def _cfg_parser_smoke_test_impl(ctx):
     asserts.true(env, cfg_matches(_cfg("any(true, false)"), mac))
     asserts.true(env, cfg_matches(_cfg("all(true)"), mac))
     asserts.false(env, cfg_matches(_cfg("all(true, false)"), mac))
+    asserts.true(env, cfg_matches(_cfg('feature = "serde"'), mac, features = ["serde"]))
+    asserts.false(env, cfg_matches(_cfg('feature = "serde"'), mac))
+    asserts.true(env, cfg_matches(_cfg('target_feature = "sse2"'), linux_gnu))
+    asserts.false(env, cfg_matches(_cfg('target_feature = "sse2"'), mac))
 
     triples = [mac, linux_gnu, linux_musl, win, win_gnu, win_gnullvm]
 
     results = cfg_matches_expr_for_triples(_cfg('all(unix, any(target_env = "gnu", target_env = "musl"))'), triples)
-    asserts.equals(env, results, [linux_gnu, linux_musl])
+    asserts.equals(env, results.matches, [linux_gnu, linux_musl])
 
     results = cfg_matches_expr_for_triples(
         _cfg('any(target_arch = "aarch64", target_arch = "x86_64", target_arch = "x86")'),
         triples)
-    asserts.equals(env, results, triples)
+    asserts.equals(env, results.matches, triples)
 
     # Cargo dependencies can target a specific triple instead of a cfg expression.
     results = cfg_matches_expr_for_triples(win_gnullvm, triples)
-    asserts.equals(env, results, [win_gnullvm])
+    asserts.equals(env, results.matches, [win_gnullvm])
 
     results = cfg_matches_expr_for_triples(_cfg('all(target_os = "windows", any(target_env = "msvc", target_env = "gnu", target_env = "gnullvm"))'), triples)
-    asserts.equals(env, results, [win, win_gnu, win_gnullvm])
+    asserts.equals(env, results.matches, [win, win_gnu, win_gnullvm])
+
+    results = cfg_matches_expr_for_triples(_cfg('feature = "serde"'), triples, features = ["serde"])
+    asserts.equals(env, results.matches, triples)
+
+    results = cfg_matches_expr_for_triples(_cfg('feature = "serde"'), triples)
+    asserts.equals(env, results.matches, [])
+
+    info = cfg_matches_expr_for_cfg_attrs(
+        _cfg('all(feature = "serde", target_feature = "sse2")'),
+        [triple_to_cfg_attrs(linux_gnu)],
+    )
+    asserts.true(env, info.uses_feature_cfg)
+    asserts.equals(env, info.matches, [])
+
+    info = cfg_matches_expr_for_cfg_attrs(win_gnullvm, [triple_to_cfg_attrs(win_gnullvm)])
+    asserts.false(env, info.uses_feature_cfg)
+    asserts.equals(env, info.matches, [win_gnullvm])
+
+    info = cfg_matches_expr_for_cfg_attrs(
+        _cfg('target_feature = "sse2"'),
+        [triple_to_cfg_attrs(linux_gnu)],
+    )
+    asserts.false(env, info.uses_feature_cfg)
+    asserts.equals(env, info.matches, [linux_gnu])
 
     return unittest.end(env)
 

rs/private/resolver.bzl

@@ -1,3 +1,5 @@
+load("//rs/private:cfg_parser.bzl", "cfg_matches_expr_for_cfg_attrs")
+
 def _count(feature_resolutions_by_fq_crate):
     n = 0
     for feature_resolutions in feature_resolutions_by_fq_crate.values():
@@ -13,7 +15,22 @@ def _count(feature_resolutions_by_fq_crate):
         # No need to count aliases, they only get set when deps are set.
     return n
 
-def _resolve_one_round(packages, dirty_package_indices, debug):
+def _dep_target_matches_triple(dep, triple, package_feature_set, cfg_attrs_by_triple):
+    remaining = dep["target"]
+    if triple not in remaining:
+        return False
+
+    if not dep.get("feature_sensitive", False):
+        return True
+
+    cfg_attr = cfg_attrs_by_triple[triple]
+    return bool(cfg_matches_expr_for_cfg_attrs(
+        dep["target_expr"],
+        [cfg_attr],
+        features = package_feature_set,
+    ).matches)
+
+def _resolve_one_round(packages, dirty_package_indices, cfg_attrs_by_triple, debug):
     new_dirty_package_indices = set()
 
     for index in dirty_package_indices:
@@ -31,6 +48,7 @@ def _resolve_one_round(packages, dirty_package_indices, debug):
             package,
             features_enabled,
             feature_resolutions,
+            cfg_attrs_by_triple,
             debug,
         ):
             package_changed = True
@@ -50,9 +68,16 @@ def _resolve_one_round(packages, dirty_package_indices, debug):
             prefixed_dep_alias = "dep:" + dep_name
             optional = dep.get("optional", False)
 
-            to_remove = None
-            match = dep["target"]
+            if dep.get("feature_sensitive"):
+                match = set([
+                    triple
+                    for triple in dep["target"]
+                    if _dep_target_matches_triple(dep, triple, features_enabled[triple], cfg_attrs_by_triple)
+                ])
+            else:
+                match = dep["target"]
 
+            to_remove = None
             for triple in match:
                 if optional:
                     features_for_triple = features_enabled[triple]
@@ -96,6 +121,7 @@ def _propagate_feature_enablement(
         package,
         features_enabled,
         feature_resolutions,
+        cfg_attrs_by_triple,
         debug):
     possible_features = feature_resolutions.possible_features
 
@@ -128,7 +154,7 @@ def _propagate_feature_enablement(
 
                 found = False
                 for dep in feature_resolutions.possible_deps:
-                    if dep_name == dep["name"] and triple in dep["target"]:
+                    if dep_name == dep["name"] and _dep_target_matches_triple(dep, triple, feature_set, cfg_attrs_by_triple):
                         found = True
                         dep_optional = dep.get("optional", False)
                         if not optional_marker or not dep_optional or dep_name in feature_set or ("dep:" + dep_name) in feature_set:
@@ -151,13 +177,13 @@ def _propagate_feature_enablement(
 
 _MAX_ROUNDS = 50
 
-def resolve(mctx, packages, feature_resolutions_by_fq_crate, debug):
+def resolve(mctx, packages, feature_resolutions_by_fq_crate, cfg_attrs_by_triple, debug):
     # Do some rounds of mutual resolution; bail when no more changes
     dirty_package_indices = range(len(packages))
     for i in range(_MAX_ROUNDS):
         mctx.report_progress("Running round %s of dependency/feature resolution" % i)
 
-        dirty_package_indices = _resolve_one_round(packages, dirty_package_indices, debug)
+        dirty_package_indices = _resolve_one_round(packages, dirty_package_indices, cfg_attrs_by_triple, debug)
         if not dirty_package_indices:
             if debug:
                 count = _count(feature_resolutions_by_fq_crate)