Merge branch 'trunk' into toolchain-int-convert

.bazelrc

@@ -128,18 +128,106 @@ build --allow_unresolved_symlinks=false
 # RC file here if present.
 try-import %workspace%/user.bazelrc
 
-# TODO: WORKSPACE will be removed in bazel 9, should finish moving off.
+# Incompatible with `rules_tree_sitter`.
+# TODO: WORKSPACE will be removed in bazel 9, and we need to move off.
+# TODO: The registry has a different treesitter rule set, and we should
+# investigate switching. See:
+# https://registry.bazel.build/modules/tree-sitter-bazel
 common --enable_workspace
-
-# The version of treesitter rules we're using depends on empty globs.
-# TODO: Look at the different rules in the registry.
+# This is on by default in bazel 8.
 common --incompatible_disallow_empty_glob=false
+# common --incompatible_auto_exec_groups
+# common --incompatible_disable_starlark_host_transitions
+
+# This excludes things like rules_android to reduce warnings.
+# TODO: There's a pending fix, so hopefully we can remove it soon.
+# - Issue: https://github.com/bazelbuild/bazel/issues/23929
+# - 8.0.1 tracker: https://github.com/bazelbuild/bazel/issues/24649
+common --incompatible_autoload_externally=+@rules_java,+@rules_python,+@rules_shell
+
+# Query error in `@bazel_tools`. This reproduces with
+# `bazel query 'deps(//...)'`.
+# TODO: Enable the flag once compatibility issues are fixed.
+# common --incompatible_disable_non_executable_java_binary
+
+# Incompatible with `rules_cc`.
+# TODO: Enable the flag once compatibility issues are fixed.
+# common --incompatible_no_rule_outputs_param
+# common --incompatible_stop_exporting_language_modules
 
-# TODO: Enable as many incompatible flags as we can, per
+# Incompatible with `rules_pkg`.
+# TODO: Enable the flag once compatibility issues are fixed.
+# common --incompatible_disable_target_default_provider_fields
+
+# Incompatible with `rules_shell`.
+# TODO: Enable the flag once compatibility issues are fixed.
+# common --incompatible_check_visibility_for_toolchains
+
+# Incompatible with `tcmalloc`.
+# TODO: Fix `bazel build -c opt //...`.
+# common --incompatible_config_setting_private_default_visibility
+
+# Enable as many incompatible flags as we can, per
 # https://bazel.build/release/backward-compatibility. To get the latest list,
 # using `bazelisk --migrate build //...` will help.
+common --incompatible_allow_tags_propagation
+common --incompatible_always_check_depset_elements
+common --incompatible_always_include_files_in_data
+common --incompatible_bazel_test_exec_run_under
+common --incompatible_check_sharding_support
+common --incompatible_check_testonly_for_output_files
+common --incompatible_default_to_explicit_init_py
+common --incompatible_depset_for_java_output_source_jars
+common --incompatible_depset_for_libraries_to_link_getter
+common --incompatible_disable_native_android_rules
+common --incompatible_disable_native_repo_rules
+common --incompatible_disable_objc_library_transition
+common --incompatible_disable_target_provider_fields
+common --incompatible_disallow_ctx_resolve_tools
+common --incompatible_disallow_legacy_py_provider
+common --incompatible_disallow_sdk_frameworks_attributes
+common --incompatible_disallow_struct_provider_syntax
+common --incompatible_do_not_split_linking_cmdline
+common --incompatible_dont_enable_host_nonhost_crosstool_features
+common --incompatible_dont_use_javasourceinfoprovider
+common --incompatible_enable_apple_toolchain_resolution
+common --incompatible_enable_deprecated_label_apis
+common --incompatible_enable_proto_toolchain_resolution
+common --incompatible_enforce_config_setting_visibility
+common --incompatible_exclusive_test_sandboxed
+common --incompatible_fail_on_unknown_attributes
+common --incompatible_fix_package_group_reporoot_syntax
+common --incompatible_java_common_parameters
+common --incompatible_legacy_local_fallback
+common --incompatible_make_thinlto_command_lines_standalone
+common --incompatible_merge_fixed_and_default_shell_env
+common --incompatible_merge_genfiles_directory
+common --incompatible_modify_execution_info_additive
+common --incompatible_new_actions_api
+common --incompatible_no_attr_license
+common --incompatible_no_implicit_file_export
+common --incompatible_no_implicit_watch_label
+common --incompatible_objc_alwayslink_by_default
+common --incompatible_package_group_has_public_syntax
+common --incompatible_py2_outputs_are_suffixed
+common --incompatible_py3_is_default
+common --incompatible_python_disable_py2
+common --incompatible_python_disallow_native_rules
+common --incompatible_remote_use_new_exit_code_for_lost_inputs
+common --incompatible_remove_legacy_whole_archive
+common --incompatible_require_ctx_in_configure_features
+common --incompatible_require_linker_input_cc_api
+common --incompatible_run_shell_command_string
+common --incompatible_sandbox_hermetic_tmp
+common --incompatible_simplify_unconditional_selects_in_rule_attrs
+common --incompatible_stop_exporting_build_file_path
 common --incompatible_strict_action_env
-
-# This excludes things like rules_android to reduce warnings.
-# TODO: Disable this completely.
-common --incompatible_autoload_externally=+@rules_java,+@rules_python,+@rules_shell
+common --incompatible_strip_executable_safely
+common --incompatible_top_level_aspects_require_providers
+common --incompatible_unambiguous_label_stringification
+common --incompatible_use_cc_configure_from_rules_cc
+common --incompatible_use_new_cgroup_implementation
+common --incompatible_use_plus_in_repo_names
+common --incompatible_use_python_toolchains
+common --incompatible_validate_top_level_header_inclusions
+common --incompatible_visibility_private_attributes_at_definition

.github/workflows/clang_tidy.yaml

@@ -20,7 +20,7 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  test:
+  clang-tidy:
     runs-on: ubuntu-22.04
 
     steps:

bazel/carbon_rules/BUILD

@@ -4,6 +4,8 @@
 
 load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")
 
+package(default_visibility = ["//visibility:public"])
+
 # Flag controlling whether the target config is used for the `carbon_*` Bazel
 # rules. The default is to use the exec config as that is more correct in cases
 # where the target config is not compatible with the exec (cross compiling), and

bazel/cc_toolchains/BUILD

@@ -4,6 +4,8 @@
 
 load("@bazel_skylib//lib:selects.bzl", "selects")
 
+package(default_visibility = ["//visibility:public"])
+
 # For use by rules.bzl.
 # Matches when asan is enabled on a macOS platform.
 selects.config_setting_group(

bazel/cc_toolchains/clang_toolchain.BUILD

@@ -7,6 +7,8 @@
 
 load(":cc_toolchain_config.bzl", "cc_local_toolchain_suite")
 
+package(default_visibility = ["//visibility:public"])
+
 cc_local_toolchain_suite(
     name = "bazel_cc_toolchain",
     configs = [

bazel/manifest/defs.bzl

@@ -9,8 +9,11 @@ def _manifest(ctx):
 
     files = []
     for src in ctx.attr.srcs:
-        files.extend([f.path for f in src.files.to_list()])
-        files.extend([f.path for f in src.default_runfiles.files.to_list()])
+        files.extend([f.path for f in src[DefaultInfo].files.to_list()])
+        files.extend([
+            f.path
+            for f in src[DefaultInfo].default_runfiles.files.to_list()
+        ])
 
     if ctx.attr.strip_package_dir:
         package_dir = ctx.label.package + "/"

toolchain/check/handle_impl.cpp

@@ -212,21 +212,45 @@ static auto PopImplIntroducerAndParamsAsNameComponent(
 
   Parse::NodeId first_param_node_id =
       context.node_stack().PopForSoloNodeId<Parse::NodeKind::ImplIntroducer>();
+
   // Subtracting 1 since we don't want to include the final `{` or `;` of the
   // declaration when performing syntactic match.
-  // TODO: Following proposal #3763, we should exclude any `where` clause, and
-  // add `Self` before `as` if needed, see:
+  auto end_node_kind = context.parse_tree().node_kind(end_of_decl_node_id);
+  CARBON_CHECK(end_node_kind == Parse::NodeKind::ImplDefinitionStart ||
+               end_node_kind == Parse::NodeKind::ImplDecl);
+  Parse::Tree::PostorderIterator last_param_iter(end_of_decl_node_id);
+  --last_param_iter;
+
+  // Following proposal #3763, exclude a final `where` clause, if present. See:
   // https://github.com/carbon-language/carbon-lang/blob/trunk/proposals/p3763.md#redeclarations
-  auto node_kind = context.parse_tree().node_kind(end_of_decl_node_id);
-  CARBON_CHECK(node_kind == Parse::NodeKind::ImplDefinitionStart ||
-               node_kind == Parse::NodeKind::ImplDecl);
-  Parse::NodeId last_param_node_id(end_of_decl_node_id.index - 1);
+
+  // Caches the NodeKind for the current value of *last_param_iter so
+  if (context.parse_tree().node_kind(*last_param_iter) ==
+      Parse::NodeKind::WhereExpr) {
+    int where_operands_to_skip = 1;
+    --last_param_iter;
+    CARBON_CHECK(Parse::Tree::PostorderIterator(first_param_node_id) <
+                 last_param_iter);
+    do {
+      auto node_kind = context.parse_tree().node_kind(*last_param_iter);
+      if (node_kind == Parse::NodeKind::WhereExpr) {
+        // If we have a nested `where`, we need to see another `WhereOperand`
+        // before we find the one that matches our original `WhereExpr` node.
+        ++where_operands_to_skip;
+      } else if (node_kind == Parse::NodeKind::WhereOperand) {
+        --where_operands_to_skip;
+      }
+      --last_param_iter;
+      CARBON_CHECK(Parse::Tree::PostorderIterator(first_param_node_id) <
+                   last_param_iter);
+    } while (where_operands_to_skip > 0);
+  }
 
   return {
       .name_loc_id = Parse::NodeId::Invalid,
       .name_id = SemIR::NameId::Invalid,
       .first_param_node_id = first_param_node_id,
-      .last_param_node_id = last_param_node_id,
+      .last_param_node_id = *last_param_iter,
       .implicit_params_loc_id = implicit_params_loc_id,
       .implicit_param_patterns_id =
           implicit_param_patterns_id.value_or(SemIR::InstBlockId::Invalid),
@@ -297,6 +321,11 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId node_id,
   // TODO: Check that its constant value is a constraint.
   auto [constraint_inst_id, constraint_type_id] =
       ExprAsType(context, constraint_node, constraint_id);
+  // TODO: Do facet type resolution here.
+  // TODO: Determine `interface_id` and `specific_id` once and save it in the
+  // resolved facet type, instead of in multiple functions called below.
+  // TODO: Skip work below if facet type resolution fails, so we don't have a
+  // valid/non-error `interface_id` at all.
 
   // Process modifiers.
   // TODO: Should we somehow permit access specifiers on `impl`s?

toolchain/check/merge.cpp

@@ -394,18 +394,38 @@ static auto CheckRedeclParamSyntax(Context& context,
   CARBON_CHECK(prev_last_param_node_id.is_valid(),
                "prev_last_param_node_id.is_valid should match "
                "prev_first_param_node_id.is_valid");
-
-  auto new_range = Parse::Tree::PostorderIterator::MakeRange(
-      new_first_param_node_id, new_last_param_node_id);
-  auto prev_range = Parse::Tree::PostorderIterator::MakeRange(
-      prev_first_param_node_id, prev_last_param_node_id);
-
-  // zip is using the shortest range. If they differ in length, there should be
-  // some difference inside the range because the range includes parameter
-  // brackets. As a consequence, we don't explicitly handle different range
-  // sizes here.
-  for (auto [new_node_id, prev_node_id] : llvm::zip(new_range, prev_range)) {
+  Parse::Tree::PostorderIterator new_iter(new_first_param_node_id);
+  Parse::Tree::PostorderIterator new_end(new_last_param_node_id);
+  Parse::Tree::PostorderIterator prev_iter(prev_first_param_node_id);
+  Parse::Tree::PostorderIterator prev_end(prev_last_param_node_id);
+  // Done when one past the last node to check.
+  ++new_end;
+  ++prev_end;
+
+  // Compare up to the shortest length.
+  for (; new_iter != new_end && prev_iter != prev_end;
+       ++new_iter, ++prev_iter) {
+    auto new_node_id = *new_iter;
+    auto prev_node_id = *prev_iter;
     if (!IsNodeSyntaxEqual(context, new_node_id, prev_node_id)) {
+      // Skip difference if it is `Self as` vs. `as` in an `impl` declaration.
+      // https://github.com/carbon-language/carbon-lang/blob/trunk/proposals/p3763.md#redeclarations
+      auto new_node_kind = context.parse_tree().node_kind(new_node_id);
+      auto prev_node_kind = context.parse_tree().node_kind(prev_node_id);
+      if (new_node_kind == Parse::NodeKind::DefaultSelfImplAs &&
+          prev_node_kind == Parse::NodeKind::SelfTypeNameExpr &&
+          context.parse_tree().node_kind(prev_iter[1]) ==
+              Parse::NodeKind::TypeImplAs) {
+        ++prev_iter;
+        continue;
+      }
+      if (prev_node_kind == Parse::NodeKind::DefaultSelfImplAs &&
+          new_node_kind == Parse::NodeKind::SelfTypeNameExpr &&
+          context.parse_tree().node_kind(new_iter[1]) ==
+              Parse::NodeKind::TypeImplAs) {
+        ++new_iter;
+        continue;
+      }
       if (!diagnose) {
         return false;
       }
@@ -420,6 +440,17 @@ static auto CheckRedeclParamSyntax(Context& context,
       return false;
     }
   }
+  // The prefixes are the same, but the lengths may still be different. This is
+  // only relevant for `impl` declarations where the final bracketing node is
+  // not included in the range of nodes being compared, and in those cases
+  // `diagnose` is false.
+  if (new_iter != new_end) {
+    CARBON_CHECK(!diagnose);
+    return false;
+  } else if (prev_iter != prev_end) {
+    CARBON_CHECK(!diagnose);
+    return false;
+  }
 
   return true;
 }