Add readability-redundant-void-arg check to clang-tidy

[LegalizeAdulthood]

Summary

This check for clang-tidy looks for function with zero arguments declared as (void) and removes the unnecessary void token.

int foo(void);

becomes

int foo();

The check performs no formatting of the surrounding context but uses the lexer to look for the token sequence "(", "void", ")" in the prototype text. If this sequence of tokens is found, a removal is issued for the void token only.

[LegalizeAdulthood]

Test

[xazax.hun]

Hi!

As far as I know this is not a valid transformation for C. I think you should check lang options whether the processed file is compiled in C or in C++ mode. (At least I did not find the check at the first glance.)

Regards,
Gábor

clang-tidy/readability/RemoveVoidArg.cpp

169 ↗

(On Diff #19961)

I think in the LLVM codebase consts tend to be written before the type and not after.

[LegalizeAdulthood]

In article <[e-mail]>,

Gábor Horváth <[e-mail]> writes:

As far as I know this is not a valid transformation for C. I think you
should check lang options whether the processed file is compiled in C or in
C++ mode. (At least I did not find the check at the first glance.)

Will do.

[alexfh]

Sorry for the long delay. Here are some initial comments. It will be easier to review once you reformat the code and place consts in front (or remove, if needed) to make the code closer to the LLVM style.

clang-tidy/readability/RemoveVoidArg.cpp

170 ↗

(On Diff #19961)

1. The "Redundant void argument list in" part is repeated everywhere. Maybe just pass the description of the location (here - "typedef") as an argument to removeVoidArgumentTokens?

2. Clang-tidy messages should use the same style as Clang diagnostics: start with a lower-case letter, and no trailing period.

clang-tidy/readability/RemoveVoidArg.h

26 ↗

(On Diff #19961)

Formatting is off in many places. Could you clang-format the code so I don't need to point to every nit?

38 ↗

(On Diff #19961)

Here and below: I'm against top-level const in function parameter types. It's just an implementation detail whether the method can modify its parameter. It doesn't make sense to pollute the interface with this.

Also, it's more common in Clang/LLVM code to put the const related to the pointed/referenced object in front:

void processFunctionDecl(const ast_matchers::MatchFinder::MatchResult &Result, const FunctionDecl *Function);

test/clang-tidy/readability-remove-void-arg.cpp

28 ↗

(On Diff #19961)

Repeating the whole message each time makes the test harder to read. Please leave the whole message once and shorten all other occurrences, e.g.:

// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: {{.*}} in function declaration

145 ↗

(On Diff #19961)

It would be more readable with a single regular expression:

{{^    \($}}

[sbenza]

clang-tidy/readability/RemoveVoidArg.cpp

65 ↗

(On Diff #19961)

The callbacks are not cheap. They generate string copies and then rerun the tokenizer on them.
These matchers should be more restricted to what we are looking for to avoid running the callbacks on uninteresting nodes.

78 ↗

(On Diff #19961)

why not auto in this one? You used it on all other cases below.

174 ↗

(On Diff #19961)

Any reason all of these convert the StringRef into a std::string?
Couldn't they work with the StringRef directly?

clang-tidy/readability/RemoveVoidArg.h

38 ↗

(On Diff #19961)

Any reason why these are members?
Making them free functions in the .cpp file would avoid filling the header with these implementation details.
It would also remove the need to include Lexer.h here.

[LegalizeAdulthood]

clang-tidy/readability/RemoveVoidArg.cpp

65 ↗

(On Diff #19961)

So... I need to write an expression that matches not just a pointer to function but a pointer to function with zero arguments? I'll play with clang-query and see what I can figure out.

I managed to get some narrowing for everything but typedefDecl; I couldn't find any narrowing matchers for it.

174 ↗

(On Diff #19961)

I tried switching it to a StringRef, but when I tried to combine it with other text to build a replacement for diag(), it got turned into Twine and then failed to match any signature for diag(). I'm open to suggestions for what you'd like to see instead.

clang-tidy/readability/RemoveVoidArg.h

26 ↗

(On Diff #19961)

I just assumed a final pass through clang-format before being committed; I haven't gone through CLion trying to make it follow clang's/llvm's indent rules.

38 ↗

(On Diff #19961)

They're members because ultimately on detection they ultimately call diag which is a member function. Otherwise, I would have made them free functions.

[LegalizeAdulthood]

Thanks for all the really great feedback. This addresses all the issues except for checking if the translation unit is C instead of C++. I will add that and update the review.

[sbenza]

clang-tidy/readability/RemoveVoidArg.cpp

65 ↗

(On Diff #19961)

It doesn't have to be a perfect matcher, but it should try to reject the most common false positives early.
Just by saying that it is a functionType() you get most of the varDecls out of the way without going into the callback.
This should be good for now.

174 ↗

(On Diff #19961)

Twine::str() constructs the string.
I don't mind constructing a string when a match happens. Those are rare.
But we should avoid allocating memory when no match happens.
Right now temporary memory allocations are a considerable part of clang-tidy's runtime. We should try to minimize that.

clang-tidy/readability/RemoveVoidArg.h

38 ↗

(On Diff #19961)

They could still be free functions, that return some data instead of taking action directly.
But I understand now.

[LegalizeAdulthood]

clang-tidy/readability/RemoveVoidArg.cpp

174 ↗

(On Diff #19961)

I think we are at that stage now; I do all early-out testing before constructing the replacement string.

[LegalizeAdulthood]

clang-tidy/readability/RemoveVoidArg.h

38 ↗

(On Diff #19961)

They'd essentially have to return all the arguments in this statement:

diag(VoidLoc, Diagnostic) << FixItHint::CreateRemoval(VoidRange);

...which feels tortured just to avoid creating member functions on the implementation.

[sbenza]

clang-tidy/readability/RemoveVoidArg.cpp

174 ↗

(On Diff #19961)

You are still generating a copy of the code before re-parsing it.
Those will happen for anything that passes the matcher, even if they have no 'void' argument.
removeVoidArgumentTokens should take the code as a StringRef directly from getText(), without converting to std::string first. GrammarLocation should also be a StringRef.
Basically, avoid std::string until you are about to call diag().

[LegalizeAdulthood]

clang-tidy/readability/RemoveVoidArg.cpp

174 ↗

(On Diff #19961)

I'll look into having getText return the StringRef and using that. I don't understand the fuss over GrammarLocation, however. Before I had it as a constant string that was just passed through and then another review wanted the duplication in the message eliminated, so I pulled that out. Now you're saying that the string joining induced by the elimination of duplication is bad. It doesn't seem possible to satisfy both comments.

[sbenza]

clang-tidy/readability/RemoveVoidArg.cpp

174 ↗

(On Diff #19961)

The problem with GrammarLocation is that it is declared as a 'const std::string &' and you are always passing literals.
This is what StringRef is for. It will avoid the unnecessary temporary string.
The signature would read like:

void RemoveVoidArg::removeVoidArgumentTokens(
    const MatchFinder::MatchResult &Result, SourceLocation StartLoc,
    StringRef DeclText, StringRef GrammarLocation);

Then you can construct the Lexer with an std::string, like:

clang::Lexer PrototypeLexer(StartLoc, Result.Context->getLangOpts(),
                            DeclText.data(), DeclText.data(),
                            DeclText.data() + DeclText.length());

[LegalizeAdulthood]

clang-tidy/readability/RemoveVoidArg.cpp

174 ↗

(On Diff #19961)

Thanks for the explanation. I'll try to take a look at this tonight. There is similar code in some of the other reviews I have posted, so I'll fold that into those reviews as well.

[alexfh]

clang-tidy/readability/RemoveVoidArg.cpp

105 ↗

(On Diff #20580)

I think, the "const" here makes the code less readable and doesn't add any value as the variable is only visible to a one-line statement. Same for the other conditions.

207 ↗

(On Diff #20580)

Please remove top-level const in the method definitions as well.

[LegalizeAdulthood]

clang-tidy/readability/RemoveVoidArg.cpp

174 ↗

(On Diff #19961)

I tried this and it almost works :-). StringRef doesn't put a NUL at the end and the lexer needs that:

clang-tidy: /llvm/tools/clang/lib/Lex/Lexer.cpp:63: void clang::Lexer::InitLexer(const char *, const char *, const char *): Assertion `BufEnd[0] == 0 && "We assume that the input buffer has a null character at the end" " to simplify lexing!"' failed.

So it looks like I still need to create a std::string right before lexing.

[LegalizeAdulthood]

Switched to using StringRef where possible.
Use QualType to narrow matches further to avoid unnecessary lexing.

[LegalizeAdulthood]

How do I get access to whether or not the translation unit is a C file or a C++ file?

I looked in LangOptions and LangOptionsBase, but I couldn't find anything there.

[xazax.hun]

I think langopts have a C99 field. Maybe something like "if (langopts.C99) return;".

[LegalizeAdulthood]

Ensure that only C++ files are processed.

[xazax.hun]

clang-tidy/readability/RemoveVoidArg.cpp

73 ↗

(On Diff #20949)

As far as I can remember when the code compiled in e.g.: CPlusPlus14 mode, the CPlusPlus11 and CPlusPlus flags are also turned on. It might be enough to check CPlusPlus.

[alexfh]

clang-tidy/readability/RemoveVoidArg.cpp

164 ↗

(On Diff #20949)

s/const std::string&/StringRef/, s/const StringRef/StringRef/

176 ↗

(On Diff #20949)

s/const auto/std::string/

186 ↗

(On Diff #20949)

You don't need to check for both kw_void and raw_identifier + "void". You won't see the former unless you resolve the identifier and set the token kind yourself.

219 ↗

(On Diff #20949)

Please use StringRef instead of auto here (see the other code review thread for an explanation).

227 ↗

(On Diff #20949)

StringRef

241 ↗

(On Diff #20949)

s/const auto/StringRef/

255 ↗

(On Diff #20949)

s/const auto/SourceLocation/

267 ↗

(On Diff #20949)

auto

clang-tidy/readability/RemoveVoidArg.h

38 ↗

(On Diff #20949)

1. You don't need to store this flag as it's easy and cheap to check it on each call to check(): it's available as Result.Context->getLangOpts().CPlusPlus.
2. This name violates LLVM Naming conventions: http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

[sbenza]

clang-tidy/readability/RemoveVoidArg.cpp

42 ↗

(On Diff #20949)

Remove the duplicate logic.
Could be something like:

if (const auto PT = QT->getAs<PointerType>()) {
  QT = PT->getPointeeType();
}
if (const auto *MPT = QT->getAs<MemberPointerType>()) {
  QT = MPT->getPointeeType();
}
if (const auto FP = QT->getAs<FunctionProtoType>()) {
    return FP->getNumParams() == 0;
}

[LegalizeAdulthood]

clang-tidy/readability/RemoveVoidArg.h

38 ↗

(On Diff #20949)

Thanks for showing me where I could get that option. I drilled around in the code for a while and couldn't find it except hanging off the Compiler. What would be really nice is if I could get ahold of that flag when I'm asked to add matchers as I could simply not add any matchers in that case, but registerPPCallbacks is called after registerMatchers. Can I navigate to it from ClangTidyContext?

[LegalizeAdulthood]

clang-tidy/readability/RemoveVoidArg.cpp

164 ↗

(On Diff #20949)

DeclText needs to stay a std::string or lexing fails.

[LegalizeAdulthood]

Fussing with string types.
Rename check to readability-redundant-void-arg
Get C++ flag when check is called.

[LegalizeAdulthood]

Simplify check for nullary function.

[xazax.hun]

clang-tidy/readability/RedundantVoidArg.cpp

148 ↗

(On Diff #21084)

You can also pass std::string to an llvm::StringRef. In the llvm codebase it is a rule of thumb to use StringRef instead of const std::string&.

158 ↗

(On Diff #21084)

It is possible to create a lexer without creating an std::string. You can check an example in: http://reviews.llvm.org/D7375 (look for getBuffer and getCharacterData). Of course, this way you have to make sure to end lexing after the interested position is left, but you avoid the heap allocation.

[alexfh]

clang-tidy/readability/RedundantVoidArg.cpp

41 ↗

(On Diff #21084)

Please use either "auto" or "const auto *" consistently. This function has usages of both.

clang-tidy/readability/RemoveVoidArg.h

38 ↗

(On Diff #20949)

Language options only become available when a compiler instance is being initialized. The matchers are registered way before that and then multiple different compilations may be run potentially with different options (if multiple translation units have been passed to clang-tidy).

So there's no way to learn what language will be analyzed when we register matchers.

[LegalizeAdulthood]

clang-tidy/readability/RedundantVoidArg.cpp

41 ↗

(On Diff #21084)

Fixed

148 ↗

(On Diff #21084)

Fixed.

158 ↗

(On Diff #21084)

This suggestion has been made several times during this review and every time I try it I get this error at runtime:

void clang::Lexer::InitLexer(const char *, const char *, const char *): Assertion `BufEnd[0] == 0 && "We assume that the input buffer has a null character at the end" " to simplify lexing!"' failed.

If you can show me how to lex a SourceRange without getting that error, I'm happy to change it. I tried the method shown in the linked diff and I get the same error.

However, one good thing came from the experiment. This checker only creates fixits that remove text, so I don't need to compute replacement text and this simplified the replacement code a little bit more.

clang-tidy/readability/RemoveVoidArg.cpp

105 ↗

(On Diff #20580)

Fixed.

207 ↗

(On Diff #20580)

Fixed

42 ↗

(On Diff #20949)

Fixed.

73 ↗

(On Diff #20949)

Fixed.

176 ↗

(On Diff #20949)

Fixed.

186 ↗

(On Diff #20949)

Fixed.

219 ↗

(On Diff #20949)

Fixed.

227 ↗

(On Diff #20949)

Fixed

241 ↗

(On Diff #20949)

Fixed.

255 ↗

(On Diff #20949)

Fixed.

267 ↗

(On Diff #20949)

Fixed

65 ↗

(On Diff #19961)

I did find ways to reject more cases before beginning a lexical scan of text, so I'm opting out earlier than I was initially.

I couldn't find the right matcher incantation with clang-query to narrow it further in the matcher instead of in the callback.

Fixed.

78 ↗

(On Diff #19961)

Fixed.

169 ↗

(On Diff #19961)

Fixed

170 ↗

(On Diff #19961)

Fixed

174 ↗

(On Diff #19961)

Fixed.

clang-tidy/readability/RemoveVoidArg.h

38 ↗

(On Diff #20949)

Fixed.

26 ↗

(On Diff #19961)

Fixed.

38 ↗

(On Diff #19961)

const issues fixed.

test/clang-tidy/readability-remove-void-arg.cpp

28 ↗

(On Diff #19961)

Fixed

145 ↗

(On Diff #19961)

Fixed.

[LegalizeAdulthood]

Correct issues found in review.

[xazax.hun]

In D7639#136129, @LegalizeAdulthood wrote:

If you can show me how to lex a SourceRange without getting that error, I'm happy to change it. I tried the method shown in the linked diff and I get the same error.

As far as I know, the point is that you can not do that without creating a null terminated string, however you do not need to do that. What you can do insead: you can lex the whole file buffer starting from the beginning of a declaration and you can make sure that you finish lexing on the end of the declaration (e.g.: end lexing when you match the closing paren of the declaration). This way you might have a bit more complicated lexing logic but you avoid heap allocation. So the point is: you construct the lexer using the whole file buffer starting from the right position and not relying on the source range at all to finish lexing.

[LegalizeAdulthood]

In D7639#136280, @xazax.hun wrote:

In D7639#136129, @LegalizeAdulthood wrote:

If you can show me how to lex a SourceRange without getting that error, I'm happy to change it. I tried the method shown in the linked diff and I get the same error.

As far as I know, the point is that you can not do that without creating a null terminated string, however you do not need to do that. What you can do insead: you can lex the whole file buffer starting from the beginning of a declaration and you can make sure that you finish lexing on the end of the declaration (e.g.: end lexing when you match the closing paren of the declaration). This way you might have a bit more complicated lexing logic but you avoid heap allocation. So the point is: you construct the lexer using the whole file buffer starting from the right position and not relying on the source range at all to finish lexing.

Relex the entire file just to avoid a heap allocation? That seems a bit excessive. Do we have any measurements on real code bases that show this to be the better approach? I don't want to optimize like this without real data to show that it is worthwhile.

Most of the time this string is going to either be "()" or "(void)". Small string optimization means that there isn't even a heap allocation.

[xazax.hun]

In D7639#136296, @LegalizeAdulthood wrote:

In D7639#136280, @xazax.hun wrote:

In D7639#136129, @LegalizeAdulthood wrote:

If you can show me how to lex a SourceRange without getting that error, I'm happy to change it. I tried the method shown in the linked diff and I get the same error.

As far as I know, the point is that you can not do that without creating a null terminated string, however you do not need to do that. What you can do insead: you can lex the whole file buffer starting from the beginning of a declaration and you can make sure that you finish lexing on the end of the declaration (e.g.: end lexing when you match the closing paren of the declaration). This way you might have a bit more complicated lexing logic but you avoid heap allocation. So the point is: you construct the lexer using the whole file buffer starting from the right position and not relying on the source range at all to finish lexing.

Relex the entire file just to avoid a heap allocation? That seems a bit excessive. Do we have any measurements on real code bases that show this to be the better approach? I don't want to optimize like this without real data to show that it is worthwhile.

Most of the time this string is going to either be "()" or "(void)". Small string optimization means that there isn't even a heap allocation.

Well, you won't relex anything that does not need to be relexed. You only initialize the lexer using the whole buffer, so you get a null terminated string. But the starting point of the lexing would be the starting source location of the declaration you want to lex. And you would only lex the tokens you need. I think the lexer is lazy, so initializing with a bigger buffer should not cause any performance regressions in case you do not lex more tokens that you need. Of course measurements would tell this for sure.

[alexfh]

clang-tidy/readability/RedundantVoidArgCheck.cpp

14 ↗

(On Diff #21435)

nit: Remove this and just start namespace clang { (or all three of them) here instead of line 50 below. Then clang::ast_matchers can be replaced with ast_matchers as well.

146 ↗

(On Diff #21435)

nit: No need for clang:: here.

195 ↗

(On Diff #21435)

It's better to use token ranges here:

CharSourceRange::getTokenRange(VoidLoc, VoidLoc)

test/clang-tidy/readability-redundant-void-arg.cpp

86 ↗

(On Diff #21435)

Please use LLVM style in tests unless needed otherwise for testing whitespace handling: 2 character indentation, left brace on the previous line, etc.

[LegalizeAdulthood]

I have experimented with using typeLoc() matchers to just match the appropriate type expressions and that mostly works, but not completely. If anyone has more experience with typeloc() style matching and how to use it, I'd like to hear their thoughts on this.

[LegalizeAdulthood]

In D7639#136297, @xazax.hun wrote:

In D7639#136296, @LegalizeAdulthood wrote:

In D7639#136280, @xazax.hun wrote:

In D7639#136129, @LegalizeAdulthood wrote:

If you can show me how to lex a SourceRange without getting that error, I'm happy to change it. I tried the method shown in the linked diff and I get the same error.

As far as I know, the point is that you can not do that without creating a null terminated string, however you do not need to do that. What you can do insead: you can lex the whole file buffer starting from the beginning of a declaration and you can make sure that you finish lexing on the end of the declaration (e.g.: end lexing when you match the closing paren of the declaration). This way you might have a bit more complicated lexing logic but you avoid heap allocation. So the point is: you construct the lexer using the whole file buffer starting from the right position and not relying on the source range at all to finish lexing.

Relex the entire file just to avoid a heap allocation? That seems a bit excessive. Do we have any measurements on real code bases that show this to be the better approach? I don't want to optimize like this without real data to show that it is worthwhile.

Most of the time this string is going to either be "()" or "(void)". Small string optimization means that there isn't even a heap allocation.

Well, you won't relex anything that does not need to be relexed. You only initialize the lexer using the whole buffer, so you get a null terminated string. But the starting point of the lexing would be the starting source location of the declaration you want to lex. And you would only lex the tokens you need. I think the lexer is lazy, so initializing with a bigger buffer should not cause any performance regressions in case you do not lex more tokens that you need. Of course measurements would tell this for sure.

I'd like to address this in a subsequent changeset, since ti seems to be a matter of performance and not a matter of correctness with the existing code and I'd like to move this check forward.

[LegalizeAdulthood]

clang-tidy/readability/RedundantVoidArgCheck.cpp

14 ↗

(On Diff #21435)

Fixed.

146 ↗

(On Diff #21435)

Fixed.

195 ↗

(On Diff #21435)

Fixed.

test/clang-tidy/readability-redundant-void-arg.cpp

86 ↗

(On Diff #21435)

Fixed.

[LegalizeAdulthood]

Revise from review comments

[LegalizeAdulthood]

run trunk clang-format on source files

[alexfh]

In D7639#173909, @LegalizeAdulthood wrote:

In D7639#136297, @xazax.hun wrote:

In D7639#136296, @LegalizeAdulthood wrote:

In D7639#136280, @xazax.hun wrote:

In D7639#136129, @LegalizeAdulthood wrote:

If you can show me how to lex a SourceRange without getting that error, I'm happy to change it. I tried the method shown in the linked diff and I get the same error.

As far as I know, the point is that you can not do that without creating a null terminated string, however you do not need to do that. What you can do insead: you can lex the whole file buffer starting from the beginning of a declaration and you can make sure that you finish lexing on the end of the declaration (e.g.: end lexing when you match the closing paren of the declaration). This way you might have a bit more complicated lexing logic but you avoid heap allocation. So the point is: you construct the lexer using the whole file buffer starting from the right position and not relying on the source range at all to finish lexing.

Relex the entire file just to avoid a heap allocation? That seems a bit excessive. Do we have any measurements on real code bases that show this to be the better approach? I don't want to optimize like this without real data to show that it is worthwhile.

Most of the time this string is going to either be "()" or "(void)". Small string optimization means that there isn't even a heap allocation.

Well, you won't relex anything that does not need to be relexed. You only initialize the lexer using the whole buffer, so you get a null terminated string. But the starting point of the lexing would be the starting source location of the declaration you want to lex. And you would only lex the tokens you need. I think the lexer is lazy, so initializing with a bigger buffer should not cause any performance regressions in case you do not lex more tokens that you need. Of course measurements would tell this for sure.

I'd like to address this in a subsequent changeset, since ti seems to be a matter of performance and not a matter of correctness with the existing code and I'd like to move this check forward.

This looks like a very small and quick change, so it would be nice to address this concern in the very first version of the check.

clang-tidy/readability/RedundantVoidArgCheck.cpp

244 ↗

(On Diff #25928)

Character-based offsets can be brittle in case escaped newlines and alternative tokens are used. I'd prefer getting the type range directly when it's possible. Here and for other casts something like this should work: CStyleCast->getTypeInfoAsWritten()->getTypeLoc().getSourceRange().

Also, do we need separate functions for ExplicitCastExpr and two of its children classes?

test/clang-tidy/readability-redundant-void-arg.c

77 ↗

(On Diff #25928)

Special tests for whitespace handling seem to be superfluous when you just need to check that no changes are made. Also, the invariant "no warnings => no fixes" seems to be safe to rely on, so you can remove all CHECK-FIXES and add one "CHECK-MESSAGES-NOT: warning:" so that the script runs the FileCheck on the output.

[LegalizeAdulthood]

clang-tidy/readability/RedundantVoidArgCheck.cpp

244 ↗

(On Diff #25928)

I don't think the character based offsets can be avoided here. The nodes don't provide getter functions for all the locations I'm interested in and I have to construct the appropriate source location myself. I will try what you suggest and see how it goes.

I tried doing things with just ExplicitCastExpr initially and when I added test cases for the functional and C-style casts, I needed to do different replacement ranges.

[LegalizeAdulthood]

clang-tidy/readability/RedundantVoidArgCheck.cpp

244 ↗

(On Diff #25928)

I was able to avoid some of the custom SourceRange computations, which allowed me to eliminate shrinkRangeByOne as well.

test/clang-tidy/readability-redundant-void-arg.c

77 ↗

(On Diff #25928)

I tried this, but it doesn't work with the shell script used to run clang-tidy. You need to pass --allow-empty to FileCheck, but the script doesn't allow for this. So I left the file unchanged.

[LegalizeAdulthood]

Update from comments

[alexfh]

Something wrong happened with this patch: there are tons of unrelated changes here. Maybe you need to update your working copy?

clang-tidy/readability/RedundantVoidArgCheck.cpp

50 ↗

(On Diff #28202)

So why do we need isExpansionInMainFile() here and in all other matchers? ClangTidy has its own mechanism to filter warnings by location.

229 ↗

(On Diff #28202)

Now processNamedCastExpr, processNamedCastExpr and processFunctionalCast methods are almost identical and can be replaced by a single method. And I think this method should be processExplicitCastExpr that should have the same implementation.

BTW, currently, the processExplicitCastExpr method can only be called if the code encounters the only class inherited from ExplicitCastExpr that is not handled explicitly: ObjCBridgedCastExpr.

Once again: replace processExplicitCastExpr implementation with this:

void RedundantVoidArgCheck::processExplicitCastExpr(
    const MatchFinder::MatchResult &Result,
    const ExplicitCastExpr *ExplicitCast) {
  if (protoTypeHasNoParms(ExplicitCast->getTypeAsWritten())) {
    removeVoidArgumentTokens(
      Result,
      ExplicitCast->getTypeInfoAsWritten()->getTypeLoc().getSourceRange(),
       "cast expression");
  }
}

... and remove the other three process...Cast methods and the corresponding if branches.

Also, register a single explicitCastExpr matcher instead of separate matchers for all kinds of casts.

[danielmarjamaki]

sorry but I am personally skeptic about this checker.

why is the void removed?

it does not cause any wrong behaviour to keep it.

the void is not likely added there by mistake, is it? the developer probably wrote it by intention and this checker thinks that the developer intentions are wrong..

how about moving it to clang-modernize?

[LegalizeAdulthood]

Rebase onto HEAD of master

[LegalizeAdulthood]

In D7639#192668, @danielmarjamaki wrote:

sorry but I am personally skeptic about this checker.

why is the void removed?

it does not cause any wrong behaviour to keep it.

the void is not likely added there by mistake, is it? the developer probably wrote it by intention and this checker thinks that the developer intentions are wrong..

how about moving it to clang-modernize?

It is a readability check, it isn't designed to detect "wrong" behavior. (void) is a C-ism and is a holdover from C-style coding. It is completely redundant and unnecessary in C++. If a developer wants to keep unnecessary and redundant tokens in their code, then they can turn this check off or not run it.

[alexfh]

In D7639#193443, @LegalizeAdulthood wrote:

In D7639#192668, @danielmarjamaki wrote:

sorry but I am personally skeptic about this checker.

why is the void removed?

it does not cause any wrong behaviour to keep it.

the void is not likely added there by mistake, is it? the developer probably wrote it by intention and this checker thinks that the developer intentions are wrong..

how about moving it to clang-modernize?

It is a readability check, it isn't designed to detect "wrong" behavior. (void) is a C-ism and is a holdover from C-style coding. It is completely redundant and unnecessary in C++. If a developer wants to keep unnecessary and redundant tokens in their code, then they can turn this check off or not run it.

Agree. Also, we want to migrate clang-modernize transformations to clang-tidy some day (it's an extremely low priority task though). Wrt this check, it might fit better in a new module named "legacy". Not extremely important and definitely not necessary for this patch.

What's necessary, is to address my comments (http://reviews.llvm.org/D7639?id=28202#inline-85173 and http://reviews.llvm.org/D7639?id=28202#inline-85176).

[danielmarjamaki]

In D7639#193527, @alexfh wrote:

In D7639#193443, @LegalizeAdulthood wrote:

In D7639#192668, @danielmarjamaki wrote:

sorry but I am personally skeptic about this checker.

why is the void removed?

it does not cause any wrong behaviour to keep it.

the void is not likely added there by mistake, is it? the developer probably wrote it by intention and this checker thinks that the developer intentions are wrong..

how about moving it to clang-modernize?

It is a readability check, it isn't designed to detect "wrong" behavior. (void) is a C-ism and is a holdover from C-style coding. It is completely redundant and unnecessary in C++. If a developer wants to keep unnecessary and redundant tokens in their code, then they can turn this check off or not run it.

I know it is redundant in C++. And probably the developer knows it too? I expect that nearly 100% of the warnings will be false positives because as I said: "this checker thinks that the developer intentions are wrong.". Stylistic checkers are problematic because people have different taste so the warnings are seen by both those who wants to see it and those who don't, however in this case I have the feeling that your warning will only be seen by those who don't want to have the warning. I doubt people add void by mistake.

Does anybody know that some C++ developers use void because they think it's safer?

When you try this on real code.. what is the false positive ratio?

Agree. Also, we want to migrate clang-modernize transformations to clang-tidy some day (it's an extremely low priority task though). Wrt this check, it might fit better in a new module named "legacy". Not extremely important and definitely not necessary for this patch.

What's necessary, is to address my comments (http://reviews.llvm.org/D7639?id=28202#inline-85173 and http://reviews.llvm.org/D7639?id=28202#inline-85176).

I am afraid I don't understand. I had the impression that checkers should try to warn about stuff that is not intentional. Imho a warning about intentional code that does not do anything wrong is a false positive.

I can understand that some checkers could be highly useful in some projects but noisy in most projects. Can we try to separate these so they can be suppressed easily and/or must be enabled specifically? Instead of the other way around where most projects that get the warning must disable it manually. If many such checkers are added it will be a pain to disable them if they are not separated.

[danielmarjamaki]

Sorry... I have changed my mind. This checker is fine. You can ignore my comments.

[xazax.hun]

I think there are valid use cases for this checker. There are several C++ codebases that was converted from C, and half of the codebase contains the void keywords in the argument list half of them is not. Making such codebases consistent is a good thing. It makes it easier to grep for a function for instance.

[LegalizeAdulthood]

I know it is redundant in C++. And probably the developer knows it too? I expect that nearly 100% of the warnings will be false positives because as I said: "this checker thinks that the developer intentions are wrong.".

In clang-tidy discussions, "false positive" means that the check incorrectly analyzed the code and that the resulting application of an automated fix would change the code to syntactically incorrect or semantically incorrect. Under this definition, the only false positive would be a case where the fix was applied to C code, where (void) is necessary to distinguish a function with no arguments as opposed to a function with an unspecified number and type of arguments.

There are plenty of readability checks in clang-tidy that impose a certain point of view. A good example is the LLVM namespace comment check. Here, a style guideline is being imposed on the code. If you don't want your code subjected to that style guideline, then you shouldn't be running that check and applying the suggested fixes to your code. clang-tidy is not part of the compiler, nothing is forcing you to run clang-tidy and even if you use clang-tidy, nothing is forcing you to run this check. You only get what you ask for.

Stylistic checkers are problematic because people have different taste so the warnings are seen by both those who wants to see it and those who don't, however in this case I have the feeling that your warning will only be seen by those who don't want to have the warning. I doubt people add void by mistake.

I created this check because I was working on C++ teams where people kept bringing their C habits into the C++ code. This is purely a C-ism and people coming from C routinely bring this into their C++ code because they operate under the (erroneous IMO) opinion that "C++ is just C with classes".

Does anybody know that some C++ developers use void because they think it's safer?

There is no issue of safety here. In C, function prototypes are underspecified if they don't write (void). In C++, no such thing occurs.

When you try this on real code.. what is the false positive ratio?

Again, there are no "false positives" in the clang-tidy sense. I believe I have added sufficient C oriented test cases to cover that. If you don't want the results of this check, or any other clang-tidy check, applied to your code, then don't ask for it.

clang-tidy/readability/RedundantVoidArgCheck.cpp

50 ↗

(On Diff #28202)

The lexer crashes with an assert if I don't limit it to the main file. Why, I don't know, but that's what happens.

[LegalizeAdulthood]

clang-tidy/readability/RedundantVoidArgCheck.cpp

229 ↗

(On Diff #28202)

Fixed. When I took out the cStyleCast matcher, this test case fails:

/home/richard/dev/llvm/tools/clang/tools/extra/test/clang-tidy/readability-redundant-void-arg.cpp:348:19: error: expected string not found in input
  // CHECK-FIXES: void (*f4)() = (void (*)()) 0;{{$}}
                  ^
/home/richard/dev/llvm-build/tools/clang/tools/extra/test/clang-tidy/Output/readability-redundant-void-arg.cpp.tmp.cpp:341:3: note: scanning from here
  //
  ^
/home/richard/dev/llvm-build/tools/clang/tools/extra/test/clang-tidy/Output/readability-redundant-void-arg.cpp.tmp.cpp:345:3: note: possible intended match here
  void (*f4)() = (void (*)(void)) 0;
  ^

So I left in the matcher for cStyleCastExpr.

[LegalizeAdulthood]

Update from comments

[LegalizeAdulthood]

Improve documentation of check.

[Eugene.Zelenko]

What is preventing to add this check to Clang-tidy? Just found another piece of fresh C++ code in LLDB with (void) as argument list...

[LegalizeAdulthood]

In D7639#266332, @Eugene.Zelenko wrote:

What is preventing to add this check to Clang-tidy? Just found another piece of fresh C++ code in LLDB with (void) as argument list...

To be honest, I don't know. This review had taken SOOOOOOOOO long to get approved. At this point, the code is correct and I'd really like to get it committed and available for use instead of fussing with it any longer.

[LegalizeAdulthood]

Can we get this committed? I've addressed all comments.

[sbenza]

I'm sorry. This change fell through the cracks. I'll take a look today.

[sbenza]

Just fyi, I am looking at this diff. It is very large with a lot of rounds of comments and I didn't have the context.
I don't know if I should giving comments at this point of the change, but here it is.
Have you considered matching on typeLoc() instead of having a large list of different cases?
For example, if you match typeLoc(loc(functionType())) it will match all the places where a function type is mentioned, including things like static_cast<XXX>, variable declarations, lambda return type declarations, etc. Might help remove redundancy in the check.

[LegalizeAdulthood]

In D7639#275504, @sbenza wrote:

Just fyi, I am looking at this diff. It is very large with a lot of rounds of comments and I didn't have the context.
I don't know if I should giving comments at this point of the change, but here it is.
Have you considered matching on typeLoc() instead of having a large list of different cases?
For example, if you match typeLoc(loc(functionType())) it will match all the places where a function type is mentioned, including things like static_cast<XXX>, variable declarations, lambda return type declarations, etc. Might help remove redundancy in the check.

That occurred to me and I did an experiment and it didn't work out. I forget the exact details now as it was months ago and this review has been sitting here languishing with a correct implementation as-is. I really just want to get this committed and make incremental improvement, instead of re-evaluating the entire thing from scratch at this time.

[LegalizeAdulthood]

I mean, look at this review. I created it on Feb 13th 2015. It's been getting ground through the review process for 8 months. Why can't we move forward?

[sbenza]

Just wanted to know it was considered, and it was.
It looks good to me then.

[alexfh]

In D7639#276641, @LegalizeAdulthood wrote:

I mean, look at this review. I created it on Feb 13th 2015. It's been getting ground through the review process for 8 months. Why can't we move forward?

Yes, I know the long reviews may be annoying. And I apologize for missing the latest updates on this patch from June. Please feel free to ping the reviews when there are no updates on them for a few days.

There are still a few comments on this patch that were not completely addressed, but it seems fine to submit the patch at this stage and address the concerns after the patch is in.

I'll commit the patch for you.

clang-tidy/readability/RedundantVoidArgCheck.cpp

51 ↗

(On Diff #28641)

I'm pretty sure the lexer is able to work on header files, if used correctly. If you need help figuring out what's wrong, please provide more information: which assert fails, call stack, and a reduced test case where the assert fails.

[alexfh]

FYI, I had to update tests' RUN lines for some recent changes. I'm also going to move the check to the modernize module, where it belongs.

clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyModernizeModule
   LoopConvertCheck.cpp
   LoopConvertUtils.cpp
   MakeUniqueCheck.cpp
   ModernizeTidyModule.cpp
   PassByValueCheck.cpp
+  RedundantVoidArgCheck.cpp
   ReplaceAutoPtrCheck.cpp
   ShrinkToFitCheck.cpp
   UseAutoCheck.cpp
   UseDefaultCheck.cpp
   UseNullptrCheck.cpp
   UseOverrideCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyReadabilityModule
   clangTidyUtils
   )

clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp

 //===--- ModernizeTidyModule.cpp - clang-tidy -----------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "LoopConvertCheck.h"
 #include "MakeUniqueCheck.h"
 #include "PassByValueCheck.h"
+#include "RedundantVoidArgCheck.h"
 #include "ReplaceAutoPtrCheck.h"
 #include "ShrinkToFitCheck.h"
 #include "UseAutoCheck.h"
 #include "UseDefaultCheck.h"
 #include "UseNullptrCheck.h"
 #include "UseOverrideCheck.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace modernize {
 
 class ModernizeModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<LoopConvertCheck>("modernize-loop-convert");
     CheckFactories.registerCheck<MakeUniqueCheck>("modernize-make-unique");
     CheckFactories.registerCheck<PassByValueCheck>("modernize-pass-by-value");
+    CheckFactories.registerCheck<RedundantVoidArgCheck>(
+        "modernize-redundant-void-arg");
     CheckFactories.registerCheck<ReplaceAutoPtrCheck>(
         "modernize-replace-auto-ptr");
     CheckFactories.registerCheck<ShrinkToFitCheck>("modernize-shrink-to-fit");
     CheckFactories.registerCheck<UseAutoCheck>("modernize-use-auto");
     CheckFactories.registerCheck<UseDefaultCheck>("modernize-use-default");
     CheckFactories.registerCheck<UseNullptrCheck>("modernize-use-nullptr");
     CheckFactories.registerCheck<UseOverrideCheck>("modernize-use-override");
   }

clang-tools-extra/trunk/clang-tidy/modernize/RedundantVoidArgCheck.h

+//===--- RedundantVoidArgCheck.h - clang-tidy --------------------*- C++-*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REDUNDANT_VOID_ARG_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REDUNDANT_VOID_ARG_CHECK_H
+
+#include "../ClangTidy.h"
+#include "clang/Lex/Token.h"
+
+#include <string>
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// \brief Find and remove redundant void argument lists.
+///
+/// Examples:
+///   `int f(void);`                    becomes `int f();`
+///   `int (*f(void))(void);`           becomes `int (*f())();`
+///   `typedef int (*f_t(void))(void);` becomes `typedef int (*f_t())();`
+///   `void (C::*p)(void);`             becomes `void (C::*p)();`
+///   `C::C(void) {}`                   becomes `C::C() {}`
+///   `C::~C(void) {}`                  becomes `C::~C() {}`
+///
+class RedundantVoidArgCheck : public ClangTidyCheck {
+public:
+  RedundantVoidArgCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  void processFunctionDecl(const ast_matchers::MatchFinder::MatchResult &Result,
+                           const FunctionDecl *Function);
+
+  void processTypedefDecl(const ast_matchers::MatchFinder::MatchResult &Result,
+                          const TypedefDecl *Typedef);
+
+  void processFieldDecl(const ast_matchers::MatchFinder::MatchResult &Result,
+                        const FieldDecl *Member);
+
+  void processVarDecl(const ast_matchers::MatchFinder::MatchResult &Result,
+                      const VarDecl *Var);
+
+  void
+  processNamedCastExpr(const ast_matchers::MatchFinder::MatchResult &Result,
+                       const CXXNamedCastExpr *NamedCast);
+
+  void
+  processExplicitCastExpr(const ast_matchers::MatchFinder::MatchResult &Result,
+                          const ExplicitCastExpr *ExplicitCast);
+
+  void processLambdaExpr(const ast_matchers::MatchFinder::MatchResult &Result,
+                         const LambdaExpr *Lambda);
+
+  void
+  removeVoidArgumentTokens(const ast_matchers::MatchFinder::MatchResult &Result,
+                           SourceRange Range, StringRef GrammarLocation);
+
+  void removeVoidToken(Token VoidToken, StringRef Diagnostic);
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REDUNDANT_VOID_ARG_CHECK_H

clang-tools-extra/trunk/clang-tidy/modernize/RedundantVoidArgCheck.cpp

+//===- RedundantVoidArgCheck.cpp - clang-tidy -----------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "RedundantVoidArgCheck.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+
+namespace {
+
+// Determine if the given QualType is a nullary function or pointer to same.
+bool protoTypeHasNoParms(QualType QT) {
+  if (auto PT = QT->getAs<PointerType>()) {
+    QT = PT->getPointeeType();
+  }
+  if (auto *MPT = QT->getAs<MemberPointerType>()) {
+    QT = MPT->getPointeeType();
+  }
+  if (auto FP = QT->getAs<FunctionProtoType>()) {
+    return FP->getNumParams() == 0;
+  }
+  return false;
+}
+
+const char FunctionId[] = "function";
+const char TypedefId[] = "typedef";
+const char FieldId[] = "field";
+const char VarId[] = "var";
+const char NamedCastId[] = "named-cast";
+const char CStyleCastId[] = "c-style-cast";
+const char ExplicitCastId[] = "explicit-cast";
+const char LambdaId[] = "lambda";
+
+} // namespace
+
+namespace tidy {
+namespace modernize {
+
+void RedundantVoidArgCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(functionDecl(isExpansionInMainFile(), parameterCountIs(0),
+                                  unless(isImplicit()),
+                                  unless(isExternC())).bind(FunctionId),
+                     this);
+  Finder->addMatcher(typedefDecl(isExpansionInMainFile()).bind(TypedefId),
+                     this);
+  auto ParenFunctionType = parenType(innerType(functionType()));
+  auto PointerToFunctionType = pointee(ParenFunctionType);
+  auto FunctionOrMemberPointer =
+      anyOf(hasType(pointerType(PointerToFunctionType)),
+            hasType(memberPointerType(PointerToFunctionType)));
+  Finder->addMatcher(
+      fieldDecl(isExpansionInMainFile(), FunctionOrMemberPointer).bind(FieldId),
+      this);
+  Finder->addMatcher(
+      varDecl(isExpansionInMainFile(), FunctionOrMemberPointer).bind(VarId),
+      this);
+  auto CastDestinationIsFunction =
+      hasDestinationType(pointsTo(ParenFunctionType));
+  Finder->addMatcher(
+      cStyleCastExpr(isExpansionInMainFile(), CastDestinationIsFunction)
+          .bind(CStyleCastId),
+      this);
+  Finder->addMatcher(
+      cxxStaticCastExpr(isExpansionInMainFile(), CastDestinationIsFunction)
+          .bind(NamedCastId),
+      this);
+  Finder->addMatcher(
+      cxxReinterpretCastExpr(isExpansionInMainFile(), CastDestinationIsFunction)
+          .bind(NamedCastId),
+      this);
+  Finder->addMatcher(cxxConstCastExpr(isExpansionInMainFile(),
+                                   CastDestinationIsFunction).bind(NamedCastId),
+                     this);
+  Finder->addMatcher(lambdaExpr(isExpansionInMainFile()).bind(LambdaId), this);
+}
+
+void RedundantVoidArgCheck::check(const MatchFinder::MatchResult &Result) {
+  if (!Result.Context->getLangOpts().CPlusPlus) {
+    return;
+  }
+
+  const BoundNodes &Nodes = Result.Nodes;
+  if (const auto *Function = Nodes.getNodeAs<FunctionDecl>(FunctionId)) {
+    processFunctionDecl(Result, Function);
+  } else if (const auto *Typedef = Nodes.getNodeAs<TypedefDecl>(TypedefId)) {
+    processTypedefDecl(Result, Typedef);
+  } else if (const auto *Member = Nodes.getNodeAs<FieldDecl>(FieldId)) {
+    processFieldDecl(Result, Member);
+  } else if (const auto *Var = Nodes.getNodeAs<VarDecl>(VarId)) {
+    processVarDecl(Result, Var);
+  } else if (const auto *NamedCast =
+                 Nodes.getNodeAs<CXXNamedCastExpr>(NamedCastId)) {
+    processNamedCastExpr(Result, NamedCast);
+  } else if (const auto *CStyleCast =
+                 Nodes.getNodeAs<CStyleCastExpr>(CStyleCastId)) {
+    processExplicitCastExpr(Result, CStyleCast);
+  } else if (const auto *ExplicitCast =
+                 Nodes.getNodeAs<ExplicitCastExpr>(ExplicitCastId)) {
+    processExplicitCastExpr(Result, ExplicitCast);
+  } else if (const auto *Lambda = Nodes.getNodeAs<LambdaExpr>(LambdaId)) {
+    processLambdaExpr(Result, Lambda);
+  }
+}
+
+void RedundantVoidArgCheck::processFunctionDecl(
+    const MatchFinder::MatchResult &Result, const FunctionDecl *Function) {
+  SourceLocation Start = Function->getLocStart();
+  if (Function->isThisDeclarationADefinition()) {
+    SourceLocation BeforeBody =
+        Function->getBody()->getLocStart().getLocWithOffset(-1);
+    removeVoidArgumentTokens(Result, SourceRange(Start, BeforeBody),
+                             "function definition");
+  } else {
+    removeVoidArgumentTokens(Result, Function->getSourceRange(),
+                             "function declaration");
+  }
+}
+
+void RedundantVoidArgCheck::removeVoidArgumentTokens(
+    const ast_matchers::MatchFinder::MatchResult &Result, SourceRange Range,
+    StringRef GrammarLocation) {
+  std::string DeclText =
+      Lexer::getSourceText(CharSourceRange::getTokenRange(Range),
+                           *Result.SourceManager,
+                           Result.Context->getLangOpts()).str();
+  Lexer PrototypeLexer(Range.getBegin(), Result.Context->getLangOpts(),
+                       DeclText.data(), DeclText.data(),
+                       DeclText.data() + DeclText.size());
+  enum TokenState {
+    NothingYet,
+    SawLeftParen,
+    SawVoid,
+  };
+  TokenState State = NothingYet;
+  Token VoidToken;
+  Token ProtoToken;
+  std::string Diagnostic =
+      ("redundant void argument list in " + GrammarLocation).str();
+
+  while (!PrototypeLexer.LexFromRawLexer(ProtoToken)) {
+    switch (State) {
+    case NothingYet:
+      if (ProtoToken.is(tok::TokenKind::l_paren)) {
+        State = SawLeftParen;
+      }
+      break;
+    case SawLeftParen:
+      if (ProtoToken.is(tok::TokenKind::raw_identifier) &&
+          ProtoToken.getRawIdentifier() == "void") {
+        State = SawVoid;
+        VoidToken = ProtoToken;
+      } else {
+        State = NothingYet;
+      }
+      break;
+    case SawVoid:
+      State = NothingYet;
+      if (ProtoToken.is(tok::TokenKind::r_paren)) {
+        removeVoidToken(VoidToken, Diagnostic);
+      } else if (ProtoToken.is(tok::TokenKind::l_paren)) {
+        State = SawLeftParen;
+      }
+      break;
+    }
+  }
+
+  if (State == SawVoid && ProtoToken.is(tok::TokenKind::r_paren)) {
+    removeVoidToken(VoidToken, Diagnostic);
+  }
+}
+
+void RedundantVoidArgCheck::removeVoidToken(Token VoidToken,
+                                            StringRef Diagnostic) {
+  SourceLocation VoidLoc(VoidToken.getLocation());
+  auto VoidRange =
+      CharSourceRange::getTokenRange(VoidLoc, VoidLoc.getLocWithOffset(3));
+  diag(VoidLoc, Diagnostic) << FixItHint::CreateRemoval(VoidRange);
+}
+
+void RedundantVoidArgCheck::processTypedefDecl(
+    const MatchFinder::MatchResult &Result, const TypedefDecl *Typedef) {
+  if (protoTypeHasNoParms(Typedef->getUnderlyingType())) {
+    removeVoidArgumentTokens(Result, Typedef->getSourceRange(), "typedef");
+  }
+}
+
+void RedundantVoidArgCheck::processFieldDecl(
+    const MatchFinder::MatchResult &Result, const FieldDecl *Member) {
+  if (protoTypeHasNoParms(Member->getType())) {
+    removeVoidArgumentTokens(Result, Member->getSourceRange(),
+                             "field declaration");
+  }
+}
+
+void RedundantVoidArgCheck::processVarDecl(
+    const MatchFinder::MatchResult &Result, const VarDecl *Var) {
+  if (protoTypeHasNoParms(Var->getType())) {
+    SourceLocation Begin = Var->getLocStart();
+    if (Var->hasInit()) {
+      SourceLocation InitStart =
+          Result.SourceManager->getExpansionLoc(Var->getInit()->getLocStart())
+              .getLocWithOffset(-1);
+      removeVoidArgumentTokens(Result, SourceRange(Begin, InitStart),
+                               "variable declaration with initializer");
+    } else {
+      removeVoidArgumentTokens(Result, Var->getSourceRange(),
+                               "variable declaration");
+    }
+  }
+}
+
+void RedundantVoidArgCheck::processNamedCastExpr(
+    const MatchFinder::MatchResult &Result, const CXXNamedCastExpr *NamedCast) {
+  if (protoTypeHasNoParms(NamedCast->getTypeAsWritten())) {
+    removeVoidArgumentTokens(
+        Result,
+        NamedCast->getTypeInfoAsWritten()->getTypeLoc().getSourceRange(),
+        "named cast");
+  }
+}
+
+void RedundantVoidArgCheck::processExplicitCastExpr(
+    const MatchFinder::MatchResult &Result,
+    const ExplicitCastExpr *ExplicitCast) {
+  if (protoTypeHasNoParms(ExplicitCast->getTypeAsWritten())) {
+    removeVoidArgumentTokens(Result, ExplicitCast->getSourceRange(),
+                             "cast expression");
+  }
+}
+
+void RedundantVoidArgCheck::processLambdaExpr(
+    const MatchFinder::MatchResult &Result, const LambdaExpr *Lambda) {
+  if (Lambda->getLambdaClass()->getLambdaCallOperator()->getNumParams() == 0 &&
+      Lambda->hasExplicitParameters()) {
+    SourceLocation Begin =
+        Lambda->getIntroducerRange().getEnd().getLocWithOffset(1);
+    SourceLocation End = Lambda->getBody()->getLocStart().getLocWithOffset(-1);
+    removeVoidArgumentTokens(Result, SourceRange(Begin, End),
+                             "lambda expression");
+  }
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang

clang-tools-extra/trunk/test/clang-tidy/modernize-redundant-void-arg.c

+// RUN: clang-tidy -checks=-*,modernize-redundant-void-arg %s -- -x c | count 0
+
+#include <stdio.h>
+
+extern int i;
+
+int foo2() {
+  return 0;
+}
+
+int j = 1;
+
+int foo(void) {
+  return 0;
+}
+
+typedef unsigned int my_uint;
+
+typedef void my_void;
+
+// A function taking void and returning a pointer to function taking void
+// and returning int.
+int (*returns_fn_void_int(void))(void);
+
+typedef int (*returns_fn_void_int_t(void))(void);
+
+int (*returns_fn_void_int(void))(void) {
+  return NULL;
+}
+
+// A function taking void and returning a pointer to a function taking void
+// and returning a pointer to a function taking void and returning void.
+void (*(*returns_fn_returns_fn_void_void(void))(void))(void);
+
+typedef void (*(*returns_fn_returns_fn_void_void_t(void))(void))(void);
+
+void (*(*returns_fn_returns_fn_void_void(void))(void))(void) {
+  return NULL;
+}
+
+void bar() {
+  int i;
+  int *pi = NULL;
+  void *pv = (void *) pi;
+  float f;
+  float *fi;
+  double d;
+  double *pd;
+}
+
+void (*f1)(void);
+void (*f2)(void) = NULL;
+void (*f3)(void) = bar;
+void (*fa)();
+void (*fb)() = NULL;
+void (*fc)() = bar;
+
+typedef void (function_ptr)(void);

clang-tools-extra/trunk/test/clang-tidy/modernize-redundant-void-arg.cpp

+// RUN: %check_clang_tidy %s modernize-redundant-void-arg %t
+
+#include <iostream>
+
+int foo();
+
+void bar();
+
+void bar2();
+
+extern "C" void ecfoo(void);
+
+extern "C" void ecfoo(void) {
+}
+
+extern int i;
+
+int j = 1;
+
+int foo(void) {
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant void argument list in function definition [modernize-redundant-void-arg]
+// CHECK-FIXES: {{^}}int foo() {{{$}}
+    return 0;
+}
+
+typedef unsigned int my_uint;
+
+typedef void my_void;
+
+// A function taking void and returning a pointer to function taking void
+// and returning int.
+int (*returns_fn_void_int(void))(void);
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: {{.*}} in function declaration
+// CHECK-MESSAGES: :[[@LINE-2]]:34: warning: {{.*}} in function declaration
+// CHECK-FIXES: {{^}}int (*returns_fn_void_int())();{{$}}
+
+typedef int (*returns_fn_void_int_t(void))(void);
+// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: {{.*}} in typedef
+// CHECK-MESSAGES: :[[@LINE-2]]:44: warning: {{.*}} in typedef
+// CHECK-FIXES: {{^}}typedef int (*returns_fn_void_int_t())();{{$}}
+
+int (*returns_fn_void_int(void))(void) {
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: {{.*}} in function definition
+// CHECK-MESSAGES: :[[@LINE-2]]:34: warning: {{.*}} in function definition
+// CHECK-FIXES: {{^}}int (*returns_fn_void_int())() {{{$}}
+  return nullptr;
+}
+
+// A function taking void and returning a pointer to a function taking void
+// and returning a pointer to a function taking void and returning void.
+void (*(*returns_fn_returns_fn_void_void(void))(void))(void);
+// CHECK-MESSAGES: :[[@LINE-1]]:42: warning: {{.*}} in function declaration
+// CHECK-MESSAGES: :[[@LINE-2]]:49: warning: {{.*}} in function declaration
+// CHECK-MESSAGES: :[[@LINE-3]]:56: warning: {{.*}} in function declaration
+// CHECK-FIXES: {{^}}void (*(*returns_fn_returns_fn_void_void())())();{{$}}
+
+typedef void (*(*returns_fn_returns_fn_void_void_t(void))(void))(void);
+// CHECK-MESSAGES: :[[@LINE-1]]:52: warning: {{.*}} in typedef
+// CHECK-MESSAGES: :[[@LINE-2]]:59: warning: {{.*}} in typedef
+// CHECK-MESSAGES: :[[@LINE-3]]:66: warning: {{.*}} in typedef
+// CHECK-FIXES: {{^}}typedef void (*(*returns_fn_returns_fn_void_void_t())())();{{$}}
+
+void (*(*returns_fn_returns_fn_void_void(void))(void))(void) {
+// CHECK-MESSAGES: :[[@LINE-1]]:42: warning: {{.*}} in function definition
+// CHECK-MESSAGES: :[[@LINE-2]]:49: warning: {{.*}} in function definition
+// CHECK-MESSAGES: :[[@LINE-3]]:56: warning: {{.*}} in function definition
+// CHECK-FIXES: {{^}}void (*(*returns_fn_returns_fn_void_void())())() {{{$}}
+    return nullptr;
+}
+
+void bar(void) {
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: {{.*}} in function definition
+// CHECK-FIXES: {{^}}void bar() {{{$}}
+}
+
+void op_fn(int i) {
+}
+
+class gronk {
+public:
+  gronk();
+  ~gronk();
+
+    void foo();
+    void bar();
+    void bar2
+        ();
+    void operation(int i) { }
+
+private:
+    int m_i;
+    int *m_pi;
+    float m_f;
+    float *m_pf;
+    double m_d;
+    double *m_pd;
+
+    void (*f1)(void);
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: {{.*}} in field declaration
+    // CHECK-FIXES: {{^    }}void (*f1)();{{$}}
+
+  void (*op)(int i);
+
+  void (gronk::*p1)(void);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: {{.*}} in field declaration
+  // CHECK-FIXES: {{^  }}void (gronk::*p1)();{{$}}
+
+  int (gronk::*p_mi);
+
+  void (gronk::*p2)(int);
+
+  void (*(*returns_fn_returns_fn_void_void(void))(void))(void);
+  // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: {{.*}} in function declaration
+  // CHECK-MESSAGES: :[[@LINE-2]]:51: warning: {{.*}} in function declaration
+  // CHECK-MESSAGES: :[[@LINE-3]]:58: warning: {{.*}} in function declaration
+  // CHECK-FIXES: {{^}}  void (*(*returns_fn_returns_fn_void_void())())();{{$}}
+
+  void (*(*(gronk::*returns_fn_returns_fn_void_void_mem)(void))(void))(void);
+  // CHECK-MESSAGES: :[[@LINE-1]]:58: warning: {{.*}} in field declaration
+  // CHECK-MESSAGES: :[[@LINE-2]]:65: warning: {{.*}} in field declaration
+  // CHECK-MESSAGES: :[[@LINE-3]]:72: warning: {{.*}} in field declaration
+  // CHECK-FIXES: {{^}}  void (*(*(gronk::*returns_fn_returns_fn_void_void_mem)())())();{{$}}
+};
+
+int i;
+int *pi;
+void *pv = (void *) pi;
+float f;
+float *fi;
+double d;
+double *pd;
+
+void (*f1)(void);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: {{.*}} in variable declaration
+// CHECK-FIXES: {{^}}void (*f1)();{{$}}
+
+void (*f2)(void) = nullptr;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: {{.*}} in variable declaration with initializer
+// CHECK-FIXES: {{^}}void (*f2)() = nullptr;{{$}}
+
+void (*f2b)(void)(nullptr);
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}} in variable declaration with initializer
+// CHECK-FIXES: {{^}}void (*f2b)()(nullptr);{{$}}
+
+void (*f2c)(void){nullptr};
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}} in variable declaration with initializer
+// CHECK-FIXES: {{^}}void (*f2c)(){nullptr};{{$}}
+
+void (*f2d)(void) = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}} in variable declaration with initializer
+// CHECK-FIXES: {{^}}void (*f2d)() = NULL;{{$}}
+
+void (*f2e)(void)(NULL);
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}} in variable declaration with initializer
+// CHECK-FIXES: {{^}}void (*f2e)()(NULL);{{$}}
+
+void (*f2f)(void){NULL};
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}} in variable declaration with initializer
+// CHECK-FIXES: {{^}}void (*f2f)(){NULL};{{$}}
+
+void (*f3)(void) = bar;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: {{.*}} in variable declaration with initializer
+// CHECK-FIXES: {{^}}void (*f3)() = bar;{{$}}
+
+void (*o1)(int i);
+void (*o2)(int i) = nullptr;
+void (*o3)(int i)(nullptr);
+void (*o4)(int i){nullptr};
+void (*o5)(int i) = NULL;
+void (*o6)(int i)(NULL);
+void (*o7)(int i){NULL};
+void (*o8)(int i) = op_fn;
+
+void (*fa)();
+
+void (*fb)() = nullptr;
+
+void (*fc)() = bar;
+
+typedef void (function_ptr)(void);
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: {{.*}} in typedef
+// CHECK-FIXES: {{^}}typedef void (function_ptr)();{{$}}
+
+// intentionally not LLVM style to check preservation of whitesapce
+typedef void (function_ptr2)
+    (
+        void
+    );
+// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: {{.*}} in typedef
+// CHECK-FIXES:      {{^typedef void \(function_ptr2\)$}}
+// CHECK-FIXES-NEXT: {{^    \($}}
+// CHECK-FIXES-NEXT: {{^        $}}
+// CHECK-FIXES-NEXT: {{^    \);$}}
+
+// intentionally not LLVM style to check preservation of whitesapce
+typedef
+void
+(
+*
+(
+*
+returns_fn_returns_fn_void_void_t2
+(
+void
+)
+)
+(
+void
+)
+)
+(
+void
+)
+;
+// CHECK-MESSAGES: :[[@LINE-11]]:1: warning: {{.*}} in typedef
+// CHECK-MESSAGES: :[[@LINE-8]]:1: warning: {{.*}} in typedef
+// CHECK-MESSAGES: :[[@LINE-5]]:1: warning: {{.*}} in typedef
+// CHECK-FIXES:      {{^typedef$}}
+// CHECK-FIXES-NEXT: {{^void$}}
+// CHECK-FIXES-NEXT: {{^\($}}
+// CHECK-FIXES-NEXT: {{^\*$}}
+// CHECK-FIXES-NEXT: {{^\($}}
+// CHECK-FIXES-NEXT: {{^\*$}}
+// CHECK-FIXES-NEXT: {{^returns_fn_returns_fn_void_void_t2$}}
+// CHECK-FIXES-NEXT: {{^\($}}
+// CHECK-FIXES-NOT:  {{[^ ]}}
+// CHECK-FIXES:      {{^\)$}}
+// CHECK-FIXES-NEXT: {{^\)$}}
+// CHECK-FIXES-NEXT: {{^\($}}
+// CHECK-FIXES-NOT:  {{[^ ]}}
+// CHECK-FIXES:      {{^\)$}}
+// CHECK-FIXES-NEXT: {{^\)$}}
+// CHECK-FIXES-NEXT: {{^\($}}
+// CHECK-FIXES-NOT:  {{[^ ]}}
+// CHECK-FIXES:      {{^\)$}}
+// CHECK-FIXES-NEXT: {{^;$}}
+
+
+void (gronk::*p1)(void);
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: {{.*}} in variable declaration
+// CHECK-FIXES: {{^}}void (gronk::*p1)();{{$}}
+
+void (gronk::*p2)(void) = &gronk::foo;
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: {{.*}} in variable declaration with initializer
+// CHECK-FIXES: {{^}}void (gronk::*p2)() = &gronk::foo;{{$}}
+
+typedef void (gronk::*member_function_ptr)(void);
+// CHECK-MESSAGES: :[[@LINE-1]]:44: warning: {{.*}} in typedef
+// CHECK-FIXES: {{^}}typedef void (gronk::*member_function_ptr)();{{$}}
+
+// intentionally not LLVM style to check preservation of whitesapce
+typedef void (gronk::*member_function_ptr2)
+    (
+        void
+    );
+// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: {{.*}} in typedef
+// CHECK-FIXES:      {{^typedef void \(gronk::\*member_function_ptr2\)$}}
+// CHECK-FIXES-NEXT: {{^    \($}}
+// CHECK-FIXES-NEXT: {{^        $}}
+// CHECK-FIXES-NEXT: {{^    \);$}}
+
+void gronk::foo() {
+  void (*f1)(void) = &::bar;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: {{.*}} in variable declaration with initializer
+  // CHECK-FIXES: {{^  }}void (*f1)() = &::bar;{{$}}
+
+  void (*f2)(void);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: {{.*}} in variable declaration
+  // CHECK-FIXES: {{^  }}void (*f2)();{{$}}
+
+  // intentionally not LLVM style to check preservation of whitesapce
+  void (*f3)
+      (
+          void
+      );
+  // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: {{.*}} in variable declaration
+  // CHECK-FIXES:      {{^  }}void (*f3){{$}}
+  // CHECK-FIXES-NEXT: {{^      \($}}
+  // CHECK-FIXES-NEXT: {{^          $}}
+  // CHECK-FIXES-NEXT: {{^      \);$}}
+}
+
+void gronk::bar(void) {
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} in function definition
+// CHECK-FIXES: {{^}}void gronk::bar() {{{$}}
+  void (gronk::*p3)(void) = &gronk::foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: {{.*}} in variable declaration with initializer
+  // CHECK-FIXES: {{^  }}void (gronk::*p3)() = &gronk::foo;{{$}}
+
+  void (gronk::*p4)(void);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: {{.*}} in variable declaration
+  // CHECK-FIXES: {{^  }}void (gronk::*p4)();{{$}}
+
+  // intentionally not LLVM style to check preservation of whitesapce
+  void (gronk::*p5)
+      (
+          void
+      );
+  // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: {{.*}} in variable declaration
+  // CHECK-FIXES:      {{^  }}void (gronk::*p5){{$}}
+  // CHECK-FIXES-NEXT: {{^      \($}}
+  // CHECK-FIXES-NExT: {{^          $}}
+  // CHECK-FIXES-NExT: {{^      \);$}}
+}
+
+// intentionally not LLVM style to check preservation of whitesapce
+void gronk::bar2
+  (
+  void
+  )
+// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: {{.*}} in function definition
+// CHECK-FIXES:      {{^void gronk::bar2$}}
+// CHECK-FIXES-NEXT: {{^  \($}}
+// CHECK-FIXES-NEXT: {{^  $}}
+// CHECK-FIXES-NEXT: {{^  \)$}}
+{
+}
+
+gronk::gronk(void)
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: {{.*}} in function definition
+// CHECK-FIXES: {{^}}gronk::gronk(){{$}}
+  : f1(nullptr),
+  p1(nullptr) {
+}
+
+gronk::~gronk(void) {
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}} in function definition
+// CHECK-FIXES: {{^}}gronk::~gronk() {{{$}}
+}
+
+class nutter {
+public:
+  nutter();
+};
+
+nutter::nutter(void) {
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: {{.*}} in function definition
+// CHECK-FIXES: {{^}}nutter::nutter() {{{$}}
+  void (*f3)(void) = static_cast<void (*)(void)>(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: {{.*}} in variable declaration with initializer
+  // CHECK-MESSAGES: :[[@LINE-2]]:43: warning: {{.*}} in named cast
+  // CHECK-FIXES: void (*f3)() = static_cast<void (*)()>(0);{{$}}
+
+  void (*f4)(void) = (void (*)(void)) 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: {{.*}} in variable declaration with initializer
+  // CHECK-MESSAGES: :[[@LINE-2]]:32: warning: {{.*}} in cast expression
+  // CHECK-FIXES: void (*f4)() = (void (*)()) 0;{{$}}
+
+  void (*f5)(void) = reinterpret_cast<void (*)(void)>(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: {{.*}} in variable declaration with initializer
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: {{.*}} in named cast
+  // CHECK-FIXES: void (*f5)() = reinterpret_cast<void (*)()>(0);{{$}}
+
+  // intentionally not LLVM style to check preservation of whitesapce
+  void (*f6)(void) = static_cast<void (*)
+      (
+          void
+      )>(0);
+  // CHECK-MESSAGES: :[[@LINE-4]]:14: warning: {{.*}} in variable declaration with initializer
+  // CHECK-MESSAGES: :[[@LINE-3]]:11: warning: {{.*}} in named cast
+  // CHECK-FIXES:      {{^  }}void (*f6)() = static_cast<void (*){{$}}
+  // CHECK-FIXES-NEXT: {{^      \($}}
+  // CHECK-FIXES-NEXT: {{^          $}}
+  // CHECK-FIXES-NEXT: {{^      }})>(0);{{$}}
+
+  // intentionally not LLVM style to check preservation of whitesapce
+  void (*f7)(void) = (void (*)
+      (
+          void
+      )) 0;
+  // CHECK-MESSAGES: :[[@LINE-4]]:14: warning: {{.*}} in variable declaration with initializer
+  // CHECK-MESSAGES: :[[@LINE-3]]:11: warning: {{.*}} in cast expression
+  // CHECK-FIXES:      {{^  }}void (*f7)() = (void (*){{$}}
+  // CHECK-FIXES-NEXT: {{^      \($}}
+  // CHECK-FIXES-NEXT: {{^          $}}
+  // CHECK-FIXES-NEXT: {{^      \)\) 0;$}}
+
+  // intentionally not LLVM style to check preservation of whitesapce
+  void (*f8)(void) = reinterpret_cast<void (*)
+      (
+          void
+      )>(0);
+  // CHECK-MESSAGES: :[[@LINE-4]]:14: warning: {{.*}} in variable declaration with initializer
+  // CHECK-MESSAGES: :[[@LINE-3]]:11: warning: {{.*}} in named cast
+  // CHECK-FIXES:      {{^  }}void (*f8)() = reinterpret_cast<void (*){{$}}
+  // CHECK-FIXES-NEXT: {{^      \($}}
+  // CHECK-FIXES-NEXT: {{^          $}}
+  // CHECK-FIXES-NEXT: {{^      \)>\(0\);$}}
+
+  void (*o1)(int) = static_cast<void (*)(int)>(0);
+  void (*o2)(int) = (void (*)(int)) 0;
+  void (*o3)(int) = reinterpret_cast<void (*)(int)>(0);
+}
+
+class generator {
+public:
+  int operator()(void) { return 1; }
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: {{.*}} in function definition
+  // CHECK-FIXES: {{^  }}int operator()() { return 1; }{{$}}
+};
+
+void test_lambda_functions() {
+  auto lamb_duh = [](void (*fn)(void)) { (*fn)(); };
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: {{.*}} in variable declaration
+  // CHECK-FIXES: {{^  }}auto lamb_duh = [](void (*fn)()) { (*fn)(); };{{$}}
+
+  auto lambda_generator = [](void) { return 1; };
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: {{.*}} in lambda expression
+  // CHECK-FIXES: {{^  }}auto lambda_generator = []() { return 1; };{{$}}
+
+  auto gen2 = []() { return 1; };
+
+  auto gen3 = []{ return 1; };
+
+  auto void_returner = [](void) -> void (*)(void) { return f1; };
+  // CHECK-MESSAGES: [[@LINE-1]]:27: warning: {{.*}} in lambda expression
+  // CHECK-MESSAGES: [[@LINE-2]]:45: warning: {{.*}} in lambda expression
+  // CHECK-FIXES: {{^  }}auto void_returner = []() -> void (*)() { return f1; };{{$}}
+}