Refureku
Refureku copied to clipboard
Add `getCanonicalTypeName()` for Field and Variable types.
Hello. As I described in one of my issues I needed something like this, so I've spend some time to dig into the source code and I think I'm close to implementing this feature.
This PR adds getCanonicalTypeName()
for VariableBase
class which means that both Field
and Variable
will have this function. Canonical type name is taken from kodgen::TypeInfo
class and added to generated files. Since non reflected types can be considered equal (while in reality it may be incorrect) I've put canonical type name to VariableBase
instead of Type
.
Here is an example:
#pragma once
#include <string>
#include <vector>
#include "Foo.generated.h"
using namespace std;
class CLASS() Foo{
public:
Foo() = default;
virtual ~Foo() = default;
FIELD()
const vector<string> myValue;
FIELD()
string myString;
Foo_GENERATED
};
File_Foo_GENERATED
For myValue
this function will return std::vector<std::basic_string<char>>
and for myString
it will be std::basic_string<char>
.
Due to the lack of knowledge of this project I was unable to compile RefurekuTests
target, got lots of errors like:
TestClass.rfks.h(23, 232): [C2259] 'UniqueInheritedProperty': cannot instantiate abstract class (compiling source file D:\Downloads\Refureku\Refureku\Library\Tests\Src\TestClass.cpp)
I've regenerated reflection for test files but was still getting these errors. Maybe you need to regenerate reflection in some special way?
Also, I've noticed that if you don't include <vector>
type name for myValue
will not be std::vector<std::basic_string<char>>
but int
and the same for string field if you don't include <string>
. So when shouldLogDiagnostic
is true
we can see:
...
[Warning] ...h:16:18: error: use of undeclared identifier 'string'
[Warning] ...h:19:5: error: unknown type name 'string'
[Warning] ...h:21:5: error: unknown type name 'Node_GENERATED'
...
I think we should detect actual errors (like use of undeclared identifier 'string'
) and stop code generation and ignore errors like unknown type name 'Node_GENERATED'
. For this I'm proposing this PR.
Hello.
Thank you for your contribution, I'm a bit busy right now but will look into it ASAP.
I've fixed tests target compilation, I had to remove Generated directory so that it will be regenerated using the new generator.
The whole point of this PR is do get type information for non-reflected types, but because Type
can incorrect for non-reflected types (for example match
ing std::string
and std::vector
will be true
) I'm adding this functionality to variables instead of types. Although we can 100% work on improving this and eventually move this to the Type
but right now this will solve my issue and allow me to continue development on my project that uses Refureku.
I will work on requested changes and will change the target branch to dev
.
Also, because I see lots of code-style mistakes that I made, have you though about using clang-format and/or clang-tidy for code-style? With them developers don't need to care about code-style because clang tools will do all of the formatting automatically (for example, on Ctrl+S press and/or in CI).
I consider this PR done. Although I also want you to merge my other PR.
The whole point of this PR is do get type information for non-reflected types
I get your point here, but it sounds off in the first place wanting to get reflection data from an unreflected class. If you want to get reflection data, you should reflect your class in the first place, and it's a design decision on your side not wanting to do so.
I will merge this to dev and think about an optimal solution before merging to master. Having these changes pushed to master and then modified afterwards will break the library API/ABI which I want to avoid. I should be able to work with your forked version until then.
have you though about using clang-format and/or clang-tidy for code-style?
I know it exists but have never taken the time to look into it yet, will definitely have to do so if it can handle all code-style for us. I wonder how far it can go.