Port swift-demangle to Windows #6821
Conversation
| @@ -30,12 +30,16 @@ | |||
|
|
|||
| #include <cstdlib> | |||
| #include <string> | |||
| #if !defined(_MSC_VER) && !defined(__MINGW32__) | |||
| #if !defined(_WIN32) && !defined(__MINGW32__) | |||
There was a problem hiding this comment.
I think that I would rather see this as
#if defined (__unix__) || (defined(__APPLE__) && defined(__MACH__))
There was a problem hiding this comment.
Sure - what I'm hoping for is to actually be able to get rid of this whole thing, and just import <iostream> for std::getline, so this might actually all go!
| #include <unistd.h> | ||
| #else | ||
| #include <io.h> | ||
| #endif | ||
|
|
||
| #include <iostream> | ||
| #include <string> | ||
| #include <stdio.h> |
There was a problem hiding this comment.
What is this header pulling in?
There was a problem hiding this comment.
Not sure. I've cleaned up the headers in the latest commit, but I haven't tested it on multiple platforms, so we'll see if it all compiles :)
|
@swift-ci please smoke test |
|
Wonderful! looks like the tests are passing! This was caused by the fact that POSIX Fixed by appending a '\n' to the printing to |
|
I guess we don't care about |
|
(That's the main reason LLVM doesn't use it.) |
|
We do. We should avoid importing |
|
Interesting, considering I actually got this code pattern from LLVM: https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-cxxfilt/llvm-cxxfilt.cpp @compnerd was the author there. What's the alternative, id like to remove unistd imports and make this platform independent. |
|
I don't think it's so bad for a standalone tool like swift-demangle. If the constructors weren't static we'd still have to run them immediately just to read from stdin. (Admittedly swift-demangle doesn't always read from stdin.) |
|
We also do care about the streaming behavior (#4412). |
|
Okay, does the code look good in its current form? I'll defer to you both for a decision on the high level direction. |
|
I guess it's fine for now. LLVM doesn't have a similar portable API? |
|
LGTM |
Windows doesn't support
unistd.h. Therefore, it doesn't expose any kind ofgetlineposix function.Instead, let's try to make the swift-demangle.cpp file platform independent by using
std::getlineI've marked this as WIP as I'm not sure how similar
getlineandstd::getlineare. I'm also getting failures running the swift-demangle unit tests, but wanted to get this out there to see what I'm doing wrong!