Skip to content

[clang][DepScan] Allow ModuleDep to be const #132968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

cyndyishida
Copy link
Member

This type can be exposed from C APIs, where instantiations of this type are not expected to mutate after creation. To support this, mark the lazy computation of build arguments mutable, as that is not intended to otherwise mutate the state of these objects.

This was reviewed separately by @jansvoboda11

* Mark the lazy computation of build arguments mutable. As that is not
  intended to otherwise mutate the state of the object.
@cyndyishida cyndyishida added the skip-precommit-approval PR for CI feedback, not intended for review label Mar 25, 2025
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Mar 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 25, 2025

@llvm/pr-subscribers-clang

Author: Cyndy Ishida (cyndyishida)

Changes

This type can be exposed from C APIs, where instantiations of this type are not expected to mutate after creation. To support this, mark the lazy computation of build arguments mutable, as that is not intended to otherwise mutate the state of these objects.

This was reviewed separately by @jansvoboda11


Full diff: https://github.com/llvm/llvm-project/pull/132968.diff

2 Files Affected:

  • (modified) clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h (+3-2)
  • (modified) clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp (+3-1)
diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index 422202caddfd4..ed150b467e3a1 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -153,7 +153,7 @@ struct ModuleDeps {
 
   /// Get (or compute) the compiler invocation that can be used to build this
   /// module. Does not include argv[0].
-  const std::vector<std::string> &getBuildArguments();
+  const std::vector<std::string> &getBuildArguments() const;
 
 private:
   friend class ModuleDepCollector;
@@ -166,7 +166,8 @@ struct ModuleDeps {
   /// including transitive dependencies.
   std::vector<std::string> FileDeps;
 
-  std::variant<std::monostate, CowCompilerInvocation, std::vector<std::string>>
+  mutable std::variant<std::monostate, CowCompilerInvocation,
+                       std::vector<std::string>>
       BuildInfo;
 };
 
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index d715ef874e002..364f156566855 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -31,7 +31,9 @@ void ModuleDeps::forEachFileDep(llvm::function_ref<void(StringRef)> Cb) const {
   }
 }
 
-const std::vector<std::string> &ModuleDeps::getBuildArguments() {
+const std::vector<std::string> &ModuleDeps::getBuildArguments() const {
+  // FIXME: this operation is not thread safe and is expected to be called
+  // on a single thread. Otherwise it should be protected with a lock.
   assert(!std::holds_alternative<std::monostate>(BuildInfo) &&
          "Using uninitialized ModuleDeps");
   if (const auto *CI = std::get_if<CowCompilerInvocation>(&BuildInfo))

@cyndyishida cyndyishida merged commit 9aecbdf into llvm:main Mar 25, 2025
14 checks passed
@cyndyishida cyndyishida deleted the eng/PR-makeModuleDepsConstCorrect branch March 25, 2025 20:14
@llvm-ci
Copy link
Collaborator

llvm-ci commented Mar 25, 2025

LLVM Buildbot has detected a new failure on builder lldb-aarch64-ubuntu running on linaro-lldb-aarch64-ubuntu while building clang at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/14918

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
PASS: lldb-api :: functionalities/gdb_remote_client/TestTargetXMLArch.py (498 of 2110)
PASS: lldb-api :: functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py (499 of 2110)
PASS: lldb-api :: functionalities/inferior-changed/TestInferiorChanged.py (500 of 2110)
PASS: lldb-api :: functionalities/inferior-crashing/TestInferiorCrashing.py (501 of 2110)
PASS: lldb-api :: functionalities/inferior-assert/TestInferiorAssert.py (502 of 2110)
PASS: lldb-api :: functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py (503 of 2110)
PASS: lldb-api :: functionalities/inline-sourcefile/TestInlineSourceFiles.py (504 of 2110)
PASS: lldb-api :: functionalities/inferior-crashing/TestInferiorCrashingStep.py (505 of 2110)
UNSUPPORTED: lldb-api :: functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py (506 of 2110)
PASS: lldb-api :: functionalities/json/object-file/TestObjectFileJSON.py (507 of 2110)
FAIL: lldb-api :: functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferior.py (508 of 2110)
******************** TEST 'lldb-api :: functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferior.py' FAILED ********************
Script:
--
/usr/bin/python3.10 /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin --arch aarch64 --build-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/lldb --compiler /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/clang --dsymutil /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin --lldb-obj-root /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb --lldb-libs-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./lib /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/functionalities/inferior-crashing/recursive-inferior -p TestRecursiveInferior.py
--
Exit Code: -11

Command Output (stdout):
--
lldb version 21.0.0git (https://github.com/llvm/llvm-project.git revision 9aecbdf8ed787a8edd1b7f97a1c7fbf6e9d12515)
  clang revision 9aecbdf8ed787a8edd1b7f97a1c7fbf6e9d12515
  llvm revision 9aecbdf8ed787a8edd1b7f97a1c7fbf6e9d12515
Skipping the following test categories: ['libc++', 'dsym', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_recursive_inferior_crashing_dsym (TestRecursiveInferior.CrashingRecursiveInferiorTestCase) (test case does not fall in any category of interest for this run) 
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_recursive_inferior_crashing_dwarf (TestRecursiveInferior.CrashingRecursiveInferiorTestCase)
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_recursive_inferior_crashing_dwo (TestRecursiveInferior.CrashingRecursiveInferiorTestCase)
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_recursive_inferior_crashing_expr_dsym (TestRecursiveInferior.CrashingRecursiveInferiorTestCase) (test case does not fall in any category of interest for this run) 
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_recursive_inferior_crashing_expr_dwarf (TestRecursiveInferior.CrashingRecursiveInferiorTestCase)
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_recursive_inferior_crashing_expr_dwo (TestRecursiveInferior.CrashingRecursiveInferiorTestCase)
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_recursive_inferior_crashing_python_dsym (TestRecursiveInferior.CrashingRecursiveInferiorTestCase) (test case does not fall in any category of interest for this run) 
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_recursive_inferior_crashing_python_dwarf (TestRecursiveInferior.CrashingRecursiveInferiorTestCase)
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_recursive_inferior_crashing_python_dwo (TestRecursiveInferior.CrashingRecursiveInferiorTestCase)
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_recursive_inferior_crashing_register_dsym (TestRecursiveInferior.CrashingRecursiveInferiorTestCase) (test case does not fall in any category of interest for this run) 
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_recursive_inferior_crashing_register_dwarf (TestRecursiveInferior.CrashingRecursiveInferiorTestCase)
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_recursive_inferior_crashing_register_dwo (TestRecursiveInferior.CrashingRecursiveInferiorTestCase)
----------------------------------------------------------------------
Ran 12 tests in 2.024s

OK (skipped=4)

--

********************
PASS: lldb-api :: functionalities/json/symbol-file/TestSymbolFileJSON.py (509 of 2110)

cyndyishida added a commit to cyndyishida/llvm-project that referenced this pull request Mar 25, 2025
This type can be exposed from C APIs, where instantiations of this type
are not expected to mutate after creation. To support this, mark the
lazy computation of build arguments mutable, as that is not intended to
otherwise mutate the state of these objects.

This was reviewed separately by @jansvoboda11

(cherry picked from commit 9aecbdf)
cyndyishida added a commit to swiftlang/llvm-project that referenced this pull request Apr 2, 2025
This type can be exposed from C APIs, where instantiations of this type
are not expected to mutate after creation. To support this, mark the
lazy computation of build arguments mutable, as that is not intended to
otherwise mutate the state of these objects.

This was reviewed separately by @jansvoboda11

(cherry picked from commit 9aecbdf)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category skip-precommit-approval PR for CI feedback, not intended for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants