Skip to content

Conversation

@llvmbot
Copy link
Member

@llvmbot llvmbot commented Nov 12, 2024

Backport 075581f

Requested by: @aheejin

@llvmbot llvmbot added this to the LLVM 19.X Release milestone Nov 12, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Nov 12, 2024

@vgvassilev What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from vgvassilev November 12, 2024 10:23
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 12, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Nov 12, 2024

@llvm/pr-subscribers-clang

Author: None (llvmbot)

Changes

Backport 075581f

Requested by: @aheejin


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

1 Files Affected:

  • (modified) clang/lib/Interpreter/CMakeLists.txt (+2)
diff --git a/clang/lib/Interpreter/CMakeLists.txt b/clang/lib/Interpreter/CMakeLists.txt
index 6a069659ebb8db..0a2d60757c216d 100644
--- a/clang/lib/Interpreter/CMakeLists.txt
+++ b/clang/lib/Interpreter/CMakeLists.txt
@@ -14,6 +14,7 @@ set(LLVM_LINK_COMPONENTS
 
 if (EMSCRIPTEN AND "lld" IN_LIST LLVM_ENABLE_PROJECTS)
   set(WASM_SRC Wasm.cpp)
+  set(WASM_LINK lldWasm)
 endif()
 
 add_clang_library(clangInterpreter
@@ -43,6 +44,7 @@ add_clang_library(clangInterpreter
   clangParse
   clangSema
   clangSerialization
+  ${WASM_LINK}
   )
 
 if ((MINGW OR CYGWIN) AND BUILD_SHARED_LIBS)

@github-actions
Copy link

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

@aheejin
Copy link
Member

aheejin commented Nov 12, 2024

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo. Please turn off Keep my email addresses private setting in your account. See LLVM Discourse for more information.

cc @anutosh491

@anutosh491
Copy link
Member

anutosh491 commented Nov 12, 2024

Yupp made the change. Have never seen such a message before and I wasn't even sure as to why my mail was private (changed it to public now). Is something more expected from my side ?

I am guessing we can try closing and opening this PR if something in CI fails due to that !

Copy link
Contributor

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, the risk of breaking binary compatibility and user base is very low.

@anutosh491
Copy link
Member

I think one of the checks has been queued since quite some time but isn't running.

@anutosh491
Copy link
Member

Hi @aheejin

The checks pass as expected. I guess this should be ready !

@aheejin
Copy link
Member

aheejin commented Nov 12, 2024

As I said I'm not a release maintainer. I don't have permission to merge commits to release branches.

@anutosh491
Copy link
Member

As I said I'm not a release maintainer.

Ahh okay, in that case I am guessing @tru might be able to help us out.

@tru tru merged commit ec947f9 into llvm:release/19.x Nov 15, 2024
…gInterpreter for wasm (llvm#113446)

While building llvm (clang, lld) for wasm using emscripten (recipe
hosted on emscripten-forge
https://github.com/emscripten-forge/recipes/tree/main/recipes/recipes_emscripten/llvm)
I ended up with this error
```
 │ │ wasm-ld: error: ../../../../lib/libclangInterpreter.a(Wasm.cpp.o): undefined symbol: lld::wasm::link(llvm::ArrayRef<char const*>, llvm::raw_ostream&, llvm:
 │ │ :raw_ostream&, bool, bool)
 ```
 This is due to the link function here
 https://github.com/llvm/llvm-project/blob/a4819bd46d8baebc3aaa8b38f78065de33593199/clang/lib/Interpreter/Wasm.cpp#L25-L30

 This was added through this PR (llvm#86402) as an attempt to support running clang-repl and executing C++ code interactively inside a Javascript engine using WebAssembly when built with Emscripten.

 The definition for link is present in lldwasm and when building for the emscripten platform we should be linking against it.

(cherry picked from commit 075581f)
@github-actions
Copy link

@aheejin (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

Development

Successfully merging this pull request may close these issues.

5 participants