Fix LLVM C API for DataLayout

[mehdi_amini]

Summary

We removed access to the DataLayout on the TargetMachine and
deprecated the C API function LLVMGetTargetMachineData() in r243114.
However the way I tried to be backward compatible was broken: I
changed the wrapper of the TargetMachine to be a structure that
includes the DataLayout as well. However the TargetMachine is also
wrapped by the ExecutionEngine, in the more classic way. A client
using the TargetMachine wrapped by the ExecutionEngine and trying
to get the DataLayout would break.

It seems tricky to solve the problem completely in the C API
implementation. This patch tries to address this backward
compatibility in a more lighter way in the C++ API. The C API is
restored in its original state and the removed C++ API is
reintroduced, but privately. The C API is friended to the
TargetMachine and should be the only consumer for this API.

[mehdi_amini]

clang-format

[deadalnix]

So, you kept the old LLVMGetTargetMachineData working, but marked it as depreacated, and introduced the new LLVMGetDataLayout method as pa preferred way to get the datalayout. Is that it ?

While creating a module from the C API, one has to request the target machine for its datalayout to feed it to the module. How one is supposed to get this datalayout (in order to create a module) if the prefereed way to get a data layout require a module to begin with ?

[deadalnix]

Also, can we please have tests for the C API ? A good start would be D10725.

[mehdi_amini]

There is no API changes in this patch: LLVMGetTargetMachineData () was already deprecated, and I don't introduce LLVMGetDataLayout() it was already there.

This patch just fixes a bug that I introduced last month, in the way I tried to support this deprecated API with respect to the new C++ API. Because the current C++ API does not provide anymore a way to implement LLVMGetTargetMachineData, I had to hack around to keep it alive. Unfortunately it is broken with JIT clients.

I know you are interested in the C API, but the discussion about how to move forward on the C API policy is separated from this patch, ping me Monday and I'll try to make progress on this.

[ributzka]

LGTM

[echristo]

This is pretty terrible and gets into maintenance of the C API questions. That said, if you want to keep the API around for a bit why not just create a new one every time?

-eric

include/llvm/Target/TargetMachine.h

 class TargetLoweringObjectFile;
 
 // The old pass manager infrastructure is hidden in a legacy namespace now.
 namespace legacy {
 class PassManagerBase;
 }
 using legacy::PassManagerBase;
 
+extern "C" {
+// This function from the C API is deprecated. We still supports it using a
+// private method on the TargetMachine for now. But it needs to be friended and
+// so we forward declare it here.
+LLVMTargetDataRef LLVMGetTargetMachineData(LLVMTargetMachineRef T);
+}
+
 //===----------------------------------------------------------------------===//
 ///
 /// Primary interface to the complete machine description for the target
 /// machine.  All target-specific information should be accessible through this
 /// interface.
 ///
 class TargetMachine {
   TargetMachine(const TargetMachine &) = delete;
protected: // Can only create subclasses.
   const MCAsmInfo *AsmInfo;
 
   const MCRegisterInfo *MRI;
   const MCInstrInfo *MII;
   const MCSubtargetInfo *STI;
 
   unsigned RequireStructuredCFG : 1;
 
+  /// This API is here to support the C API, deprecated in 3.7 release.
+  /// This should never be used outside of legacy existing client.
+  const DataLayout &getDataLayout() const { return DL; }
+  friend struct LLVMOpaqueTargetData * ::LLVMGetTargetMachineData(
+      LLVMTargetMachineRef T);
+
 public:
   mutable TargetOptions Options;
 
   virtual ~TargetMachine();
 
   const Target &getTarget() const { return TheTarget; }
 
   const Triple &getTargetTriple() const { return TargetTriple; }

lib/Target/TargetMachineC.cpp

 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetSubtargetInfo.h"
 #include <cassert>
 #include <cstdlib>
 #include <cstring>
 
 using namespace llvm;
 
-
-// The TargetMachine uses to offer access to a DataLayout member. This is reflected
-// in the C API. For backward compatibility reason, this structure allows to keep
-// a DataLayout member accessible to C client that have a handle to a
-// LLVMTargetMachineRef.
-struct LLVMOpaqueTargetMachine {
-  std::unique_ptr<TargetMachine> Machine;
-  DataLayout DL;
-};
-
-
 static TargetMachine *unwrap(LLVMTargetMachineRef P) {
-  return P->Machine.get();
+  return reinterpret_cast<TargetMachine *>(P);
 }
 static Target *unwrap(LLVMTargetRef P) {
   return reinterpret_cast<Target*>(P);
 }
 static LLVMTargetMachineRef wrap(const TargetMachine *P) {
-  return new LLVMOpaqueTargetMachine{ std::unique_ptr<TargetMachine>(const_cast<TargetMachine*>(P)),  P->createDataLayout() };
+  return reinterpret_cast<LLVMTargetMachineRef>(const_cast<TargetMachine *>(P));
 }
 static LLVMTargetRef wrap(const Target * P) {
   return reinterpret_cast<LLVMTargetRef>(const_cast<Target*>(P));
 }
 
 LLVMTargetRef LLVMGetFirstTarget() {
   if (TargetRegistry::targets().begin() == TargetRegistry::targets().end()) {
     return nullptr;
auto I = std::find_if(
       TargetRegistry::targets().begin(), TargetRegistry::targets().end(),
       [&](const Target &T) { return T.getName() == NameRef; });
   return I != TargetRegistry::targets().end() ? wrap(&*I) : nullptr;
 }
 
 LLVMBool LLVMGetTargetFromTriple(const char* TripleStr, LLVMTargetRef *T,
                                  char **ErrorMessage) {
   std::string Error;
-  
   *T = wrap(TargetRegistry::lookupTarget(TripleStr, Error));
-  
   if (!*T) {
     if (ErrorMessage)
       *ErrorMessage = strdup(Error.c_str());
 
     return 1;
   }
-  
   return 0;
 }
 
 const char * LLVMGetTargetName(LLVMTargetRef T) {
   return unwrap(T)->getName();
 }
 
 const char * LLVMGetTargetDescription(LLVMTargetRef T) {
default:
       break;
   }
 
   TargetOptions opt;
   return wrap(unwrap(T)->createTargetMachine(Triple, CPU, Features, opt, RM,
     CM, OL));
 }
 
-
-void LLVMDisposeTargetMachine(LLVMTargetMachineRef T) {
-  delete T;
-}
+void LLVMDisposeTargetMachine(LLVMTargetMachineRef T) { delete unwrap(T); }
 
 LLVMTargetRef LLVMGetTargetMachineTarget(LLVMTargetMachineRef T) {
   const Target* target = &(unwrap(T)->getTarget());
   return wrap(target);
 }
 
 char* LLVMGetTargetMachineTriple(LLVMTargetMachineRef T) {
   std::string StringRep = unwrap(T)->getTargetTriple().str();
   return strdup(StringRep.c_str());
 }
 
 char* LLVMGetTargetMachineCPU(LLVMTargetMachineRef T) {
   std::string StringRep = unwrap(T)->getTargetCPU();
   return strdup(StringRep.c_str());
 }
 
 char* LLVMGetTargetMachineFeatureString(LLVMTargetMachineRef T) {
   std::string StringRep = unwrap(T)->getTargetFeatureString();
   return strdup(StringRep.c_str());
 }
 
-/// @deprecated: see "struct LLVMOpaqueTargetMachine" description above
+/** Deprecated: use LLVMGetDataLayout(LLVMModuleRef M) instead. */
 LLVMTargetDataRef LLVMGetTargetMachineData(LLVMTargetMachineRef T) {
-  return wrap(&T->DL);
+  return wrap(&unwrap(T)->getDataLayout());
 }
 
 void LLVMSetTargetMachineAsmVerbosity(LLVMTargetMachineRef T,
                                       LLVMBool VerboseAsm) {
   unwrap(T)->Options.MCOptions.AsmVerbose = VerboseAsm;
 }
 
 static LLVMBool LLVMTargetMachineEmit(LLVMTargetMachineRef T, LLVMModuleRef M,