charmplusplus / charm

The Charm++ parallel programming system. Visit https://charmplusplus.org/ for more information.
Apache License 2.0
202 stars 49 forks source link

C++ cleanup of code formerly compiled as C #1898

Open stwhite91 opened 6 years ago

stwhite91 commented 6 years ago

Original issue: https://charm.cs.illinois.edu/redmine/issues/1898


Now that we compile machine and parts of Converse as C++, we can clean up some parts of the code using templates, STL containers, optional arguments to functions, typed enums, etc.

The following is a procedure for renaming .c files to .C.

  1. git mv the file(s) in question.
    
    $ git mv convcore.c convcore.C
2. Change directory to the `src` subfolder of Charm.

$ pwd /Users/evan/charm/src

3. Make a temporary commit containing just the rename and nothing else.

$ git commit -am "A 1" [charm 020ef6f6d] A 1 1 file changed, 0 insertions(+), 0 deletions(-) rename src/conv-core/{convcore.c => convcore.C} (100%)

4. Run the following one-liner to replace all instances of yourfile.c with yourfile.C. This will only work correctly if the temporary commit from step 3 has been made. This command assumes GNU `grep`, `sed`, and `awk`. On Darwin, use Homebrew to install these (`brew install grep awk gnu-sed`), and prefix `grep` and `sed` with `g` (i.e. `ggrep`, `gsed`), or `alias sed=gsed` and `alias grep=ggrep` to save some trouble.

$ for i in git diff --name-only HEAD~1 | sed 's/\.C/.c/' | sed 's,/, ,g' | awk '{print $NF}'; do ( grep -rFlw "$i" | xargs sed -i "s,${i%.c}\.c,${i%.c}.C,g" ); done

5. Make a temporary commit containing only these changes.

$ git commit -am "B 1" [charm c516df959] B 1 13 files changed, 34 insertions(+), 34 deletions(-)

6. Review the diff of the new commit to remove any changes that should not have been made. For example, if renaming isomalloc.c to isomalloc.C, the command will also change memory-isomalloc.c to memory-isomalloc.C. `git gui` is exceedingly useful here. Select the "Amend Last Commit" radio button, click on each file one by one, and for any change that should not be present, highlight the line(s) in question, right-click, and "Unstage Lines From Commit". When you're done, select all files in the top-left pane, open the Commit toolbar item, and "Revert Changes". Press the "Commit" button in the center-bottom of the window when this is done.
7. Some files are frequently referenced in comments that also have trailing whitespace. We can fix this in batch for all lines touched in the second temporary commit. _Do not apply this command to a commit containing a file rename, or else the entire file will have its whitespace fixed, generating a very large amount of diff noise._

$ git rebase --whitespace=fix HEAD~1 Current branch charm is up to date, rebase forced. First, rewinding head to replay your work on top of it... Applying: B 1


8. Try building `LIBS`. If there are any issues that need fixing, do so and commit as "C 1".
9. When the rename is fully functional, you can use an interactive rebase to squash "A 1", "B 1", and "C 1" (if present) together. This will need to be done before pushing the change, but it does not need to be done right now if you are doing further work in the patch sequence.
10. If you are renaming a file from C to C++, you are probably trying to use a C++ feature in it. You can now make the changes you need on top of the prior commit(s). _Do not squash your functional change onto the commit that renames the file!_ If during your work you find that you need to rename additional files, which is a common occurence if you are using C++ in a header, you can repeat steps 1-9, and if you chose not to squash the A, B, and C commits together, you can name them "A 2", "B 2", etc. It is perfectly fine, and recommended, to squash multiple file renames into one commit.
stwhite91 commented 5 years ago

Original date: 2018-05-08 02:26:40


Also std::atomic rather than compiler builtins like _sync*

ericjbohm commented 5 years ago

Original date: 2018-05-09 15:59:07


Assigning to Evan as I think he's already accomplished most of this task.

ericjbohm commented 5 years ago

Original date: 2018-05-09 16:00:08


We should probably break this down into a few different subtasks based on the categorical changes left. Some of the really mechanical stuff could be assigned to interns.

evan-charmworks commented 5 years ago

Original date: 2018-05-09 16:01:24


Eric Bohm wrote:

Assigning to Evan as I think he's already accomplished most of this task.

The work I've done is only enough to compile the source as C++ without errors or warnings with our current settings. Sam is talking about leveraging C++ to refactor the code.

evan-charmworks commented 5 years ago

Original date: 2018-05-10 20:42:37


Added C to C++ renaming tutorial.

evan-charmworks commented 1 year ago

Anything else that could benefit from cleanup made possible by C++ and not C?