Skip to content

Commit

Permalink
Make all WinAdapter destructions explicit
Browse files Browse the repository at this point in the history
This prevents warnings about non-virtual destructor usage that trip up
the Linux build. It represents status quo on Windows.
  • Loading branch information
pow2clk authored and MarijnS95 committed Mar 15, 2022
1 parent 18b6edb commit 322b181
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 30 deletions.
16 changes: 9 additions & 7 deletions include/dxc/Support/microcom.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ class CComInterfaceArray {
}
};

template<typename T>
void DxcCallDestructor(T *obj) {
obj->T::~T();
}

#define DXC_MICROCOM_REF_FIELD(m_dwRef) \
volatile std::atomic<llvm::sys::cas_flag> m_dwRef = {0};
#define DXC_MICROCOM_ADDREF_IMPL(m_dwRef) \
Expand All @@ -86,8 +91,10 @@ class CComInterfaceArray {
DXC_MICROCOM_ADDREF_IMPL(m_dwRef) \
ULONG STDMETHODCALLTYPE Release() override { \
ULONG result = (ULONG)--m_dwRef; \
if (result == 0) \
delete this; \
if (result == 0) { \
DxcCallDestructor(this); \
operator delete(this); \
} \
return result; \
}

Expand All @@ -99,11 +106,6 @@ inline T *CreateOnMalloc(IMalloc * pMalloc, Args&&... args) {
return (T *)P;
}

template<typename T>
void DxcCallDestructor(T *obj) {
obj->T::~T();
}

// The "TM" version keep an IMalloc field that, if not null, indicate
// ownership of 'this' and of any allocations used during release.
#define DXC_MICROCOM_TM_REF_FIELDS() \
Expand Down
21 changes: 9 additions & 12 deletions include/dxc/Test/CompilationResult.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <atomic>

#include "dxc/Support/WinIncludes.h"
#include "dxc/Support/microcom.h"

#include "dxc/dxcapi.h"
#include "dxc/dxcisense.h"
Expand Down Expand Up @@ -49,14 +50,15 @@ inline HRESULT GetFirstChildFromCursor(IDxcCursor *cursor,
return hr;
}

class TrivialDxcUnsavedFile final : IDxcUnsavedFile
class TrivialDxcUnsavedFile : public IDxcUnsavedFile
{
private:
volatile std::atomic<llvm::sys::cas_flag> m_dwRef;
DXC_MICROCOM_REF_FIELD(m_dwRef)
LPCSTR m_fileName;
LPCSTR m_contents;
unsigned m_length;
public:
DXC_MICROCOM_ADDREF_RELEASE_IMPL(m_dwRef)
TrivialDxcUnsavedFile(LPCSTR fileName, LPCSTR contents)
: m_dwRef(0), m_fileName(fileName), m_contents(contents)
{
Expand All @@ -68,13 +70,8 @@ class TrivialDxcUnsavedFile final : IDxcUnsavedFile
CComPtr<TrivialDxcUnsavedFile> pNewValue = new TrivialDxcUnsavedFile(fileName, contents);
return pNewValue.QueryInterface(pResult);
}
ULONG STDMETHODCALLTYPE AddRef() { return (ULONG)++m_dwRef; }
ULONG STDMETHODCALLTYPE Release() {
ULONG result = (ULONG)--m_dwRef;
if (result == 0) delete this;
return result;
}
HRESULT STDMETHODCALLTYPE QueryInterface(REFIID iid, void** ppvObject)

HRESULT STDMETHODCALLTYPE QueryInterface(REFIID iid, void** ppvObject) override
{
if (ppvObject == nullptr) return E_POINTER;
if (IsEqualIID(iid, __uuidof(IUnknown)) ||
Expand All @@ -88,19 +85,19 @@ class TrivialDxcUnsavedFile final : IDxcUnsavedFile

return E_NOINTERFACE;
}
HRESULT STDMETHODCALLTYPE GetFileName(LPSTR* pFileName)
HRESULT STDMETHODCALLTYPE GetFileName(LPSTR* pFileName) override
{
*pFileName = (LPSTR)CoTaskMemAlloc(1 + strlen(m_fileName));
strcpy(*pFileName, m_fileName);
return S_OK;
}
HRESULT STDMETHODCALLTYPE GetContents(LPSTR* pContents)
HRESULT STDMETHODCALLTYPE GetContents(LPSTR* pContents) override
{
*pContents = (LPSTR)CoTaskMemAlloc(m_length + 1);
memcpy(*pContents, m_contents, m_length + 1);
return S_OK;
}
HRESULT STDMETHODCALLTYPE GetLength(unsigned* pLength)
HRESULT STDMETHODCALLTYPE GetLength(unsigned* pLength) override
{
*pLength = m_length;
return S_OK;
Expand Down
2 changes: 1 addition & 1 deletion lib/DxcSupport/FileIOHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
// Alias for CP_UTF16LE, which is the only one we actually handle.
#define CP_UTF16 CP_UTF16LE

struct HeapMalloc final : public IMalloc {
struct HeapMalloc : public IMalloc {
public:
ULONG STDMETHODCALLTYPE AddRef() override { return 1; }
ULONG STDMETHODCALLTYPE Release() override { return 1; }
Expand Down
2 changes: 1 addition & 1 deletion lib/DxcSupport/WinFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ unsigned char _BitScanForward(unsigned long * Index, unsigned long Mask) {
return 1;
}

struct CoMalloc final : public IMalloc {
struct CoMalloc : public IMalloc {
CoMalloc() : m_dwRef(0) {};

DXC_MICROCOM_ADDREF_RELEASE_IMPL(m_dwRef)
Expand Down
7 changes: 3 additions & 4 deletions tools/clang/tools/dxclib/dxc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ int DxcContext::VerifyRootSignature() {
}
}

class DxcIncludeHandlerForInjectedSources final : public IDxcIncludeHandler {
class DxcIncludeHandlerForInjectedSources : public IDxcIncludeHandler {
private:
DXC_MICROCOM_REF_FIELD(m_dwRef)

Expand Down Expand Up @@ -708,16 +708,15 @@ void DxcContext::Recompile(IDxcBlob *pSource, IDxcLibrary *pLibrary,
IFT(pPdbUtils->GetEntryPoint(&pEntryPoint));

CComPtr<IDxcBlobEncoding> pCompileSource;
DxcIncludeHandlerForInjectedSources *pIncludeHandlerForInjectedSources = new DxcIncludeHandlerForInjectedSources();
CComPtr<IDxcIncludeHandler> pIncludeHandler = pIncludeHandlerForInjectedSources;
CComPtr<DxcIncludeHandlerForInjectedSources> pIncludeHandler = new DxcIncludeHandlerForInjectedSources();
UINT32 uSourceCount = 0;
IFT(pPdbUtils->GetSourceCount(&uSourceCount));
for (UINT32 i = 0; i < uSourceCount; i++) {
CComPtr<IDxcBlobEncoding> pSourceFile;
CComBSTR pFileName;
IFT(pPdbUtils->GetSource(i, &pSourceFile));
IFT(pPdbUtils->GetSourceName(i, &pFileName));
IFT(pIncludeHandlerForInjectedSources->insertIncludeFile(pFileName, pSourceFile, 0));
IFT(pIncludeHandler->insertIncludeFile(pFileName, pSourceFile, 0));
if (pMainFileName == pFileName) {
pCompileSource.Attach(pSourceFile);
}
Expand Down
6 changes: 4 additions & 2 deletions tools/clang/tools/libclang/dxcisenseimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ HRESULT CreateDxcIntelliSense(_In_ REFIID riid, _Out_ LPVOID* ppv) throw()

// This is exposed as a helper class, but the implementation works on
// interfaces; we expect callers should be able to use their own.
class DxcBasicUnsavedFile final : public IDxcUnsavedFile
class DxcBasicUnsavedFile : public IDxcUnsavedFile
{
private:
DXC_MICROCOM_TM_REF_FIELDS()
Expand Down Expand Up @@ -581,7 +581,9 @@ HRESULT DxcBasicUnsavedFile::Create(
HRESULT hr = newValue->Initialize(fileName, contents, contentLength);
if (FAILED(hr))
{
delete newValue;
CComPtr<IMalloc> pTmp(newValue->m_pMalloc);
newValue->DxcBasicUnsavedFile::~DxcBasicUnsavedFile();
pTmp->Free(newValue);
return hr;
}
newValue->AddRef();
Expand Down
2 changes: 1 addition & 1 deletion tools/clang/unittests/HLSL/CompilerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ using namespace std;
using namespace hlsl_test;


class TestIncludeHandler final : public IDxcIncludeHandler {
class TestIncludeHandler : public IDxcIncludeHandler {
DXC_MICROCOM_REF_FIELD(m_dwRef)
public:
DXC_MICROCOM_ADDREF_RELEASE_IMPL(m_dwRef)
Expand Down
4 changes: 2 additions & 2 deletions tools/clang/unittests/HLSL/ExtensionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ class IntrinsicTable {
}
};

class TestIntrinsicTable final : public IDxcIntrinsicTable {
class TestIntrinsicTable : public IDxcIntrinsicTable {
private:
DXC_MICROCOM_REF_FIELD(m_dwRef)
std::vector<IntrinsicTable> m_tables;
Expand Down Expand Up @@ -395,7 +395,7 @@ class TestIntrinsicTable final : public IDxcIntrinsicTable {
// and defines that should cause warnings. A more realistic validator
// would look at the values and make sure (for example) they are
// the correct type (integer, string, etc).
class TestSemanticDefineValidator final : public IDxcSemanticDefineValidator {
class TestSemanticDefineValidator : public IDxcSemanticDefineValidator {
private:
DXC_MICROCOM_REF_FIELD(m_dwRef)
std::vector<std::string> m_errorDefines;
Expand Down

0 comments on commit 322b181

Please sign in to comment.