[Lldb-commits] MSVC12 support

Malea, Daniel daniel.malea at intel.com
Fri Sep 20 10:55:10 PDT 2013


The changes seem reasonable to me too.

A windows buildbot is a fantastic idea!

Dan

-----Original Message-----
From: lldb-commits-bounces at cs.uiuc.edu [mailto:lldb-commits-bounces at cs.uiuc.edu] On Behalf Of Greg Clayton
Sent: Friday, September 20, 2013 1:51 PM
To: Virgile Bello
Cc: lldb-commits
Subject: Re: [Lldb-commits] MSVC12 support

Looks good.

On Sep 20, 2013, at 10:37 AM, Virgile Bello <virgile.bello at gmail.com> 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 <virgile.bello at gmail.com> 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 <gclayton at apple.com> 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 <virgile.bello at gmail.com> 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 <gclayton at apple.com> wrote:
> > Looks fine to me.
> >
> > On Sep 4, 2013, at 9:19 AM, Virgile Bello <virgile.bello at gmail.com> 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 <gclayton at apple.com> 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 <virgile.bello at gmail.com> 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 <ripzonetriton at gmail.com> 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
> > > > lldb-commits at cs.uiuc.edu
> > > > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> > >
> > >
> > > <lldb-getopt.diff>
> >
> >
> > <lldb-msvc12-v3.diff>
> 
> 
> 
> <lldb-msvc12-v4.diff>


_______________________________________________
lldb-commits mailing list
lldb-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits




More information about the lldb-commits mailing list