Skip to content

[clang-repl] Remove redundant shared flag while running clang-repl in browser #118107

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
merged 1 commit into from
Dec 6, 2024

Conversation

anutosh491
Copy link
Member

@anutosh491 anutosh491 commented Nov 29, 2024

While running clang-repl in the browser, we would be interested in this cc1 command

"" -cc1 -triple wasm32-unknown-emscripten -emit-obj -disable-free -clear-ast-before-backend -disable-llvm-verifier -discard-value-names -main-file-name "<<< inputs >>>" -mrelocation-model static -mframe-pointer=none -ffp-contract=on -fno-rounding-math -mconstructor-aliases -target-cpu generic -debugger-tuning=gdb -fdebug-compilation-dir=/ -v -fcoverage-compilation-dir=/ -resource-dir /lib/clang/19 -internal-isystem /include/wasm32-emscripten/c++/v1 -internal-isystem /include/c++/v1 -internal-isystem /lib/clang/19/include -internal-isystem /include/wasm32-emscripten -internal-isystem /include -std=c++17 -fdeprecated-macro -ferror-limit 19 -fvisibility=default -fgnuc-version=4.2.1 -fskip-odr-check-in-gmf -fcxx-exceptions -fexceptions -fincremental-extensions -o "<<< inputs >>>.o" -x c++ "<<< inputs >>>"

As can be seen shared is anyway overwritten by static which is also what would be provided by default. Hence we can get rid of the shared flag here.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 29, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 29, 2024

@llvm/pr-subscribers-clang

Author: Anutosh Bhat (anutosh491)

Changes

While running clang-repl in the browser, we would be interested in this cc1 command

"" -cc1 -triple wasm32-unknown-emscripten -emit-obj -disable-free -clear-ast-before-backend -disable-llvm-verifier -discard-value-names -main-file-name "&lt;&lt;&lt; inputs &gt;&gt;&gt;" -mrelocation-model static -mframe-pointer=none -ffp-contract=on -fno-rounding-math -mconstructor-aliases -target-cpu generic -debugger-tuning=gdb -fdebug-compilation-dir=/ -v -fcoverage-compilation-dir=/ -resource-dir /lib/clang/19 -internal-isystem /include/wasm32-emscripten/c++/v1 -internal-isystem /include/c++/v1 -internal-isystem /lib/clang/19/include -internal-isystem /include/wasm32-emscripten -internal-isystem /include -std=c++17 -fdeprecated-macro -ferror-limit 19 -fvisibility=default -fgnuc-version=4.2.1 -fskip-odr-check-in-gmf -fcxx-exceptions -fexceptions -fincremental-extensions -o "&lt;&lt;&lt; inputs &gt;&gt;&gt;.o" -x c++ "&lt;&lt;&lt; inputs &gt;&gt;&gt;"

As can be seen shared is anyway overwritten by static which is also what would be provided by default. Hence we can get rid of the shared flag here.


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

1 Files Affected:

  • (modified) clang/lib/Interpreter/Interpreter.cpp (-1)
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index 887b494ff98f19..fa4c1439c92612 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -200,7 +200,6 @@ IncrementalCompilerBuilder::CreateCpp() {
 #ifdef __EMSCRIPTEN__
   Argv.push_back("-target");
   Argv.push_back("wasm32-unknown-emscripten");
-  Argv.push_back("-shared");
   Argv.push_back("-fvisibility=default");
 #endif
   Argv.insert(Argv.end(), UserArgs.begin(), UserArgs.end());

@anutosh491
Copy link
Member Author

cc @vgvassilev

Now that 19.1.5 is out. We can run clang-repl in the browser. And this flag is just adding redundancy and doesn't play a role.
Small change. Should be ready !

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!

@vgvassilev vgvassilev merged commit da24c02 into llvm:main Dec 6, 2024
11 checks passed
@anutosh491 anutosh491 deleted the patch-1 branch December 6, 2024 08:40
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
None yet
Development

Successfully merging this pull request may close these issues.

3 participants