Skip to content

[clang-repl] Skip tests for non-linux and non-darwin platforms from #152562 #153005

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

kr-2003
Copy link
Contributor

@kr-2003 kr-2003 commented Aug 11, 2025

No description provided.

@kr-2003 kr-2003 changed the title [Clang-Repl] Skip tests for non-linux and non-darwin platforms for out-of-process JIT [Clang-Repl] Skip tests for non-linux and non-darwin platforms from #152562 Aug 11, 2025
@kr-2003 kr-2003 changed the title [Clang-Repl] Skip tests for non-linux and non-darwin platforms from #152562 [clang-repl] Skip tests for non-linux and non-darwin platforms from #152562 Aug 11, 2025
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Aug 11, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 11, 2025

@llvm/pr-subscribers-clang

Author: Abhinav Kumar (kr-2003)

Changes

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

1 Files Affected:

  • (modified) clang/unittests/Interpreter/OutOfProcessInterpreterTests.cpp (+8)
diff --git a/clang/unittests/Interpreter/OutOfProcessInterpreterTests.cpp b/clang/unittests/Interpreter/OutOfProcessInterpreterTests.cpp
index 4d5ef5c70d135..1e261e786c2e8 100644
--- a/clang/unittests/Interpreter/OutOfProcessInterpreterTests.cpp
+++ b/clang/unittests/Interpreter/OutOfProcessInterpreterTests.cpp
@@ -134,6 +134,14 @@ TEST_F(InterpreterTestBase, SanityWithRemoteExecution) {
   if (!HostSupportsJIT())
     GTEST_SKIP();
 
+  llvm::Triple SystemTriple(llvm::sys::getProcessTriple());
+
+  // FIXME: In the future, support more platforms beyond linux-x86_64 and macOS.
+  if (!SystemTriple.isOSDarwin() && !SystemTriple.isOSLinux()) {
+    GTEST_SKIP()
+        << "Out-of-process interpreter only supports linux-x86_64 and macos";
+  }
+
   std::unique_ptr<Interpreter> Interp = createInterpreterWithRemoteExecution();
 
   using PTU = PartialTranslationUnit;

@kr-2003
Copy link
Contributor Author

kr-2003 commented Aug 11, 2025

@rorth Can you review this?

@rorth rorth self-requested a review August 11, 2025 14:19
@rorth
Copy link
Collaborator

rorth commented Aug 11, 2025

(Some commenting the code directly didn't work for me ;-)

This check

!SystemTriple.isOSLinux()

isn't enough: it encompasses any version of Linux, not just Linux/x86_64. Besides, as I said, getOrcRuntimePath currently hardcodes the runtimes layout while one can still use the projects layout if one so chooses. I'd rather you'd skip the test unconditionally until this is all figured out: as is, the cost is way too fragile and makes many unwarranted assumptions.

@kr-2003
Copy link
Contributor Author

kr-2003 commented Aug 11, 2025

I understand your concern about the fragility of the check and the hardcoded assumptions in getOrcRuntimePath.
In this case, though, the feature under test is explicitly implemented only for linux-x86_64 and macOS/Darwin. Because of this, I think it’s important to still run the test on these supported platforms.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants