Skip to content

Fixed interpret to C++ #68

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
uselessgoddess wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fixed interpret to C++ #68

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bfed46e
(Для начала стоит конечно проверить работоспособность, что у меня не …
uselessgoddess Feb 7, 2021
e5bc170
(Для начала стоит конечно проверить работоспособность, что у меня не …
uselessgoddess Feb 7, 2021
ca2585f
В принципе всё
uselessgoddess Feb 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions cpp/Platform.Disposables/Disposable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
namespace Platform::Disposables
#ifndef DISPOSABLES_DISPOSABLE_H
#define DISPOSABLES_DISPOSABLE_H

#include <functional>

#include "DisposableBase.h"
#include "Disposal.h"

namespace Platform::Disposables
{
template <typename ...> class Disposable;
template<> class Disposable<> : public DisposableBase
Expand All @@ -22,22 +30,22 @@

public: Disposable() { OnDispose = _emptyDelegate; }

public: Disposable(std::function<void()> action) : Disposable(action) { }

public: Disposable(std::function<Disposal> disposal) : Disposable(disposal) { }

protected: void Dispose(bool manual, bool wasDisposed) override { this->RaiseOnDisposeEvent(manual, wasDisposed); }

protected: void RaiseOnDisposeEvent(bool manual, bool wasDisposed) { this->OnDispose(manual, wasDisposed); }

public: template <typename T> static bool TryDisposeAndResetToDefault(T* object)
{
auto result = object.TryDispose();
auto result = object->TryDispose();
if (result)
{
object = 0;
Copy link

@poul250 poul250 Mar 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point of nulling a local variable? I'm sure the compiler will even cut out this piece of dead code.

Copy link
Member

@konard konard Mar 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In C#, it forces an object to be a candidate for garbage collection if no one else reference it. It also prevents the object from being used again (a safety from stupid mistakes). Translation is incorrect here, should be *object = nullptr and argument should be T** object

}
return result;
}
};
}
}


#endif
73 changes: 50 additions & 23 deletions cpp/Platform.Disposables/DisposableBase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,27 @@
namespace Platform::Disposables
#ifndef DISPOSABLES_DISPOSABLE_BASE_H
#define DISPOSABLES_DISPOSABLE_BASE_H

#include <stack>




#include "IDisposable.h"
#include "EnsureExtensions.h"
#include "../../../Exceptions/cpp/Platform.Exceptions/IgnoredExceptions.h"
#include "../../../Exceptions/cpp/Platform.Exceptions/ExceptionExtensions.h"
#include "../../../Exceptions/cpp/Platform.Exceptions/Ensure.h"


namespace Platform::Disposables
{
class DisposableBase : public IDisposable
{
private: static readonly ConcurrentStack<WeakReference<DisposableBase>> _disposablesWeekReferencesStack = ConcurrentStack<WeakReference<DisposableBase>>();
private: static std::stack<std::weak_ptr<DisposableBase>> _disposablesWeekReferencesStack;

private: volatile std::int32_t _disposed;
private: volatile std::atomic<std::int32_t> _disposed;

public: bool IsDisposed()
public: bool IsDisposed() override
{
return _disposed > 0;
}
Expand All @@ -26,29 +41,34 @@
return false;
}

static DisposableBase() { std::atexit(OnProcessExit); }
Copy link
Member

@konard konard Feb 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove static constructor? OnProcessExit method should be called once per all instances of DisposableBase.

Copy link
Member Author

@uselessgoddess uselessgoddess Feb 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I probably should have commented on that.
There are no static constructors in C++

But you can do it like this: https://stackoverflow.com/questions/1197106/static-constructors-in-c-i-need-to-initialize-private-static-objects

Copy link
Member

@konard konard Feb 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you can add an inner class with StaticInitializer name. This class should contain only constructor. And it should be immediately embedded as a static field.


protected: DisposableBase()
public: DisposableBase()
{
std::shared_ptr<DisposableBase> this_ptr = std::shared_ptr<DisposableBase>(this);
Copy link

@poul250 poul250 Mar 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bad and not working solution. If we create an object of this class on the stack, then after exiting the constructor, its destructor will be called, and the program will crash due to std::terminate, because we will try to free memory on the stack (invalid operation). If We create an object in the form std::make_shared<T>(), then we will have two different counters for the number of pointers, which will lead to the fact that we will try to free memory in the heap twice (invalid operation).
I suggest doing the following:

  1. Additionally inherit this class from std::enable_shared_from_this
class DisposableBase : public std::enable_shared_from_this<DisposableBase>, public IDisposable { /*...*/};
  1. In constructor safely get weak_ptr
auto this_weak = weak_from_this();

std::weak_ptr<DisposableBase> this_weak = std::weak_ptr<DisposableBase>(this_ptr);


_disposed = 0;
_disposablesWeekReferencesStack.Push(WeakReference<DisposableBase>(this, false));
_disposablesWeekReferencesStack.push(this_weak);
std::atexit(OnProcessExit);
}

~DisposableBase() { Destruct(); }
//TODO: завязывайте с вызовом всякой виртуальщины в конструкторе/деструкторе
//~DisposableBase() { Destruct(); }
Copy link
Member

@konard konard Feb 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we cannot call virtual methods in destructor?

Copy link
Member Author

@uselessgoddess uselessgoddess Feb 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because clang says "Inspection' Virtual call from constructor or destructor' "

In addition, you can look at these two codes:
https://rextester.com/PUOW20883 (С++)
https://rextester.com/TWL32777 (C#)

In principle, this problem can be easily solved, but I'm not sure that it will be in the style of C# Disposables
For example, something like this:

class DisposableWrapper {
    DisposableBase* disposableObject;
    
public: 
    
    DisposableWrapper(DisposableBase* disposableObject) : disposableObject(disposableObject) {}
    
    ~DisposableWrapper() {
        if(disposableObject != null) 
            disposableObject->Dispose(...);
        else 
            .....
            
        delete disposableObject;
    }
};

Copy link
Member

@konard konard Feb 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, we can skip it at the moment, and force child implementations to call Destruct method in the destructor. Anyway I think we can still call the Destruct method in the destructor, but we also should do it in any implementation of DisposableBase. We also should make base destructor virtual to make child destructor able to run.

Copy link
Member Author

@uselessgoddess uselessgoddess Feb 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to figure it out


public: virtual void Dispose(bool manual, bool wasDisposed) = 0;

protected: virtual void Dispose(bool manual, bool wasDisposed) = 0;

public: void Dispose()
public: void Dispose() override
{
this->Dispose(true);
GC.SuppressFinalize(this);
}

public: void Destruct()
public: void Destruct() override
{
try
{
if (!IsDisposed)
if (!IsDisposed())
{
this->Dispose(false);
}
Expand All @@ -59,30 +79,37 @@
}
}

protected: virtual void Dispose(bool manual)
public: virtual void Dispose(bool manual)
{
auto originalDisposedValue = Interlocked.CompareExchange(ref _disposed, 1, 0);
int compare_value = 1;
bool originalDisposedValue = _disposed.compare_exchange_weak(compare_value, 0);
auto wasDisposed = originalDisposedValue > 0;
if (wasDisposed && !AllowMultipleDisposeCalls && manual)
if (wasDisposed && !AllowMultipleDisposeCalls() && manual)
{
Platform::Disposables::EnsureExtensions::NotDisposed(Platform::Exceptions::Ensure::Always, this, ObjectName, "Multiple dispose calls are not allowed. Override AllowMultipleDisposeCalls property to modify behavior.");
Platform::Disposables::EnsureExtensions::NotDisposed(Platform::Exceptions::Ensure::Always, this, ObjectName(), "Multiple dispose calls are not allowed. Override AllowMultipleDisposeCalls property to modify behavior.");
}
if (AllowMultipleDisposeAttempts || !wasDisposed)
if (AllowMultipleDisposeAttempts() || !wasDisposed)
{
this->Dispose(manual, wasDisposed);
}
}

private: static void OnProcessExit()
{
while (_disposablesWeekReferencesStack.TryPop(out WeakReference<DisposableBase> weakReference))
while (!_disposablesWeekReferencesStack.empty())
{
if (weakReference.TryGetTarget(out DisposableBase disposable))
auto weakReference = _disposablesWeekReferencesStack.top();
_disposablesWeekReferencesStack.pop();

if (auto disposable = weakReference.lock())
{
GC.SuppressFinalize(disposable);
disposable.Destruct();
disposable->Destruct();
}
}
}
};
}
}

std::stack<std::weak_ptr<Platform::Disposables::DisposableBase>> Platform::Disposables::DisposableBase::_disposablesWeekReferencesStack = std::stack<std::weak_ptr<DisposableBase>>();
Copy link
Member

@konard konard Feb 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this line complied with no errors?

Copy link
Member Author

@uselessgoddess uselessgoddess Feb 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, static fields in C++ leave much to be desired. This seems to be solved via 'Instance'.
But it works. As my favorite singer sings: "Wet flesh, good size"

Copy link
Member

@konard konard Feb 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a template class library, and we use C#-like style. So, please, move this initialization inside class definition scope.

Copy link
Member Author

@uselessgoddess uselessgoddess Feb 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In C++, you can't initialize any static fields inside a class. In an ideal implementation, this should generally be done in a file .cpp of the following format:

    //in the CLASS.cpp file

    #include "CLASS.h"

    /*type*/ CLASS::static_field = ...;

Copy link
Member

@konard konard Mar 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible if you use inline keyword, look at the example.


#endif
22 changes: 17 additions & 5 deletions cpp/Platform.Disposables/Disposable[TPrimary, TAuxiliary].h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
namespace Platform::Disposables
#ifndef DISPOSABLES_DISPOSABLE_T1_T2_H
#define DISPOSABLES_DISPOSABLE_T1_T2_H

#include "Disposal.h"
#include "Disposable[T].h"

namespace Platform::Disposables
{
template <typename ...> class Disposable;
Copy link
Member

@konard konard Feb 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove default template root?


template <typename TPrimary, typename TAuxiliary> class Disposable<TPrimary, TAuxiliary> : public Disposable<TPrimary>
{
public: const TAuxiliary AuxiliaryObject;
public: Platform::Delegates::MulticastDelegate<Disposal> OnDispose;
public: const TPrimary Object;

public: Disposable(TPrimary object, TAuxiliary auxiliaryObject, std::function<void(TPrimary, TAuxiliary)> action)
: Disposable<TPrimary>(object)
Expand All @@ -18,11 +26,11 @@
};
}

public: Disposable(TPrimary object, TAuxiliary auxiliaryObject, std::function<void()> action) : Disposable<TPrimary>(object, action) { return AuxiliaryObject = auxiliaryObject; }
public: Disposable(TPrimary object, TAuxiliary auxiliaryObject, std::function<void()> action) : Disposable<TPrimary>(object, action) { AuxiliaryObject = auxiliaryObject; }

public: Disposable(TPrimary object, TAuxiliary auxiliaryObject, Disposal disposal) : Disposable<TPrimary>(object, disposal) { return AuxiliaryObject = auxiliaryObject; }
public: Disposable(TPrimary object, TAuxiliary auxiliaryObject, Disposal disposal) : Disposable<TPrimary>(object, disposal) { AuxiliaryObject = auxiliaryObject; }

public: Disposable(TPrimary object, TAuxiliary auxiliaryObject) : Disposable<TPrimary>(object) { return AuxiliaryObject = auxiliaryObject; }
public: Disposable(TPrimary object, TAuxiliary auxiliaryObject) : Disposable<TPrimary>(object) { AuxiliaryObject = auxiliaryObject; }

public: Disposable(TPrimary object) : Disposable<TPrimary>(object) { }

Expand All @@ -44,5 +52,9 @@
AuxiliaryObject.TryDispose();
Object.TryDispose();
}


};
}

#endif
27 changes: 21 additions & 6 deletions cpp/Platform.Disposables/Disposable[T].h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
namespace Platform::Disposables
#ifndef DISPOSABLES_DISPOSABLE_T_H
#define DISPOSABLES_DISPOSABLE_T_H


#include "Disposal.h"
#include "Disposable.h"


namespace Platform::Disposables
{
template <typename ...> class Disposable;
Copy link
Member

@konard konard Feb 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove default template root?


template <typename T> class Disposable<T> : public Disposable<>
{
public: const T Object;
Expand All @@ -17,9 +25,9 @@
};
}

public: Disposable(T object, std::function<void()> action) : Disposable<>(action) { return Object = object; }
public: Disposable(T object, std::function<void()> action) : Disposable<>(action) { Object = object; }

public: Disposable(T object, Disposal disposal) : Disposable<>(disposal) { return Object = object; }
public: Disposable(T object, Disposal disposal) : Disposable<>(disposal) { Object = object; }

public: Disposable(T object) { Object = object; }

Expand All @@ -33,8 +41,15 @@

protected: void Dispose(bool manual, bool wasDisposed) override
{
base.Dispose(manual, wasDisposed);
// о нет так можно делать только в visual c++
// __super::Dispose(manual, wasDisposed)
// ----------------------------------
// base.Dispose(manual, wasDisposed);
Copy link
Member

@konard konard Feb 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can try to use Disposable<>::Dispose(manual, wasDisposed); here.


RaiseOnDisposeEvent(manual, wasDisposed);
Object.TryDispose();
}
};
}
}

#endif
7 changes: 6 additions & 1 deletion cpp/Platform.Disposables/Disposal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
namespace Platform::Disposables
#ifndef DISPOSABLES_DISPOSAl_H
#define DISPOSABLES_DISPOSAl_H

namespace Platform::Disposables
{
using Disposal = void(bool, bool);
}

#endif
22 changes: 14 additions & 8 deletions cpp/Platform.Disposables/EnsureExtensions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,29 @@
namespace Platform::Disposables
#ifndef DISPOSABLES_ENSURE_EXTENSION_H
#define DISPOSABLES_ENSURE_EXTENSION_H


namespace Platform::Disposables
{
class EnsureExtensions
{
public: static void NotDisposed(Platform::Exceptions::ExtensionRoots::EnsureAlwaysExtensionRoot root, IDisposable &disposable, std::string objectName, std::string message)
public: static void NotDisposed(Platform::Exceptions::ExtensionRoots::EnsureAlwaysExtensionRoot root, IDisposable* disposable, std::string objectName, std::string message)
Copy link
Member

@konard konard Feb 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you change the reference to pointer here?

Copy link
Member Author

@uselessgoddess uselessgoddess Feb 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can fix this

Copy link
Member Author

@uselessgoddess uselessgoddess Feb 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot fix

Copy link
Member

@konard konard Feb 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Member Author

@uselessgoddess uselessgoddess Feb 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Because any class with undefined virtual methods(virtual ... foo (...) = 0;) can only be a pointer. IDisposable is exactly like this

Copy link
Member

@konard konard Mar 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did an experiment. I found that it is possible to pass abstract class by reference.

It was based on an idea from Stack Overflow, which I found using С++ abstract class by reference search request in Google.

{
if (disposable.IsDisposed)
if (disposable->IsDisposed())
{
throw std::runtime_error(std::string("Attempt to access disposed object [").append(objectName).append("]: ").append(message).append(1, '.'));
}
}

public: static void NotDisposed(Platform::Exceptions::ExtensionRoots::EnsureAlwaysExtensionRoot root, IDisposable &disposable, std::string objectName) { NotDisposed(root, disposable, objectName, {}); }
public: static void NotDisposed(Platform::Exceptions::ExtensionRoots::EnsureAlwaysExtensionRoot root, IDisposable* disposable, std::string objectName) { NotDisposed(root, disposable, objectName, {}); }

public: static void NotDisposed(Platform::Exceptions::ExtensionRoots::EnsureAlwaysExtensionRoot root, IDisposable &disposable) { NotDisposed(root, disposable, {}, {}); }
public: static void NotDisposed(Platform::Exceptions::ExtensionRoots::EnsureAlwaysExtensionRoot root, IDisposable* disposable) { NotDisposed(root, disposable, {}, {}); }

public: static void NotDisposed(Platform::Exceptions::ExtensionRoots::EnsureOnDebugExtensionRoot root, IDisposable &disposable, std::string objectName, std::string message) { Platform::Disposables::EnsureExtensions::NotDisposed(Platform::Exceptions::Ensure::Always, disposable, objectName, message); }
public: static void NotDisposed(Platform::Exceptions::ExtensionRoots::EnsureOnDebugExtensionRoot root, IDisposable* disposable, std::string objectName, std::string message) { Platform::Disposables::EnsureExtensions::NotDisposed(Platform::Exceptions::Ensure::Always, disposable, objectName, message); }

public: static void NotDisposed(Platform::Exceptions::ExtensionRoots::EnsureOnDebugExtensionRoot root, IDisposable &disposable, std::string objectName) { Platform::Disposables::EnsureExtensions::NotDisposed(Platform::Exceptions::Ensure::Always, disposable, objectName, {}); }
public: static void NotDisposed(Platform::Exceptions::ExtensionRoots::EnsureOnDebugExtensionRoot root, IDisposable* disposable, std::string objectName) { Platform::Disposables::EnsureExtensions::NotDisposed(Platform::Exceptions::Ensure::Always, disposable, objectName, {}); }

public: static void NotDisposed(Platform::Exceptions::ExtensionRoots::EnsureOnDebugExtensionRoot root, IDisposable &disposable) { Platform::Disposables::EnsureExtensions::NotDisposed(Platform::Exceptions::Ensure::Always, disposable, {}, {}); }
public: static void NotDisposed(Platform::Exceptions::ExtensionRoots::EnsureOnDebugExtensionRoot root, IDisposable* disposable) { Platform::Disposables::EnsureExtensions::NotDisposed(Platform::Exceptions::Ensure::Always, disposable, {}, {}); }
};
}

#endif
19 changes: 14 additions & 5 deletions cpp/Platform.Disposables/GenericObjectExtensions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,25 @@
namespace Platform::Disposables
#ifndef DISPOSABLES_GENERIC_DISPOSABLE_H
#define DISPOSABLES_GENERIC_DISPOSABLE_H

namespace Platform::Disposables
{
class GenericObjectExtensions
{
public: template <typename T> static bool TryDispose(T object)
{
try
{
if (object is DisposableBase disposableBase)
if (std::is_base_of<T, DisposableBase>::value) // до изменения задания: if(disposableBase = (DisposableBase*)object)
{
disposableBase.DisposeIfNotDisposed();
auto disposableBase = (DisposableBase*)object;

//TODO: add this method
//disposableBase->DisposeIfNotDisposed();
}
else if (object is System::IDisposable &disposable)
else if (std::is_base_of<T, IDisposable>::value) // до изменения задания: if(disposable = (IDisposable*)object)
{
disposable.Dispose();
auto disposable = (IDisposable*)object;
disposable->Dispose();
}
return true;
}
Expand All @@ -26,3 +33,5 @@
public: template <typename T> static void DisposeIfPossible(T object) { TryDispose(object); }
};
}

#endif
12 changes: 10 additions & 2 deletions cpp/Platform.Disposables/IDisposable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
namespace Platform::Disposables
#ifndef DISPOSABLES_IDISPOSABLE_H
#define DISPOSABLES_IDISPOSABLE_H

#include "System.IDisposable.h"


namespace Platform::Disposables
{
class IDisposable : public System::IDisposable
{
Expand All @@ -7,4 +13,6 @@

virtual void Destruct() = 0;
};
Copy link
Member

@konard konard Mar 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Abstract virtual destructor should be added here and in all other abstract classes (including System::IDisposable).

See also linksplatform/Interfaces#56

}
}

#endif
11 changes: 9 additions & 2 deletions cpp/Platform.Disposables/IDisposableExtensions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
namespace Platform::Disposables
#ifndef DISPOSABLES_IDISPOSABLE_EXTENSIONS_H
#define DISPOSABLES_IDISPOSABLE_EXTENSIONS_H
Copy link
Member

@konard konard Feb 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment there is no need to use header guards in each class file. The template library is included in one piece as a cpp/Platform.Disposables/Platform.Disposables.h file.




namespace Platform::Disposables
{
class IDisposableExtensions
{
public: static void DisposeIfNotDisposed(IDisposable &disposable)
{
if (!disposable.IsDisposed)
if (!disposable.IsDisposed())
{
disposable.Dispose();
}
}
};
}

#endif
Loading