Merge branch 'trunk' into toolchain-remove-int32

.clang-tidy

@@ -48,6 +48,9 @@ WarningsAsErrors: '*'
 #   explicit value.
 # - `readability-function-cognitive-complexity` warns too frequently.
 # - `readability-magic-numbers` warns in reasonably documented situations.
+# - `readability-redundant-member-init` warns on `= {}` which is also used to
+#   indicate which fields do not need to be explicitly initialized in aggregate
+#   initialization.
 # - `readability-suspicious-call-argument` warns when callers use similar names
 #   as different parameters.
 #
@@ -80,7 +83,7 @@ Checks:
   -readability-else-after-return, -readability-enum-initial-value,
   -readability-function-cognitive-complexity, -readability-identifier-length,
   -readability-implicit-bool-conversion, -readability-magic-numbers,
-  -readability-make-member-function-const,
+  -readability-make-member-function-const, -readability-redundant-member-init,
   -readability-static-definition-in-anonymous-namespace,
   -readability-suspicious-call-argument, -readability-use-anyofallof
 CheckOptions:

.pre-commit-config.yaml

@@ -12,7 +12,7 @@ default_language_version:
 
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: 2c9f875913ee60ca25ce70243dc24d5b6415598c # frozen: v4.6.0
+    rev: cef0300fd0fc4d2a87a85fa2093c6b283ea36f4b # frozen: v5.0.0
     hooks:
       - id: check-added-large-files
       - id: check-case-conflict
@@ -45,7 +45,7 @@ repos:
 
   # Formatters should be run late so that they can re-format any prior changes.
   - repo: https://github.com/psf/black
-    rev: 3702ba224ecffbcec30af640c149f231d90aebdb # frozen: 24.4.2
+    rev: 1b2427a2b785cc4aac97c19bb4b9a0de063f9547 # frozen: 24.10.0
     hooks:
       - id: black
   - repo: https://github.com/pre-commit/mirrors-prettier
@@ -131,11 +131,11 @@ repos:
         files: ^.*/BUILD$
         pass_filenames: false
   - repo: https://github.com/PyCQA/flake8
-    rev: 7d37d9032d0d161634be4554273c30efd4dea0b3 # frozen: 7.0.0
+    rev: e43806be3607110919eff72939fda031776e885a # frozen: 7.1.1
     hooks:
       - id: flake8
   - repo: https://github.com/pre-commit/mirrors-mypy
-    rev: 'e5ea6670624c24f8321f6328ef3176dbba76db46' # frozen: v1.10.0
+    rev: 'f56614daa94d5cd733d3b7004c5df9caad267b4a' # frozen: v1.13.0
     hooks:
       - id: mypy
         # Use setup.cfg to match the command line.
@@ -157,7 +157,7 @@ repos:
               .*_test\.py
           )$
   - repo: https://github.com/codespell-project/codespell
-    rev: 6e41aba91fb32e9feb741a6258eefeb9c6e4a482 # frozen: v2.2.6
+    rev: 193cd7d27cd571f79358af09a8fb8997e54f8fff # frozen: v2.3.0
     hooks:
       - id: codespell
         args: ['-I', '.codespell_ignore', '--uri-ignore-words-list', '*']

common/raw_hashtable_metadata_group.h

@@ -477,7 +477,7 @@ class MetadataGroup : public Printable<MetadataGroup> {
       -> bool;
 
   // SIMD implementations of each operation. We minimize platform-specific APIs
-  // to reduce the scope of errors that can only be discoverd building on one
+  // to reduce the scope of errors that can only be discovered building on one
   // platform, so the bodies of these contain the platform specific code. Their
   // behavior and semantics exactly match the documentation for the un-prefixed
   // functions.

docs/design/code_and_name_organization/README.md

@@ -33,7 +33,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
         -   [Imports from the current package](#imports-from-the-current-package)
         -   [Exporting imported names](#exporting-imported-names)
     -   [Namespaces](#namespaces)
-        -   [Re-declaring imported namespaces](#re-declaring-imported-namespaces)
+        -   [Redeclaring imported namespaces](#redeclaring-imported-namespaces)
         -   [Declaring namespace members](#declaring-namespace-members)
         -   [Aliasing](#aliasing)
 -   [Caveats](#caveats)
@@ -684,7 +684,7 @@ the file's namespace. In the above example, after declaring
 `namespace Timezones.Internal;`, `Timezones` is available as an identifier and
 `Internal` is reached through `Timezones`.
 
-#### Re-declaring imported namespaces
+#### Redeclaring imported namespaces
 
 Namespaces may exist in imported package entities, in addition to being declared
 in the current file. However, even if the namespace already exists in an

docs/design/generics/details.md

@@ -1852,7 +1852,7 @@ class IntWrapper {
       return left.x - right.x;
     }
   }
-  impl as Comparable = ComparableFromDifferenceFn(IntWrapper);
+  impl as Comparable = ComparableFromDifference(IntWrapper);
 }
 ```
 
@@ -2164,7 +2164,7 @@ specifying the values of associated facets, as in:
 ```carbon
 impl VeryLongTypeName as Add
     // `Self` here means `VeryLongTypeName`
-    where .Result == Self {
+    where .Result = Self {
   ...
 }
 ```

docs/project/contribution_tools.md

@@ -110,7 +110,7 @@ echo "build --repo_env=CC=$(readlink -f $(which clang-16))" >> user.bazelrc
 ### macOS
 
 ```shell
-# Install Hombrew.
+# Install Homebrew.
 /bin/bash -c "$(curl -fsSL \
   https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)"
 

docs/project/pull_request_workflow.md

@@ -122,8 +122,8 @@ and any previously queued merge. Checks run on this commit help to ensure that
 the `trunk` stays "green" in presence of conflicting merges.
 
 After the merge queue checks pass, the `trunk` branch pointer is updated to
-include the now merged changset. The resulting commit title and description will
-match exactly those of the pull request, so it is important to set those
+include the now merged changeset. The resulting commit title and description
+will match exactly those of the pull request, so it is important to set those
 appropriately before merging. Co-authors are preserved by this operation. If a
 failure happens at any point, the merge fails, with both `trunk` and the pull
 request branch kept in their original state.

explorer/file_test.cpp

@@ -28,8 +28,9 @@ class ExplorerFileTest : public FileTestBase {
   }
 
   auto Run(const llvm::SmallVector<llvm::StringRef>& test_args,
-           llvm::vfs::InMemoryFileSystem& fs, llvm::raw_pwrite_stream& stdout,
-           llvm::raw_pwrite_stream& stderr) -> ErrorOr<RunResult> override {
+           llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem>& fs,
+           llvm::raw_pwrite_stream& stdout, llvm::raw_pwrite_stream& stderr)
+      -> ErrorOr<RunResult> override {
     // Add the prelude.
     llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> prelude =
         llvm::MemoryBuffer::getFile("explorer/data/prelude.carbon");
@@ -41,7 +42,8 @@ class ExplorerFileTest : public FileTestBase {
     // here.
     static constexpr llvm::StringLiteral PreludePath =
         "/explorer/data/prelude.carbon";
-    if (!fs.addFile(PreludePath, /*ModificationTime=*/0, std::move(*prelude))) {
+    if (!fs->addFile(PreludePath, /*ModificationTime=*/0,
+                     std::move(*prelude))) {
       return ErrorBuilder() << "Duplicate prelude.carbon";
     }
 
@@ -52,7 +54,7 @@ class ExplorerFileTest : public FileTestBase {
 
     int exit_code = ExplorerMain(
         args.size(), args.data(), /*install_path=*/"", PreludePath, stdout,
-        stderr, check_trace_output() ? stdout : trace_stream_, fs);
+        stderr, check_trace_output() ? stdout : trace_stream_, *fs);
 
     return {{.success = exit_code == EXIT_SUCCESS}};
   }

testing/base/source_gen_test.cpp

@@ -143,15 +143,16 @@ TEST(SourceGenTest, UniqueIdentifiers) {
 
 // Check that the source code doesn't have compiler errors.
 auto TestCompile(llvm::StringRef source) -> bool {
-  llvm::vfs::InMemoryFileSystem fs;
+  llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> fs =
+      new llvm::vfs::InMemoryFileSystem;
   InstallPaths installation(
       InstallPaths::MakeForBazelRunfiles(Testing::GetExePath()));
   Driver driver(fs, &installation, llvm::outs(), llvm::errs());
 
-  AddPreludeFilesToVfs(installation, &fs);
+  AddPreludeFilesToVfs(installation, fs);
 
-  fs.addFile("test.carbon", /*ModificationTime=*/0,
-             llvm::MemoryBuffer::getMemBuffer(source));
+  fs->addFile("test.carbon", /*ModificationTime=*/0,
+              llvm::MemoryBuffer::getMemBuffer(source));
   return driver.RunCommand({"compile", "--phase=check", "test.carbon"}).success;
 }
 

testing/file_test/file_test_base.cpp

@@ -297,12 +297,13 @@ auto FileTestBase::ProcessTestFileAndRun(TestContext& context)
       DoArgReplacements(context.test_args, context.test_files));
 
   // Create the files in-memory.
-  llvm::vfs::InMemoryFileSystem fs;
+  llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> fs =
+      new llvm::vfs::InMemoryFileSystem;
   for (const auto& test_file : context.test_files) {
-    if (!fs.addFile(test_file.filename, /*ModificationTime=*/0,
-                    llvm::MemoryBuffer::getMemBuffer(
-                        test_file.content, test_file.filename,
-                        /*RequiresNullTerminator=*/false))) {
+    if (!fs->addFile(test_file.filename, /*ModificationTime=*/0,
+                     llvm::MemoryBuffer::getMemBuffer(
+                         test_file.content, test_file.filename,
+                         /*RequiresNullTerminator=*/false))) {
       return ErrorBuilder() << "File is repeated: " << test_file.filename;
     }
   }

testing/file_test/file_test_base.h

@@ -77,7 +77,7 @@ class FileTestBase : public testing::Test {
   // The return value should be an error if there was an abnormal error, and
   // RunResult otherwise.
   virtual auto Run(const llvm::SmallVector<llvm::StringRef>& test_args,
-                   llvm::vfs::InMemoryFileSystem& fs,
+                   llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem>& fs,
                    llvm::raw_pwrite_stream& stdout,
                    llvm::raw_pwrite_stream& stderr) -> ErrorOr<RunResult> = 0;
 

testing/file_test/file_test_base_test.cpp

@@ -22,8 +22,9 @@ class FileTestBaseTest : public FileTestBase {
       : FileTestBase(output_mutex, test_name) {}
 
   auto Run(const llvm::SmallVector<llvm::StringRef>& test_args,
-           llvm::vfs::InMemoryFileSystem& fs, llvm::raw_pwrite_stream& stdout,
-           llvm::raw_pwrite_stream& stderr) -> ErrorOr<RunResult> override;
+           llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem>& fs,
+           llvm::raw_pwrite_stream& stdout, llvm::raw_pwrite_stream& stderr)
+      -> ErrorOr<RunResult> override;
 
   auto GetArgReplacements() -> llvm::StringMap<std::string> override {
     return {{"replacement", "replaced"}};
@@ -217,10 +218,10 @@ static auto EchoFileContent(TestParams& params)
   return {{.success = true}};
 }
 
-auto FileTestBaseTest::Run(const llvm::SmallVector<llvm::StringRef>& test_args,
-                           llvm::vfs::InMemoryFileSystem& fs,
-                           llvm::raw_pwrite_stream& stdout,
-                           llvm::raw_pwrite_stream& stderr)
+auto FileTestBaseTest::Run(
+    const llvm::SmallVector<llvm::StringRef>& test_args,
+    llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem>& fs,
+    llvm::raw_pwrite_stream& stdout, llvm::raw_pwrite_stream& stderr)
     -> ErrorOr<RunResult> {
   PrintArgs(test_args, stdout);
 
@@ -260,8 +261,8 @@ auto FileTestBaseTest::Run(const llvm::SmallVector<llvm::StringRef>& test_args,
           .Default(&EchoFileContent);
 
   // Call the appropriate test function for the file.
-  TestParams params = {.fs = fs, .stdout = stdout, .stderr = stderr};
-  CARBON_ASSIGN_OR_RETURN(params.files, GetFilesFromArgs(test_args, fs));
+  TestParams params = {.fs = *fs, .stdout = stdout, .stderr = stderr};
+  CARBON_ASSIGN_OR_RETURN(params.files, GetFilesFromArgs(test_args, *fs));
   return test_fn(params);
 }
 

testing/fuzzing/libfuzzer.h

@@ -14,8 +14,7 @@ namespace Carbon::Testing {
 // defining an undeclared extern function due to a Clang warning bug:
 // https://github.com/llvm/llvm-project/issues/94138
 // NOLINTNEXTLINE: Match the documented fuzzer entry point declaration style.
-extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data,
-                                      std::size_t size);
+extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data, size_t size);
 
 // Optional API that can be implemented but isn't required. This allows fuzzers
 // to observe the `argv` during initialization.

toolchain/check/check.cpp

@@ -387,7 +387,7 @@ class NextDeferredDefinitionCache {
   // be encountered.
   auto SkipTo(Parse::DeferredDefinitionIndex next_index) -> void {
     index_ = next_index;
-    if (static_cast<std::size_t>(index_.index) ==
+    if (static_cast<size_t>(index_.index) ==
         tree_->deferred_definitions().size()) {
       start_id_ = Parse::NodeId::Invalid;
     } else {

toolchain/check/check_fuzzer.cpp

@@ -22,18 +22,18 @@ extern "C" auto LLVMFuzzerInitialize(int* argc, char*** argv) -> int {
 }
 
 // NOLINTNEXTLINE: Match the documented fuzzer entry point declaration style.
-extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data,
-                                      std::size_t size) {
+extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data, size_t size) {
   // Ignore large inputs.
   // TODO: See tokenized_buffer_fuzzer.cpp.
   if (size > 100000) {
     return 0;
   }
 
   static constexpr llvm::StringLiteral TestFileName = "test.carbon";
-  llvm::vfs::InMemoryFileSystem fs;
+  llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> fs =
+      new llvm::vfs::InMemoryFileSystem;
   llvm::StringRef data_ref(reinterpret_cast<const char*>(data), size);
-  CARBON_CHECK(fs.addFile(
+  CARBON_CHECK(fs->addFile(
       TestFileName, /*ModificationTime=*/0,
       llvm::MemoryBuffer::getMemBuffer(data_ref, /*BufferName=*/TestFileName,
                                        /*RequiresNullTerminator=*/false)));

toolchain/check/context.cpp

@@ -481,6 +481,12 @@ auto Context::AppendLookupScopesForConstant(
     }
     return true;
   }
+  if (base_const_id == SemIR::ConstantId::Error) {
+    // Lookup into this scope should fail without producing an error.
+    scopes->push_back(LookupScope{.name_scope_id = SemIR::NameScopeId::Invalid,
+                                  .specific_id = SemIR::SpecificId::Invalid});
+    return true;
+  }
   // TODO: Per the design, if `base_id` is any kind of type, then lookup should
   // treat it as a name scope, even if it doesn't have members. For example,
   // `(i32*).X` should fail because there's no name `X` in `i32*`, not because
@@ -1021,7 +1027,7 @@ class TypeCompleter {
     llvm_unreachable("All builtin kinds were handled above");
   }
 
-  auto BuildStructOrTupleValueRepr(std::size_t num_elements,
+  auto BuildStructOrTupleValueRepr(size_t num_elements,
                                    SemIR::TypeId elementwise_rep,
                                    bool same_as_object_rep) const
       -> SemIR::ValueRepr {

toolchain/check/convert.cpp

@@ -147,7 +147,7 @@ template <typename AccessInstT, typename InstBlockT>
 static auto MakeElementAccessInst(Context& context, SemIR::LocId loc_id,
                                   SemIR::InstId aggregate_id,
                                   SemIR::TypeId elem_type_id, InstBlockT& block,
-                                  std::size_t i) {
+                                  size_t i) {
   if constexpr (std::is_same_v<AccessInstT, SemIR::ArrayIndex>) {
     // TODO: Add a new instruction kind for indexing an array at a constant
     // index so that we don't need an integer literal instruction here, and
@@ -183,7 +183,7 @@ static auto ConvertAggregateElement(
     SemIR::TypeId src_elem_type,
     llvm::ArrayRef<SemIR::InstId> src_literal_elems,
     ConversionTarget::Kind kind, SemIR::InstId target_id,
-    SemIR::TypeId target_elem_type, PendingBlock* target_block, std::size_t i) {
+    SemIR::TypeId target_elem_type, PendingBlock* target_block, size_t i) {
   // Compute the location of the source element. This goes into the current code
   // block, not into the target block.
   // TODO: Ideally we would discard this instruction if it's unused.

toolchain/check/handle_class.cpp

@@ -405,26 +405,8 @@ auto HandleParseNode(Context& context, Parse::AdaptDeclId node_id) -> bool {
 
   // Extend the class scope with the adapted type's scope if requested.
   if (introducer.modifier_set.HasAnyOf(KeywordModifierSet::Extend)) {
-    auto extended_scope_inst_id = SemIR::InstId::Invalid;
-    if (adapted_type_id == SemIR::TypeId::Error) {
-      // Recover by not extending any scope. We instead set has_error to true
-      // below.
-    } else if (auto* adapted_class_info =
-                   TryGetAsClass(context, adapted_type_id)) {
-      extended_scope_inst_id = adapted_inst_id;
-      CARBON_CHECK(adapted_class_info->scope_id.is_valid(),
-                   "Complete class should have a scope");
-    } else {
-      // TODO: Accept any type that has a scope.
-      context.TODO(node_id, "extending non-class type");
-    }
-
     auto& class_scope = context.name_scopes().Get(class_info.scope_id);
-    if (extended_scope_inst_id.is_valid()) {
-      class_scope.extended_scopes.push_back(extended_scope_inst_id);
-    } else {
-      class_scope.has_error = true;
-    }
+    class_scope.extended_scopes.push_back(adapted_inst_id);
   }
   return true;
 }