Revert Thread-Safety #729

Vipul-Cariappa · 2025-10-14T15:46:51Z

To be merged later, after fixing stuff and performance.

This reverts commit c42c907.

…ance" This reverts commit db363c6.

This reverts commit da06567.

This reverts commit d210d68.

codecov · 2025-10-14T15:55:08Z

Codecov Report

❌ Patch coverage is 94.53125% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.83%. Comparing base (e1e07e0) to head (9368bb9).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/CppInterOp/CppInterOp.cpp	94.53%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #729      +/-   ##
==========================================
- Coverage   81.03%   79.83%   -1.21%     
==========================================
  Files           9        9              
  Lines        4214     3963     -251     
==========================================
- Hits         3415     3164     -251     
  Misses        799      799

Files with missing lines	Coverage Δ
include/CppInterOp/CppInterOp.h	`95.55% <ø> (ø)`
lib/CppInterOp/CppInterOpInterpreter.h	`88.60% <ø> (+0.51%)`	⬆️
lib/CppInterOp/CppInterOp.cpp	`88.04% <94.53%> (-0.77%)`	⬇️

... and 2 files with indirect coverage changes

Files with missing lines	Coverage Δ
include/CppInterOp/CppInterOp.h	`95.55% <ø> (ø)`
lib/CppInterOp/CppInterOpInterpreter.h	`88.60% <ø> (+0.51%)`	⬆️
lib/CppInterOp/CppInterOp.cpp	`88.04% <94.53%> (-0.77%)`	⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 21. Check the log or trigger a new build to see more.

github-actions · 2025-10-14T16:00:22Z

lib/CppInterOp/CppInterOp.cpp

-  return getInterpInfo(D).Interpreter->getSema();
-}
-static clang::Sema& getSema(const void* D) { return getInterp(D).getSema(); }
+static llvm::ManagedStatic<std::deque<InterpreterInfo>> sInterpreters;


warning: variable 'sInterpreters' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

static llvm::ManagedStatic<std::deque<InterpreterInfo>> sInterpreters; ^

github-actions · 2025-10-14T16:00:22Z

lib/CppInterOp/CppInterOp.cpp

+    return GetGlobalScope();

-  auto* ND = static_cast<NamedDecl*>(GetNamed(name, parent, interp));
+  auto* ND = (NamedDecl*)GetNamed(name, parent);


warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

auto* ND = (NamedDecl*)GetNamed(name, parent); ^

github-actions · 2025-10-14T16:00:22Z

lib/CppInterOp/CppInterOp.cpp

-  auto* Within = llvm::dyn_cast<clang::DeclContext>(D);
+TCppScope_t GetNamed(const std::string& name,
+                     TCppScope_t parent /*= nullptr*/) {
+  clang::DeclContext* Within = 0;


warning: use nullptr [modernize-use-nullptr]

Suggested change

clang::DeclContext* Within = 0;

clang::DeclContext* Within = nullptr;

github-actions · 2025-10-14T16:00:23Z

lib/CppInterOp/CppInterOp.cpp

+                     TCppScope_t parent /*= nullptr*/) {
+  clang::DeclContext* Within = 0;
+  if (parent) {
+    auto* D = (clang::Decl*)parent;


warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

auto* D = (clang::Decl*)parent; ^

github-actions · 2025-10-14T16:00:23Z

lib/CppInterOp/CppInterOp.cpp

+  }

-  auto* ND = Cpp_utils::Lookup::Named(&getSema(D), name, Within);
+  auto* ND = Cpp_utils::Lookup::Named(&getSema(), name, Within);


warning: no header providing "Cpp::utils::Lookup::Named" is directly included [misc-include-cleaner]

lib/CppInterOp/CppInterOp.cpp:12:

+ #include "CppInterOpInterpreter.h"

github-actions · 2025-10-14T16:00:23Z

lib/CppInterOp/CppInterOp.cpp

-  if (CXXRD->getNumBases() <= ibase)
-    return nullptr;
+  if (!CXXRD || CXXRD->getNumBases() <= ibase)
+    return 0;


warning: use nullptr [modernize-use-nullptr]

Suggested change

return 0;

return nullptr;

github-actions · 2025-10-14T16:00:23Z

lib/CppInterOp/CppInterOp.cpp


 TCppType_t GetFunctionArgType(TCppFunction_t func, TCppIndex_t iarg) {
-  auto* D = static_cast<clang::Decl*>(func);
+  auto* D = (clang::Decl*)func;


warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

auto* D = (clang::Decl*)func; ^

github-actions · 2025-10-14T16:00:23Z

lib/CppInterOp/CppInterOp.cpp

-  auto* Within = llvm::dyn_cast<DeclContext>(D);
-  LOCK(getInterpInfo(D));
+bool ExistsFunctionTemplate(const std::string& name, TCppScope_t parent) {
+  DeclContext* Within = 0;


warning: use nullptr [modernize-use-nullptr]

Suggested change

DeclContext* Within = 0;

DeclContext* Within = nullptr;

github-actions · 2025-10-14T16:00:23Z

lib/CppInterOp/CppInterOp.cpp

+bool ExistsFunctionTemplate(const std::string& name, TCppScope_t parent) {
+  DeclContext* Within = 0;
+  if (parent) {
+    auto* D = (Decl*)parent;


warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

auto* D = (Decl*)parent; ^

github-actions · 2025-10-14T16:00:24Z

lib/CppInterOp/CppInterOp.cpp

 static TCppFuncAddr_t GetFunctionAddress(const FunctionDecl* FD) {
  const auto get_mangled_name = [](const FunctionDecl* FD) {
-    auto* MangleCtxt = getASTContext(FD).createMangleContext();
+    auto MangleCtxt = getASTContext().createMangleContext();


warning: 'auto MangleCtxt' can be declared as 'auto *MangleCtxt' [llvm-qualified-auto]

Suggested change

auto MangleCtxt = getASTContext().createMangleContext();

auto *MangleCtxt = getASTContext().createMangleContext();

vgvassilev

LGTM. Having a TakeInterpreter interface outside of threading is useful I think.

Vipul-Cariappa added 4 commits October 14, 2025 17:44

Revert "Add tests and dev-doc"

74bcb4a

This reverts commit c42c907.

Revert "Add TInterp_t argument to funcs to resolve interpreter inst…

d606c8f

…ance" This reverts commit db363c6.

Revert "Add Cpp::TakeInterpreter"

f9e78db

This reverts commit da06567.

Revert "Thread-Safty: Add Mutex per Interpreter"

9368bb9

This reverts commit d210d68.

Vipul-Cariappa requested review from aaronj0 and vgvassilev October 14, 2025 15:46

github-actions bot reviewed Oct 14, 2025

View reviewed changes

vgvassilev approved these changes Oct 14, 2025

View reviewed changes

Vipul-Cariappa mentioned this pull request Oct 15, 2025

[Feature Request]: TakeInterpreter functionality to use be UseExternalInterpreter #732

Open

Vipul-Cariappa merged commit 1f43b4e into compiler-research:main Oct 15, 2025
56 of 58 checks passed

Vipul-Cariappa deleted the revert/thread-safety branch October 15, 2025 13:12

	clang::DeclContext* Within = 0;
	clang::DeclContext* Within = nullptr;

	auto MangleCtxt = getASTContext().createMangleContext();
	auto *MangleCtxt = getASTContext().createMangleContext();

Uh oh!

Revert Thread-Safety #729

Revert Thread-Safety #729

Conversation

Vipul-Cariappa commented Oct 14, 2025

Uh oh!

codecov bot commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

vgvassilev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Oct 14, 2025 •

edited

Loading