Turns FileData into a C++-style class (phase 1 of #1369) #1375
Conversation
|
@qarkai @caclark I tried to include the minimum number of commits that still keeps mechanical changes separate from changes that actually need more thought to review. Also, sadly, the 2-step copy procedure (from https://devblogs.microsoft.com/oldnewthing/20190919-00/?p=102904 ) is the only way I could find to retain file history across a copy, and it takes up 3 commits 😢 This is a big change, but I've tried to make sure everything is being run through tests and static analysis, and that the sequence of changes is fairly logical and easy to understand/review. |
|
@xsdg - or anyone else... Sorry, I have more problems with git. This pull request conflicts with the latest Russian translation. Thanks |
Usually author of PR should resolve conflicts and update PR. |
|
If I run
I do not understand why the error did not show when the pull request tests ran on GitHub. |
You're probably using a newer version of clang. This check was added very recently: (Incidentally, this is one of the big downsides of things that behave like |
This is required to maintain history for both branches of the file.
This is required to maintain history for both branches of the file.
…n in src/filedata.cc
This trampoline translates C-style API calls into C++-style API calls. It was generated using the following steps:
1) Creates trampoline functions for void FileData methods that take a FileData as the first argument.
`ruby -pi -e '$_.gsub!(%r{^(void ([a-zA-Z_]+)\(FileData \*([a-z]+)(.*)\));}) {"#$1\n{\n\t#$3->#$2(#$3#$4);\n}\n"}' filedata.cc`
2) Creates trampoline functions for non-void FileData methods that take a FileData as the first argument.
`ruby -pi -e '$_.gsub!(%r{^(([a-zA-Z_ *]+?) ([a-zA-Z_]+)\(FileData \*([a-z]+)(.*)\));}) {"#$1\n{\n\treturn #$4->#$3(#$4#$5);\n}\n"}' filedata.cc`
3) Cleans up types that were copied into method calls:
`ruby -pi -e '$_.gsub!(%r{(?<=,)(?:[^,]* \*?)?([a-zA-Z_]+)(?=[,)])}) {" #$1"} if $_ =~ /^\t.*;$/' filedata.cc`
4) Creates trampoline functions for void methods that don't take a FileData as the first arg. (Along with auto-type-cleanup)
`ruby -pi -e '$_.gsub!(%r{^(void ([a-zA-Z_]+)\((.*)\));}) {whole=$1; fxn=$2; args=$3.gsub(%r{(?<=^|,)(?:[^,]* \**)?([a-zA-Z_]+)(?=,|$)}){" #$1"}; "#{whole}\n{\n\tFileData::#{fxn}(#{args});\n}\n"}' filedata.cc`
5) Creates trampoline functions for non-void methods that don't take a FileData as the first arg. (Along with auto-type-cleanup)
`ruby -pi -e '$_.gsub!(%r{^([a-zA-Z_]+ \**([a-zA-Z_]+)\((.*)\));}) {whole=$1; fxn=$2; args=$3.gsub(%r{(?<=^|,)(?:[^,]* \**)?([a-zA-Z_]+)(?=,|$)}){" #$1"}; "#{whole}\n{\n\treturn FileData::#{fxn}(#{args});\n}\n"}' filedata.cc`
6) Cleanup spurious space following open-paren from prior steps.
`perl -pi -e 's|\( |(|' filedata.cc`
Manual fixups Drops header-file contents Drops file_data_ref #defines fd-> to FileData:: for filelist_sort_compare_filedata nullptr checks for functions that had nullptr checks
Adds `static` specifier to FileData class functions that don't receive a `FileData` argument TODO: A future cleanup should get rid of all of the "_list" functions that are basically a foreach. Adds private declarations for methods marked "_unused" TODO: Should probably drop these, but that's also not in-scope for this refactor.
…ned method that was undeclared This aligns the header file method declarations and the implementation file definitions for the FileData class.
This necessary because you can't call a non-static method on the current object directly from a static method. The "::" forces the method call to resolve to the flat C API instead of the C++ class API. The real fix is to switch to the `fd->method()` calling pattern, but this is a smaller change for now, and the calling pattern transition will happen later.
…atic. As the refactor progresses, these methods will take more advantage of being class methods instead of just static class functions. This warning won't be meaningful until that has been done.
…or file_data_(un)ref Also, drops the file_data_(un)ref_debug name in favor of keeping the name identical (with only arguments changing) between the debug and non-debug implementations. This extension is supported by clang since at least 9.0.0, and gcc since at least 6.1 (and is also supported by msvc, even though that's not a relevant target for geeqie).
This also implicitly runs tests and static analysis against src/filedata/filedata.cc
|
Rebased to pick up ru.po update. Should patch cleanly now. |
| @@ -74,7 +70,11 @@ struct FileDataChangeInfo { | |||
| gboolean regroup_when_finished; | |||
| }; | |||
|
|
|||
| struct FileData { | |||
| class FileData { | |||
There was a problem hiding this comment.
Reminder for the next PR. Convert forward declarations from struct FileData to class FileData.
See #1369 for more context, but at a high level, this is a step towards being able to use C++-style fine-grained access control on the class methods, functions, and members to clearly define a public FileData API, as well as internal APIs between the different FileData subcomponents. Furthermore, this will allow us to test the internal implementations (especially around refcounting) without making those internal implementations part of the public API.