Port swift-lldb to Windows #105
Conversation
|
I'm fairly sure I've found a bug in Notice that we append
However, surely if we append |
|
We need LLDB reviewers in here more than us. Calling @scallanan and @clayborg. |
| break; | ||
| } | ||
|
|
||
| #if defined(_WIN32) |
There was a problem hiding this comment.
If making temp files needs abstraction for other systems we should look and see if there is a LLVM equivalent. We don't want every place in the LLVM/Clang/LLDB source base to do any "#if defined(_WIN32)"
There was a problem hiding this comment.
Yeah, this is ugly. Let me know about this function: http://llvm.org/docs/doxygen/html/namespacellvm_1_1sys_1_1fs.html#ae3cdedf952d5eeb089d175c61d9f6fee
I think it does what we want
| #include "lldb/Core/State.h" | ||
| #include "lldb/Core/StreamFile.h" | ||
| #include "lldb/Core/ValueObjectRegister.h" | ||
| #ifndef LLDB_DISABLE_LIBEDIT |
There was a problem hiding this comment.
Does this file differ from the LLDB llvm.org SVN top of tree and the changes here are just syncing with top of tree SVN?
There was a problem hiding this comment.
I basically fixed the build errors that I presume were caused by a bad merge - many syntax errors occured as a result - e.g. missing {, unhandled identifiers etc. The changes to this file were copy-pasted from upstream, but the file is different.
Take a look at this commit here: https://github.com/hughbe/swift-lldb/commit/8d544a47ba95533c369949fa3bf0ad01dadea0a3
It demonstrates what the diff looks like when I copy-and-paste the remaining bits of IOHandler.cpp from upstream. Basically, its changes in API that I didn't want to modify right now because I don't really wan't to break all the things!
| if (!success) | ||
| FileSystem::Unlink(FileSpec(expr_source_path.c_str(), true)); | ||
|
|
||
| #if defined(_WIN32) |
There was a problem hiding this comment.
this line and the two below should go away if we abstract temp filenames into llvm or the LLDB host layer.
| if (log) | ||
| log->Printf("%p Socket::Close (fd = %i)", static_cast<void *>(this), | ||
| m_socket); | ||
| static_cast<int>(m_socket)); |
There was a problem hiding this comment.
Isn't m_socket a SOCKET on windows?:
SOCKET WSAAPI socket(
In int af,
In int type,
In int protocol
);
How does casting this to an integer help here?
There was a problem hiding this comment.
Socket is defined as:
typedef UINT_PTR SOCKET;
The warning we therefore get is trying to convert unsigned long long to int
| class ParserVars { | ||
| public: | ||
| ParserVars(ClangExpressionDeclMap &decl_map) : m_decl_map(decl_map) {} | ||
| ParserVars() {} |
There was a problem hiding this comment.
I'll let Sean Callanan comment on all expression parser changes.
|
No harm in testing. @swift-ci Please test |
|
@swift-ci Please test |
|
It looks like the expression parser changes are just cleanup that removes unused state. I love it (assuming the tests succeed)! |
Summary: Previously the builting demangler was on for platforms that explicitly set a flag by modifying Mangled.cpp (windows, freebsd). The Xcode build always used builtin demangler by passing a compiler flag. This adds a cmake flag (defaulting to ON) to configure the demangling library used at build time. The flag is only available on non-windows platforms as there the system demangler is not present (in the form we're trying to use it, at least). The impact of this change is: - linux: switches to the builtin demangler - freebsd, windows: NFC (I hope) - netbsd: switches to the builtin demangler - osx cmake build: switches to the builtin demangler (matching the XCode build) The main motivation for this is the cross-platform case, where it should bring more consistency by removing the dependency on the host demangler (which can be completely unrelated to the debug target). Reviewers: zturner, emaste, krytarowski Subscribers: emaste, clayborg, lldb-commits Differential Revision: https://reviews.llvm.org/D23830 git-svn-id: https://llvm.org/svn/llvm-project/lldb/trunk@279808 91177308-0d34-0410-b5e6-96231b3b80d8
- Basically copied from upstream
git-svn-id: https://llvm.org/svn/llvm-project/lldb/trunk@285834 91177308-0d34-0410-b5e6-96231b3b80d8
|
@swift-ci please test |
|
I get the feeling this is targeting the wrong branch... Which branch should I submit this PR to? (there's no upstream-with-swift here, so its unclear) |
|
I believe you should just target the master branch. |
|
Thanks! I know that trying to change the branch of this PR will end up notifying and sending emails to several hundred people as GitHub attempts to push their commits from the old branch to the new branch - see apple/swift-clang#44 So unfortunately, I'll have to close this PR and create a new one: #125 Sorry for losing the discussion history |
7 participants
This requires some CMake stuff, some upstream cherry-picking, and some modification of swift-specific source code.
Note: the changes in "Fix invalid namespace qualified UTF conversion helpers on Windows" changes lldb code. However: other actual lldb code was changed from
llvm::UTF8toUTF8inswift-clang,swift-llvmandswift-lldb. This just manifests itself now, because we never built these files on non-Windows platforms./cc @compnerd @jrose-apple