Conversation
Changed code style for whole solution.
There is circular dependency error.
…n-experiments # Conflicts: # applications/reflection-experiments/src/reflection_parsers/ast_source_parser.cpp # applications/reflection-experiments/src/reflection_parsers/ast_source_parser.h
Still circular issue.
| rsl::cli_parser cmdl(argv); | ||
|
|
||
| // any particular reason for unordered_set? | ||
| std::unordered_set<std::string> folders; |
There was a problem hiding this comment.
I still think there's no reason for this to be an unordered_set. unordered_set does not keep its data contiguous. iteration is thus more likely to run into cache misses than iterating a dyanmic_array/vector. while push_back on a dynamic array is also O(1). an unordered_set is mostly for low complexity lookup of whether an object is contained in the set or not.
| } | ||
|
|
||
| for (size_t i = 0; i < cmdl.size(); ++i) | ||
| std::cout << i << ": [" << cmdl[i] << "]\n"; |
There was a problem hiding this comment.
I'm personally not a huge fan of scopes without braces anymore tbh. it's pretty easy to accidentaly intoduce bugs with them
| generated_path = std::filesystem::path(generate_folder) / compile_file.name.data(); | ||
|
|
||
| rsl::dynamic_string final = get_gen_source_file( | ||
| rsl::dynamic_string::from_buffer( |
There was a problem hiding this comment.
This should probably be a string_view instead. no need to dynamically allocate a string and copy over the data from the filesystem::path
|
|
||
| // Probably should consider different identifier, as name can be repeated in different namespaces. | ||
| std::string extracted_name = extract_name(compile_file.name.data()); | ||
| file << "void register_reflection_file_" << extracted_name << "()\n"; |
There was a problem hiding this comment.
you could just do file << "void register_reflection_file_" << extract_name(compile_file.name.data()) << "()\n";
The compiler would probably optimize extracted_name away anyways, but currently the compiler is forced to actually create the std::string on the stack in debug mode.
by putting the extract_name function call inline you don't create a named variable. and thus the std::string can be created in-place in the file write stream operator call in debug as well.
| file << "#include \"runtime_reflection_containers.h\"\n"; | ||
| file << "#include \"impl/reflection_id/reflection_id.h\"\n"; | ||
| file << "#include \"impl/reflection_context/reflection_registration_registry.h\"\n"; | ||
| file << "#include \"impl/reflection_context/reflection_context.h\"\n"; |
There was a problem hiding this comment.
you can do:
file << "#include \"runtime_reflection_containers.h\"\n"
"#include \"impl/reflection_id/reflection_id.h\"\n"
"#include \"impl/reflection_context/reflection_registration_registry.h\"\n"
"#include \"impl/reflection_context/reflection_context.h\"\n";
| file << "};\n\n"; | ||
|
|
||
| file << "struct reflection_file_registration_helper\n"; | ||
| file << "{\n"; | ||
| file << " reflection_file_registration_helper()\n"; | ||
| file << " {\n"; | ||
| file << " reflection_registration_registry::instance().add(®ister_reflection_file_" << | ||
| extracted_name << ");\n"; | ||
| file << " }\n"; | ||
| file << "};\n" << "static reflection_file_registration_helper registration_instance;"; |
There was a problem hiding this comment.
file << "};\n"
"\n"
"struct reflection_file_registration_helper\n"
"{\n"
" reflection_file_registration_helper()\n";
" {\n";
" reflection_registration_registry::instance().add(®ister_reflection_file_" <<
extracted_name << ");\n"
" }\n"
"};\n"
"static reflection_file_registration_helper registration_instance;";
| char buffer[256]; | ||
| std::memcpy(buffer, source_location.data(), source_location.size()); | ||
| buffer[source_location.size()] = '\0'; | ||
|
|
||
| char* last_dot = std::strrchr(buffer, '.'); | ||
| if(last_dot != nullptr) { *last_dot = '\0'; } | ||
|
|
||
| strcat_s(buffer, "_generated.cpp"); | ||
|
|
||
| return rsl::dynamic_string::from_buffer(buffer, std::strlen(buffer) + 1); |
There was a problem hiding this comment.
using rsl::operator""_sv;
source_location = source_location.subview(0ull, rsl::reverse_linear_search(source_location, '.'));
return rsl::dynamic_string::from_view(source_location) + "_generated.cpp"_sv
| void reflection_code_generator::generate_reflected_variable( | ||
| std::ofstream& file, | ||
| const compile_reflected_variable& variable, | ||
| const std::string& parent_name) |
There was a problem hiding this comment.
this could've been an std::string_view instead
| file << " " << generate_reflection_id(variable.id, variable_name).data(); | ||
| file << " " << variable_name << ".offset = " << variable.offset << ";\n"; | ||
| file << " " << variable_name << ".type_spelling = " << | ||
| "rsl::dynamic_string::from_string_length(\"" << variable.type_spelling.data() << "\");\n"; |
There was a problem hiding this comment.
you could use "rsl::dynamic_string::from_array(\"" instead. because C++ will implicitly compile string literals as arrays of char
|
|
||
| file << " runtime_reflected_class " << class_var << ";\n"; | ||
| file << " " << generate_reflection_id(cls.id, class_var).data(); | ||
| file << " " << class_var << ".type_spelling = rsl::dynamic_string::from_string_length(\"" << |
There was a problem hiding this comment.
same here dynamic_string::from_array
| const compile_reflected_class& variable, | ||
| [[maybe_unused]] const std::string& parent_name); | ||
|
|
||
| std::string generate_reflection_id(reflection_id id, const std::string& owner) const; |
There was a problem hiding this comment.
if a string& is const, then it can be a string_view instead. a const string& requires the call site to have an adress to an existing string object, a string_view just needs a char* no matter in what kind of representation it might be stored or used at the call site.
| [[nodiscard]] const std::vector<std::unique_ptr<compile_reflected_class>>& | ||
| get_class_container() const; |
There was a problem hiding this comment.
This function name would logically to me return a compile_reflection_container<compile_reflected_class> so maybe get_sub_classes would be a better name.
It's probably also better to return a std::span<const std::unique_ptr<compile_reflected_class>> instead if you want to give read access without giving implementation details.
2 participants
No description provided.