-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
cling: 1.0 -> 1.2 #439741
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
cling: 1.0 -> 1.2 #439741
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
85 changes: 85 additions & 0 deletions
85
...plications/editors/jupyter-kernels/xeus-cling/0003-Remove-unsupported-src-root-flag.patch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| diff --git a/CMakeLists.txt b/CMakeLists.txt | ||
| index 43718f5..d0d8670 100644 | ||
| --- a/CMakeLists.txt | ||
| +++ b/CMakeLists.txt | ||
| @@ -63,8 +63,7 @@ if(LLVM_CONFIG) | ||
| "--bindir" | ||
| "--libdir" | ||
| "--includedir" | ||
| - "--prefix" | ||
| - "--src-root") | ||
| + "--prefix") | ||
| execute_process(COMMAND ${CONFIG_COMMAND} | ||
| RESULT_VARIABLE HAD_ERROR | ||
| OUTPUT_VARIABLE CONFIG_OUTPUT) | ||
| diff --git a/src/xmagics/executable.cpp b/src/xmagics/executable.cpp | ||
| index 391c8c9..aba5e03 100644 | ||
| --- a/src/xmagics/executable.cpp | ||
| +++ b/src/xmagics/executable.cpp | ||
| @@ -12,6 +12,7 @@ | ||
| #include <iterator> | ||
| #include <fstream> | ||
| #include <memory> | ||
| +#include <optional> | ||
| #include <string> | ||
| #include <vector> | ||
|
|
||
| @@ -25,7 +26,7 @@ | ||
| #include "clang/AST/ASTContext.h" | ||
| #include "clang/AST/DeclGroup.h" | ||
| #include "clang/AST/RecursiveASTVisitor.h" | ||
| -#include "clang/Basic/DebugInfoOptions.h" | ||
| +#include "llvm/Frontend/Debug/Options.h" | ||
| #include "clang/Basic/Sanitizers.h" | ||
| #include "clang/Basic/TargetInfo.h" | ||
| #include "clang/CodeGen/BackendUtil.h" | ||
| @@ -115,7 +116,7 @@ namespace xcpp | ||
| // Filter out functions added by Cling. | ||
| if (auto Identifier = D->getIdentifier()) | ||
| { | ||
| - if (Identifier->getName().startswith("__cling")) | ||
| + if (Identifier->getName().starts_with("__cling")) | ||
| { | ||
| return true; | ||
| } | ||
| @@ -153,12 +154,13 @@ namespace xcpp | ||
| if (EnableDebugInfo) | ||
| { | ||
| CodeGenOpts.setDebugInfo( | ||
| - clang::codegenoptions::DebugInfoKind::FullDebugInfo); | ||
| + llvm::codegenoptions::DebugInfoKind::FullDebugInfo); | ||
| } | ||
|
|
||
| std::unique_ptr<clang::CodeGenerator> CG(clang::CreateLLVMCodeGen( | ||
| - CI->getDiagnostics(), "object", HeaderSearchOpts, | ||
| - CI->getPreprocessorOpts(), CodeGenOpts, *Context)); | ||
| + CI->getDiagnostics(), "object", | ||
| + llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>(&CI->getVirtualFileSystem()), | ||
| + HeaderSearchOpts, CI->getPreprocessorOpts(), CodeGenOpts, *Context)); | ||
| CG->Initialize(AST); | ||
|
|
||
| FindTopLevelDecls Visitor(CG.get()); | ||
| @@ -186,7 +188,9 @@ namespace xcpp | ||
| EmitBackendOutput(CI->getDiagnostics(), HeaderSearchOpts, | ||
| CodeGenOpts, CI->getTargetOpts(), | ||
| CI->getLangOpts(), DataLayout, CG->GetModule(), | ||
| - clang::Backend_EmitObj, std::move(OS)); | ||
| + clang::Backend_EmitObj, | ||
| + llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>(&CI->getVirtualFileSystem()), | ||
| + std::move(OS)); | ||
| return true; | ||
| } | ||
|
|
||
| @@ -222,10 +226,10 @@ namespace xcpp | ||
|
|
||
| llvm::StringRef OutputFileStr(OutputFile); | ||
| llvm::StringRef ErrorFileStr(ErrorFile); | ||
| - llvm::SmallVector<llvm::Optional<llvm::StringRef>, 16> Redirects = {llvm::NoneType::None, OutputFileStr, ErrorFileStr}; | ||
| + llvm::SmallVector<std::optional<llvm::StringRef>, 16> Redirects = {std::nullopt, OutputFileStr, ErrorFileStr}; | ||
|
|
||
| // Finally run the linker. | ||
| - int ret = llvm::sys::ExecuteAndWait(Compiler, Args, llvm::NoneType::None, | ||
| + int ret = llvm::sys::ExecuteAndWait(Compiler, Args, std::nullopt, | ||
| Redirects); | ||
|
|
||
| // Read back output and error streams. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| From cd4d1d8c4963620a6a84834948845df81fbbd70b Mon Sep 17 00:00:00 2001 | ||
| From: Jonas Hahnfeld <jonas.hahnfeld@cern.ch> | ||
| Date: Tue, 17 Dec 2024 14:54:18 +0100 | ||
| Subject: [PATCH] Use single Parser for LookupHelper | ||
|
|
||
| It is the only construction of a temporary parser, and it seems not | ||
| necessary (anymore). | ||
| --- | ||
| include/cling/Interpreter/LookupHelper.h | 2 +- | ||
| lib/Interpreter/Interpreter.cpp | 11 ++++------- | ||
| 2 files changed, 5 insertions(+), 8 deletions(-) | ||
|
|
||
| diff --git a/include/cling/Interpreter/LookupHelper.h b/include/cling/Interpreter/LookupHelper.h | ||
| index 6e6e281470..cd79b2a65c 100644 | ||
| --- a/include/cling/Interpreter/LookupHelper.h | ||
| +++ b/include/cling/Interpreter/LookupHelper.h | ||
| @@ -56,7 +56,7 @@ namespace cling { | ||
| WithDiagnostics | ||
| }; | ||
| private: | ||
| - std::unique_ptr<clang::Parser> m_Parser; | ||
| + clang::Parser* m_Parser; | ||
| Interpreter* m_Interpreter; // we do not own. | ||
| std::array<const clang::Type*, kNumCachedStrings> m_StringTy = {{}}; | ||
| /// A map containing the hash of the lookup buffer. This allows us to avoid | ||
| diff --git a/lib/Interpreter/Interpreter.cpp b/lib/Interpreter/Interpreter.cpp | ||
| index 13c8409cc5..f04695439b 100644 | ||
| --- a/lib/Interpreter/Interpreter.cpp | ||
| +++ b/lib/Interpreter/Interpreter.cpp | ||
| @@ -265,13 +265,6 @@ namespace cling { | ||
| } | ||
|
|
||
| Sema& SemaRef = getSema(); | ||
| - Preprocessor& PP = SemaRef.getPreprocessor(); | ||
| - | ||
| - m_LookupHelper.reset(new LookupHelper(new Parser(PP, SemaRef, | ||
| - /*SkipFunctionBodies*/false, | ||
| - /*isTemp*/true), this)); | ||
| - if (!m_LookupHelper) | ||
| - return; | ||
|
|
||
| if (!isInSyntaxOnlyMode() && !m_Opts.CompilerOpts.CUDADevice) { | ||
| m_Executor.reset(new IncrementalExecutor(SemaRef.Diags, *getCI(), | ||
| @@ -317,6 +310,10 @@ namespace cling { | ||
| return; | ||
| } | ||
|
|
||
| + m_LookupHelper.reset(new LookupHelper(m_IncrParser->getParser(), this)); | ||
| + if (!m_LookupHelper) | ||
| + return; | ||
| + | ||
| // When not using C++ modules, we now have a PCH and we can safely setup | ||
| // our callbacks without fearing that they get overwritten by clang code. | ||
| // The modules setup is handled above. |
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should probably use
stdenv.cc.libcxx, as the libc++ libraries are backwards‐compatible with older headers but mixing the library versions is a bad idea. (Not a blocker, as it’s not a new problem.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean
llvmPackages_18.libcxx->stdenv.cc.libcxx? That doesn't seem like a good idea considering these are library flags. And also that we deliberately useclangStdenvabove.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clangStdenvdoesn’t imply the use of libstdc++ vs. libc++, only the C compiler; it keeps the default C++ library for the platform. (libcxxStdenvexplicitly picks the combination of Clang and libc++.)The C++ standard library is ABI‐sensitive, so mixing different versions of libc++, or libstdc++ with libc++, can cause compatibility issues. However, libc++’s headers only work with a specific range of Clang versions, but can be used to target later versions of the library. So it is correct to use a specific include directory here, but you generally want to ensure the use of the same version to actually link with.
That’s why it probably doesn’t make sense to have a separate
useLLVMLibcxxhere, as opposed to conditioning on the platform (and hence why I dropped it in my PR). So, in that case,stdenv.cc.libcxxwould be non‐nullexactly when the platform uses libc++, and you’d want to link with that version while using the headers from Cling’s version of LLVM.Anyway, this is all stuff you don’t have to think about if you reuse our existing wrapper logic, ideally :) And in this case hopefully the things Cling builds don’t link with much, so the scope for problems is limited. But generally linking against a specific C++ standard library isn’t too reliable (and indeed problems with mixing them in obscure cases are what is driving us to switch to linking against the system‐provided library from the SDK on Darwin).