Looks good. On Sep 20, 2013, at 10:37 AM, Virgile Bello <[email protected]> wrote:
> MSVC12 RC is now out, so I updated the patch to work with it (some changes > are not needed anymore due to fixes and better C++ and C++11 support). > Concerning the previous friend class namespace issue, I noticed those class > are not even used so I just removed the friend declaration. > > I've already committed many trivial changes over the past few days (used > llvm_unreachable where needed), and here is a patch with what's left. > It's quite small and trivial (biggest change is allowing backslash for both > MinGW and MSVC), so I will probably commit it soon except if you think > something is wrong with it. > > That should finally bring full support for both MSVC12 and MinGW compilation > for LLDB. > Next step might be to setup a buildbot slave so that they are both kept in > good shape and I can easily notice compilation errors. > > Virgile > > > On Mon, Sep 9, 2013 at 7:38 PM, Virgile Bello <[email protected]> wrote: > Ok thanks for the remarks, I will fix assert and forward declaration. > > For demangling, I also had a pending patch to readd recently removed LLDB > demangling code (with some MSVC specific fixes for stdcall for @). > It seemed to work well on callstack, including C++ and Win32 system calls. > I'll take a look to check if there is any differences between LLDB old one > and http://llvm.org/svn/llvm-project/libcxxabi/trunk > I will probably do that as a separate patch if that's OK. > > Thanks, > Virgile > > > On Tue, Sep 10, 2013 at 2:28 AM, Greg Clayton <[email protected]> wrote: > Looks ok, a few things to fix: > > Change any "assert(false)" code to be "assert(false && "explanation of why we > are asserting")". You did this in a few places, but do let people know what > the assertion is firing. > > You added a few forward class definitions, please move these over to > "lldb-forward.h" if needed and include that file if it already isn't. > > You very soon need to be able to demangle names, and the MSVC version has > this commented out so no demangling will occur. A set of open source sources > for demangling are at: > > http://llvm.org/svn/llvm-project/libcxxabi/trunk > > The file you will want is "src/cxa_demangle.cpp". We used to inline this into > our source code. You will need to change the namespace to not use the > reserved compiler namespaces for the standard library if you do inline this. > > Greg > > On Sep 6, 2013, at 10:15 PM, Virgile Bello <[email protected]> wrote: > > > Here is an updated MSVC2013 patch with what's not merged yet. Much smaller > > than before! > > > > > > On Thu, Sep 5, 2013 at 2:07 AM, Greg Clayton <[email protected]> wrote: > > Looks fine to me. > > > > On Sep 4, 2013, at 9:19 AM, Virgile Bello <[email protected]> wrote: > > > > > Would such a patch be OK for getopt? (with OptionParser.h) > > > Didn't try to hide actual getopt architecture, but at least it prevents > > > <getopt.h> from being included directly. > > > > > > If this gets accepted, the remaining part of MSVC patch should be quite > > > small. > > > > > > > > > On Thu, Aug 29, 2013 at 5:02 AM, Greg Clayton <[email protected]> wrote: > > > Yes in general to abstract ourselves from the system on which we are > > > compiling: > > > > > > 1 - First try and use LLVM functionality where it makes sense > > > 2 - Use LLDB Host layer > > > 3 - Use #ifdef in .cpp files only if possible > > > 4 - Use $ifdef in .h files as a last resort. > > > > > > To get back to the "getopt.h" stuff, it would be great to make a Host > > > class that abstracts us from the "getopt.h" functionality. > > > > > > Something like "include/lldb/Host/OptionParser.h" and then a .cpp file in > > > common that hides the abstraction for the actual unix version that is > > > used for everything but windows, and a windows specific one that could > > > use a compatibility layer... > > > > > > Greg > > > > > > On Aug 28, 2013, at 9:35 AM, Virgile Bello <[email protected]> > > > wrote: > > > > > > > Ha yes sure, I thought that was not recommended since LLVM was not used > > > > for many other similar situations (i.e. Mutex, Regex, Argument parsing > > > > vs getopt, some path functions, etc...). > > > > But I suppose it is maybe due more to the fact it wasn't good/stable > > > > enough at the time you needed it, and switch would be OK now? > > > > > > > > > > > > On Thu, Aug 29, 2013 at 1:07 AM, João Matos <[email protected]> > > > > wrote: > > > > +#ifdef _MSC_VER > > > > + InterlockedIncrement(&m_last_revision); > > > > +#else > > > > __sync_add_and_fetch(&m_last_revision, +1); > > > > +#endif > > > > > > > > I see this pattern ifdef'd in a lot of places, I think we should > > > > abstract it in an helper "atomics" function, or even better, just > > > > re-use LLVM support libraries (llvm::sys::AtomicIncrement). > > > > > > > > > > > > -- > > > > João Matos > > > > > > > > _______________________________________________ > > > > lldb-commits mailing list > > > > [email protected] > > > > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits > > > > > > > > > <lldb-getopt.diff> > > > > > > <lldb-msvc12-v3.diff> > > > > <lldb-msvc12-v4.diff> _______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
