add-override-specifier example tool

Hi everyone,

I’d like to submit my refactoring tool that adds the override specifier to appropriate virtual functions to the clang tooling branch. Given the recent discussion on this list about creating a larger refactoring tool with many different C++11 refactoring options I thought I would submit it here for discussion before submitting it to cfe-commit list.

Regardless of the discussion on the larger tool, I think that the add-override-specifier example is still a useful example of the work that has been done in the tooling branch. It includes writing a custom ASTMatcher to identify virtual functions which are candidates for the override keyword. It also demonstrates a different problem than the rename-method example and remove-cstr-calls tool on which it is based. If there isn’t any opposition to this, I will submit it to cfe-commit.

I am still not entirely happy with the technique used to identify where the override keyword should be inserted and would welcome discussion on this problem.

Thanks,
Phil

add-override-specifier_example.patch (7.42 KB)

Again, with a few bug fixes…

Phil

add-override-specifier_example.patch (7.7 KB)

Again, with a few bug fixes…

Phil

Philip Dunstan
phil@philipdunstan.com
www.philipdunstan.com

Hi everyone,

I’d like to submit my refactoring tool that adds the override specifier to appropriate virtual functions to the clang tooling branch. Given the recent discussion on this list about creating a larger refactoring tool with many different C++11 refactoring options I thought I would submit it here for discussion before submitting it to cfe-commit list.

Cool stuff :slight_smile: Feel free to check this into the tooling branch. Before it goes anywhere else it would obviously need tests…

Regardless of the discussion on the larger tool, I think that the add-override-specifier example is still a useful example of the work that has been done in the tooling branch. It includes writing a custom ASTMatcher to identify virtual functions which are candidates for the override keyword. It also demonstrates a different problem than the rename-method example and remove-cstr-calls tool on which it is based. If there isn’t any opposition to this, I will submit it to cfe-commit.

We’ll need to figure out a place where to put those tools. At the moment the AST matchers are not in mainline yet.

I am still not entirely happy with the technique used to identify where the override keyword should be inserted and would welcome discussion on this problem.

I think currently the best way to do this is to re-lex those parts (cc’ed djasper, who has more hands on experience with that).

A different question is how much information we want to store in the AST to make it easier for tools to figure out the answers to questions like “where is the right place to put attributes for this function declaration?”.

Cheers,
/Manuel

I haven't had a chance to look at this in great detail, but at a
glance it doesn't seem to cover an idea I had, so I'll mention it
here:

Should you consider removing explicit 'virtual' from functions you're
marking up as 'override' (since the latter implies the former) -
perhaps as an option? (I can understand some people might like the
code layout/obviousness of having 'virtual' at the start of every
declaration that's virtual - overriden or otherwise)

[also - see my comment on Sam Panzer's thread regarding migration
tools & ongoing validators/autoformatters which seems somewhat
relevant]

- David