[Lldb-commits] MSVC12 support

[Virgile Bello]

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>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20130910/2bdd7ce1/attachment.html>