[PATCH] Make an RAII initializer for COM on Windows

[Zachary Turner]

On Mon, Apr 27, 2015 at 5:52 AM Aaron Ballman <aaron.ballman at gmail.com>
wrote:

> On Fri, Apr 24, 2015 at 5:39 PM, Zachary Turner <zturner at google.com>
> wrote:
> > Hi aaron.ballman, compnerd,
> >
> > Patch should be fairly self-explanatory.  Implementation is empty on
> non-Windows platforms, and on Windows platforms simply calls CoInitializeEx
> and CoUninitialize.
> >
> > http://reviews.llvm.org/D9267
> >
> > Files:
> >   include/llvm/Support/COM.h
> >   lib/Support/CMakeLists.txt
> >   lib/Support/COM.cpp
> >   lib/Support/Unix/COM.inc
> >   lib/Support/Windows/COM.inc
> >
> > EMAIL PREFERENCES
> >   http://reviews.llvm.org/settings/panel/emailpreferences/
>
> In addition to David's suggestion, some minor nits and questions below:
>
> > Index: include/llvm/Support/COM.h
> > ===================================================================
> > --- /dev/null
> > +++ include/llvm/Support/COM.h
> > @@ -0,0 +1,35 @@
> > +//===- llvm/Support/COM.h ---------------------------------------*- C++
> -*-===//
> > +//
> > +//                     The LLVM Compiler Infrastructure
> > +//
> > +// This file is distributed under the University of Illinois Open Source
> > +// License. See LICENSE.TXT for details.
> > +//
> >
> +//===----------------------------------------------------------------------===//
> > +/// \file
> > +///
> > +/// Provides a library for accessing COM functionality of the Host OS.
> > +///
> >
> +//===----------------------------------------------------------------------===//
> > +
> > +#ifndef LLVM_SUPPORT_COM_H
> > +#define LLVM_SUPPORT_COM_H
> > +
> > +namespace llvm {
> > +namespace sys {
> > +
> > +enum class COMThreadingMode { ApartmentThreaded, FreeThreaded };
>
> While I know that these are the COM terms, they're pretty
> unintelligible for folks who aren't used to COM. While not 100%
> accurate, do you think SingleThreaded and MultiThreaded would be more
> clear terms?
>
I'm not sure which is better.  On the one hand someone who doesn't
understand how COM works shouldn't be using this code in the first place,
but on the other hand I agree that it's hard to understand even if you do
know COM.  I don't feel strongly either way, but in any case I've made the
chance locally to Single and Multi.


>
> > +
> > +class COMInitializer {
> > +public:
> > +  COMInitializer(COMThreadingMode Threading, bool SpeedOverMemory =
> false);
>
> Explicit constructor (though not overly important)?
>
> > +  ~COMInitializer();
> > +
> > +private:
> > +  COMInitializer(const COMInitializer &) = delete;
> > +  void operator=(const COMInitializer &) = delete;
> > +};
> > +}
> > +}
> > +
> > +#endif
> > Index: lib/Support/CMakeLists.txt
> > ===================================================================
> > --- lib/Support/CMakeLists.txt
> > +++ lib/Support/CMakeLists.txt
> > @@ -37,6 +37,7 @@
> >    BlockFrequency.cpp
> >    BranchProbability.cpp
> >    circular_raw_ostream.cpp
> > +  COM.cpp
> >    CommandLine.cpp
> >    Compression.cpp
> >    ConvertUTF.c
>
> Do we need to do any special magic to ensure Ole32 is linked in, or
> have we always linked against that?
>
It's always linked in.  ole32 is one of the defaultlibs, and we don't
specify /NODEFAULTLIB


>
> > Index: lib/Support/COM.cpp
> > ===================================================================
> > --- /dev/null
> > +++ lib/Support/COM.cpp
> > @@ -0,0 +1,24 @@
> > +//===-- COM.cpp - Implement COM utility classes -----------------*- C++
> -*-===//
> > +//
> > +//                     The LLVM Compiler Infrastructure
> > +//
> > +// This file is distributed under the University of Illinois Open Source
> > +// License. See LICENSE.TXT for details.
> > +//
> >
> +//===----------------------------------------------------------------------===//
> > +//
> > +//  This file implements utility classes related to COM.
> > +//
> >
> +//===----------------------------------------------------------------------===//
> > +
> > +#include "llvm/Support/COM.h"
> > +
> > +#include "llvm/Config/config.h"
> > +
> > +// Include the platform-specific parts of this class.
> > +#ifdef LLVM_ON_UNIX
> > +#include "Unix/COM.inc"
> > +#endif
> > +#ifdef LLVM_ON_WIN32
> > +#include "Windows/COM.inc"
> > +#endif
>
> Use #elseif?
>
Done.


>
> > Index: lib/Support/Unix/COM.inc
> > ===================================================================
> > --- /dev/null
> > +++ lib/Support/Unix/COM.inc
> > @@ -0,0 +1,27 @@
> > +//===- llvm/Support/Unix/COM.inc - Unix COM Implementation -----*- C++
> -*-===//
> > +//
> > +//                     The LLVM Compiler Infrastructure
> > +//
> > +// This file is distributed under the University of Illinois Open Source
> > +// License. See LICENSE.TXT for details.
> > +//
> >
> +//===----------------------------------------------------------------------===//
> > +//
> > +// This file implements the Unix portion of COM support.
> > +//
> >
> +//===----------------------------------------------------------------------===//
> > +
> >
> +//===----------------------------------------------------------------------===//
> > +//=== WARNING: Implementation here must contain only generic UNIX code
> that
> > +//===          is guaranteed to work on *all* UNIX variants.
> >
> +//===----------------------------------------------------------------------===//
> > +
> > +namespace llvm {
> > +namespace sys {
> > +
> > +COMInitializer::COMInitializer(COMThreadingMode Threading,
> > +                               bool SpeedOverMemory) {}
> > +
> > +COMInitializer::~COMInitializer() {}
> > +}
> > +}
> > Index: lib/Support/Windows/COM.inc
> > ===================================================================
> > --- /dev/null
> > +++ lib/Support/Windows/COM.inc
> > @@ -0,0 +1,41 @@
> > +//===- llvm/Support/Windows/COM.inc - Windows COM Implementation *- C++
> -*-===//
> > +//
> > +//                     The LLVM Compiler Infrastructure
> > +//
> > +// This file is distributed under the University of Illinois Open Source
> > +// License. See LICENSE.TXT for details.
> > +//
> >
> +//===----------------------------------------------------------------------===//
> > +//
> > +// This file implements the Windows portion of COM support.
> > +//
> >
> +//===----------------------------------------------------------------------===//
> > +
> >
> +//===----------------------------------------------------------------------===//
> > +//=== WARNING: Implementation here must contain only generic UNIX code
> that
> > +//===          is guaranteed to work on *all* UNIX variants.
> >
> +//===----------------------------------------------------------------------===//
>
> Can you fix the comments regarding UNIX vs Windows? ;-)
>
Done.


>
> > +
> > +// We need to #include <windows.h> directly, and not WindowsSupport.h,
> because
> > +// WindowsSupport.h #defines WIN32_LEAN_AND_MEAN, which will prevent
> COM headers
> > +// from getting included.
> > +#include <windows.h>
>
> Do we need *all* of Windows.h though? I would guess that all we would
> need is Objbase.h here, and not Windows.h at all.
>
Didn't think this would work originally because I was using DWORD, but I
tried it and it does work, so this is done.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150427/bfd0178c/attachment.html>