diff --git a/.bazelrc b/.bazelrc index 22d3f12c7b941..30d6309cf63fc 100644 --- a/.bazelrc +++ b/.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 diff --git a/.github/workflows/clang_tidy.yaml b/.github/workflows/clang_tidy.yaml index 760ce84b28a84..4a5673303d81b 100644 --- a/.github/workflows/clang_tidy.yaml +++ b/.github/workflows/clang_tidy.yaml @@ -20,7 +20,7 @@ concurrency: cancel-in-progress: true jobs: - test: + clang-tidy: runs-on: ubuntu-22.04 steps: diff --git a/bazel/carbon_rules/BUILD b/bazel/carbon_rules/BUILD index 7aca964de584a..1250c691225e3 100644 --- a/bazel/carbon_rules/BUILD +++ b/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 diff --git a/bazel/cc_toolchains/BUILD b/bazel/cc_toolchains/BUILD index 6cf693cca0292..2059ee1fb7ba7 100644 --- a/bazel/cc_toolchains/BUILD +++ b/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( diff --git a/bazel/cc_toolchains/clang_toolchain.BUILD b/bazel/cc_toolchains/clang_toolchain.BUILD index 0da0ac8935649..13bf032931f31 100644 --- a/bazel/cc_toolchains/clang_toolchain.BUILD +++ b/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 = [ diff --git a/bazel/manifest/defs.bzl b/bazel/manifest/defs.bzl index 9e9bbd65006f0..926ec1f514c3d 100644 --- a/bazel/manifest/defs.bzl +++ b/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 + "/" diff --git a/toolchain/check/handle_impl.cpp b/toolchain/check/handle_impl.cpp index 808aa89d44d4e..30367ceaa7e09 100644 --- a/toolchain/check/handle_impl.cpp +++ b/toolchain/check/handle_impl.cpp @@ -212,21 +212,45 @@ static auto PopImplIntroducerAndParamsAsNameComponent( Parse::NodeId first_param_node_id = context.node_stack().PopForSoloNodeId(); + // 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? diff --git a/toolchain/check/merge.cpp b/toolchain/check/merge.cpp index b19eb0cc87ddd..20340a93cb07d 100644 --- a/toolchain/check/merge.cpp +++ b/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; } diff --git a/toolchain/check/testdata/impl/no_prelude/impl_self_as.carbon b/toolchain/check/testdata/impl/no_prelude/impl_self_as.carbon new file mode 100644 index 0000000000000..cdb16c10026be --- /dev/null +++ b/toolchain/check/testdata/impl/no_prelude/impl_self_as.carbon @@ -0,0 +1,180 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// EXTRA-ARGS: --no-dump-sem-ir +// +// AUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/impl/no_prelude/impl_self_as.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/impl/no_prelude/impl_self_as.carbon + +// --- match.carbon +library "[[@TEST_NAME]]"; + +interface I1 {} +interface I2 {} +interface J1(T1:! type) {} +interface J2(T2:! type) {} + +// `impl Self as` should match `impl as`, so these should not trigger impl +// declaration without definition diagnostics. + +class C1 { + impl Self as I1; + impl as I1 {} + + impl as I2; + impl Self as I2 {} + + impl forall [U:! type] Self as J1(U); + impl forall [U:! type] as J1(U) {} + + impl forall [V:! type] as J2(V); + impl forall [V:! type] Self as J2(V) {} +} + +class C2(W:! type) { + impl Self as I1; + impl as I1 {} + + impl as I2; + impl Self as I2 {} + + impl forall [X:! type] Self as J1(X); + impl forall [X:! type] as J1(X) {} + + impl forall [Y:! type] as J2(Y); + impl forall [Y:! type] Self as J2(Y) {} +} + + +// --- fail_no_match.carbon +library "[[@TEST_NAME]]"; + +interface I3 {} +interface I4 {} +interface I5 {} +interface I6 {} +interface J3(T3:! type) {} +interface J4(T4:! type) {} +interface J5(T5:! type) {} +interface J6(T6:! type) {} + +// `impl C as` should not match `impl Self as` or `impl as`. + +class C3 { + // CHECK:STDERR: fail_no_match.carbon:[[@LINE+4]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: impl C3 as I3; + // CHECK:STDERR: ^~~~~~~~~~~~~~ + // CHECK:STDERR: + impl C3 as I3; + impl as I3 {} + + // CHECK:STDERR: fail_no_match.carbon:[[@LINE+4]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: impl C3 as I4; + // CHECK:STDERR: ^~~~~~~~~~~~~~ + // CHECK:STDERR: + impl C3 as I4; + impl Self as I4 {} + + // CHECK:STDERR: fail_no_match.carbon:[[@LINE+4]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: impl as I5; + // CHECK:STDERR: ^~~~~~~~~~~ + // CHECK:STDERR: + impl as I5; + impl C3 as I5 {} + + // CHECK:STDERR: fail_no_match.carbon:[[@LINE+4]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: impl Self as I6; + // CHECK:STDERR: ^~~~~~~~~~~~~~~~ + // CHECK:STDERR: + impl Self as I6; + impl C3 as I6 {} + + // CHECK:STDERR: fail_no_match.carbon:[[@LINE+4]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: impl forall [Z3:! type] C3 as J3(Z3); + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: + impl forall [Z3:! type] C3 as J3(Z3); + impl forall [Z3:! type] as J3(Z3) {} + + // CHECK:STDERR: fail_no_match.carbon:[[@LINE+4]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: impl forall [Z4:! type] C3 as J4(Z4); + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: + impl forall [Z4:! type] C3 as J4(Z4); + impl forall [Z4:! type] Self as J4(Z4) {} + + // CHECK:STDERR: fail_no_match.carbon:[[@LINE+4]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: impl forall [Z5:! type] as J5(Z5); + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: + impl forall [Z5:! type] as J5(Z5); + impl forall [Z5:! type] C3 as J5(Z5) {} + + // CHECK:STDERR: fail_no_match.carbon:[[@LINE+4]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: impl forall [Z6:! type] Self as J6(Z6); + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: + impl forall [Z6:! type] Self as J6(Z6); + impl forall [Z6:! type] C3 as J6(Z6) {} +} + +class C4(A:! type) { + // CHECK:STDERR: fail_no_match.carbon:[[@LINE+4]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: impl C4(A) as I3; + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~ + // CHECK:STDERR: + impl C4(A) as I3; + impl as I3 {} + + // CHECK:STDERR: fail_no_match.carbon:[[@LINE+4]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: impl C4(A) as I4; + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~ + // CHECK:STDERR: + impl C4(A) as I4; + impl Self as I4 {} + + // CHECK:STDERR: fail_no_match.carbon:[[@LINE+4]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: impl as I5; + // CHECK:STDERR: ^~~~~~~~~~~ + // CHECK:STDERR: + impl as I5; + impl C4(A) as I5 {} + + // CHECK:STDERR: fail_no_match.carbon:[[@LINE+4]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: impl Self as I6; + // CHECK:STDERR: ^~~~~~~~~~~~~~~~ + // CHECK:STDERR: + impl Self as I6; + impl C4(A) as I6 {} + + // CHECK:STDERR: fail_no_match.carbon:[[@LINE+4]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: impl forall [B3:! type] C4(A) as J3(B3); + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: + impl forall [B3:! type] C4(A) as J3(B3); + impl forall [B3:! type] as J3(B3) {} + + // CHECK:STDERR: fail_no_match.carbon:[[@LINE+4]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: impl forall [B4:! type] C4(A) as J4(B4); + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: + impl forall [B4:! type] C4(A) as J4(B4); + impl forall [B4:! type] Self as J4(B4) {} + + // CHECK:STDERR: fail_no_match.carbon:[[@LINE+4]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: impl forall [B5:! type] as J5(B5); + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: + impl forall [B5:! type] as J5(B5); + impl forall [B5:! type] C4(A) as J5(B5) {} + + // CHECK:STDERR: fail_no_match.carbon:[[@LINE+3]]:3: error: impl declared but not defined [MissingImplDefinition] + // CHECK:STDERR: impl forall [B6:! type] Self as J6(B6); + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + impl forall [B6:! type] Self as J6(B6); + impl forall [B6:! type] C4(A) as J6(B6) {} +} diff --git a/toolchain/check/testdata/impl/no_prelude/impl_where_redecl.carbon b/toolchain/check/testdata/impl/no_prelude/impl_where_redecl.carbon new file mode 100644 index 0000000000000..c59c393f64714 --- /dev/null +++ b/toolchain/check/testdata/impl/no_prelude/impl_where_redecl.carbon @@ -0,0 +1,47 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// EXTRA-ARGS: --no-dump-sem-ir +// +// AUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/impl/no_prelude/impl_where_redecl.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/impl/no_prelude/impl_where_redecl.carbon + +// --- match.carbon +library "[[@TEST_NAME]]"; + +interface J {} + +// `impl` matching ignores the `where` clause. It should not get confused by +// nested `where`, so the following should not trigger impl declaration without +// definition diagnostics. + +impl () as J; +impl () as J where .Self impls type and .Self impls (type where .Self impls type) {} + +impl {} as J where .Self impls type and .Self impls (type where .Self impls type); +impl {} as J {} + +// --- parens_other_nesting.carbon +library "[[@TEST_NAME]]"; + +interface K {} + +// `impl` matching only ignores a root-level `where` clause. + +impl {} as (K where .Self impls type) where .Self impls type; +impl {} as (K where .Self impls type) {} + +// --- fail_other_nesting.carbon +library "[[@TEST_NAME]]"; + +interface L {} + +// CHECK:STDERR: fail_other_nesting.carbon:[[@LINE+3]]:1: error: impl declared but not defined [MissingImplDefinition] +// CHECK:STDERR: impl () as (L where .Self impls type) where .Self impls type; +// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +impl () as (L where .Self impls type) where .Self impls type; +impl () as L {} diff --git a/toolchain/driver/compile_subcommand.cpp b/toolchain/driver/compile_subcommand.cpp index 175a0c6313063..f0cb9e47d1e85 100644 --- a/toolchain/driver/compile_subcommand.cpp +++ b/toolchain/driver/compile_subcommand.cpp @@ -369,14 +369,17 @@ class CompilationUnit { source_ = SourceBuffer::MakeFromFileOrStdin(*driver_env_->fs, input_filename_, *consumer_); }); - if (mem_usage_) { - mem_usage_->Add("source_", source_->text().size(), - source_->text().size()); - } + if (!source_) { success_ = false; return; } + + if (mem_usage_) { + mem_usage_->Add("source_", source_->text().size(), + source_->text().size()); + } + CARBON_VLOG("*** SourceBuffer ***\n```\n{0}\n```\n", source_->text()); LogCall("Lex::Lex", "lex", diff --git a/toolchain/driver/driver_test.cpp b/toolchain/driver/driver_test.cpp index 3e6fd0d77944e..3c8112d9b4589 100644 --- a/toolchain/driver/driver_test.cpp +++ b/toolchain/driver/driver_test.cpp @@ -122,6 +122,17 @@ TEST_F(DriverTest, CompileCommandErrors) { StrEq("error: not all required positional arguments were provided; first " "missing and required positional argument: `FILE`\n")); + // Pass non-existing file + EXPECT_FALSE(driver_ + .RunCommand({"compile", "--dump-mem-usage", + "non-existing-file.carbon"}) + .success); + EXPECT_THAT( + test_error_stream_.TakeStr(), + ContainsRegex("No such file or directory[\\n]*non-existing-file.carbon")); + // Flush output stream, because it's ok that it's not empty here. + test_output_stream_.TakeStr(); + // Invalid output filename. No reliably error message here. // TODO: Likely want a different filename on Windows. auto empty_file = MakeTestFile(""); diff --git a/toolchain/sem_ir/impl.cpp b/toolchain/sem_ir/impl.cpp index 9bb5cb0b47632..89d369cef7b89 100644 --- a/toolchain/sem_ir/impl.cpp +++ b/toolchain/sem_ir/impl.cpp @@ -10,11 +10,22 @@ namespace Carbon::SemIR { auto ImplStore::GetOrAddLookupBucket(const Impl& impl) -> LookupBucketRef { auto self_id = sem_ir_.constant_values().GetConstantInstId(impl.self_id); - auto constraint_id = - sem_ir_.constant_values().GetConstantInstId(impl.constraint_id); + InterfaceId interface_id = InterfaceId::Invalid; + SpecificId specific_id = SpecificId::Invalid; + auto facet_type_id = TypeId::ForTypeConstant( + sem_ir_.constant_values().Get(impl.constraint_id)); + if (auto facet_type = + sem_ir_.types().TryGetAs(facet_type_id)) { + const SemIR::FacetTypeInfo& facet_type_info = + sem_ir_.facet_types().Get(facet_type->facet_type_id); + if (auto interface_type = facet_type_info.TryAsSingleInterface()) { + interface_id = interface_type->interface_id; + specific_id = interface_type->specific_id; + } + } return LookupBucketRef( *this, lookup_ - .Insert(std::pair{self_id, constraint_id}, + .Insert(std::tuple{self_id, interface_id, specific_id}, [] { return ImplOrLookupBucketId::Invalid; }) .value()); } diff --git a/toolchain/sem_ir/impl.h b/toolchain/sem_ir/impl.h index 3dbd2973d5dd0..a7769477e7e96 100644 --- a/toolchain/sem_ir/impl.h +++ b/toolchain/sem_ir/impl.h @@ -189,7 +189,8 @@ class ImplStore { private: File& sem_ir_; ValueStore values_; - Map, ImplOrLookupBucketId> lookup_; + Map, ImplOrLookupBucketId> + lookup_; // Buckets with at least 2 entries, which will be rare; see LookupBucketRef. llvm::SmallVector> lookup_buckets_; }; diff --git a/toolchain/sem_ir/inst_fingerprinter.cpp b/toolchain/sem_ir/inst_fingerprinter.cpp index 8140f7dbbc1d5..0ca3c5555a1a4 100644 --- a/toolchain/sem_ir/inst_fingerprinter.cpp +++ b/toolchain/sem_ir/inst_fingerprinter.cpp @@ -233,7 +233,7 @@ struct Worklist { } auto Add(ImportIRId ir_id) -> void { - auto* ir = sem_ir->import_irs().Get(ir_id).sem_ir; + const auto* ir = sem_ir->import_irs().Get(ir_id).sem_ir; Add(ir->package_id()); Add(ir->library_id()); } @@ -267,7 +267,7 @@ struct Worklist { using Kind = decltype(kind); // Build a lookup table to add an argument of the given kind. - static constexpr std::array table = [] { + static constexpr std::array Table = [] { std::array table; table[Kind::None.ToIndex()] = [](Worklist& /*worklist*/, uint64_t /*arg*/) {}; @@ -283,7 +283,7 @@ struct Worklist { return table; }(); - table[kind.ToIndex()](*this, arg); + Table[kind.ToIndex()](*this, arg); } // Ensure all the instructions on the todo list have fingerprints. diff --git a/toolchain/sem_ir/inst_namer.cpp b/toolchain/sem_ir/inst_namer.cpp index 73de9b7e07008..86bea57299bb3 100644 --- a/toolchain/sem_ir/inst_namer.cpp +++ b/toolchain/sem_ir/inst_namer.cpp @@ -249,7 +249,7 @@ auto InstNamer::Namespace::AllocateName( // Append location information to try to disambiguate. // TODO: Consider handling inst_id cases. - if (LocId* loc_id = std::get_if(&loc_id_or_fingerprint)) { + if (auto* loc_id = std::get_if(&loc_id_or_fingerprint)) { if (loc_id->is_node_id()) { const auto& tree = inst_namer.sem_ir_->parse_tree(); auto token = tree.node_token(loc_id->node_id());