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

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

105 ↗

(On Diff #20580)

Fixed.

207 ↗

(On Diff #20580)

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

15

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.

147

nit: No need for clang:: here.

196

It's better to use token ranges here:

CharSourceRange::getTokenRange(VoidLoc, VoidLoc)

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

87

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

15

Fixed.

147

Fixed.

196

Fixed.

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

87

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

245

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

78

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

245

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

245

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

78

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

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

229

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

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

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

52

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-tidy/ClangTidy.h

 private:
   void run(const ast_matchers::MatchFinder::MatchResult &Result) override;
   StringRef getID() const override { return CheckName; }
   std::string CheckName;
   ClangTidyContext *Context;
 
 protected:
   OptionsView Options;
-  /// \brief Returns the main file name of the current translation unit.
-  StringRef getCurrentMainFile() const { return Context->getCurrentFile(); }
 };
 
 class ClangTidyCheckFactories;
 
 class ClangTidyASTConsumerFactory {
 public:
   ClangTidyASTConsumerFactory(ClangTidyContext &Context);
 

clang-tidy/ClangTidyDiagnosticConsumer.h

public:
   /// \brief Sets the \c SourceManager of the used \c DiagnosticsEngine.
   ///
   /// This is called from the \c ClangTidyCheck base class.
   void setSourceManager(SourceManager *SourceMgr);
 
   /// \brief Should be called when starting to process new translation unit.
   void setCurrentFile(StringRef File);
 
-  /// \brief Returns the main file name of the current translation unit.
-  StringRef getCurrentFile() const { return CurrentFile; }
-
   /// \brief Sets ASTContext for the current translation unit.
   void setASTContext(ASTContext *Context);
 
   /// \brief Returns the name of the clang-tidy check which produced this
   /// diagnostic ID.
   StringRef getCheckName(unsigned DiagnosticID) const;
 
   /// \brief Returns check filter for the \c CurrentFile.

clang-tidy/google/AvoidCStyleCastsCheck.cpp

void AvoidCStyleCastsCheck::check(const MatchFinder::MatchResult &Result) {
 
   // The rest of this check is only relevant to C++.
   if (!Result.Context->getLangOpts().CPlusPlus)
     return;
   // Ignore code inside extern "C" {} blocks.
   if (!match(expr(hasAncestor(linkageSpecDecl())), *CastExpr, *Result.Context)
            .empty())
     return;
-  // Ignore code in .c files and headers included from them, even if they are
-  // compiled as C++.
-  if (getCurrentMainFile().endswith(".c"))
-    return;
-  // Ignore code in .c files #included in other files (which shouldn't be done,
-  // but people still do this for test and other purposes).
-  SourceManager &SM = *Result.SourceManager;
-  if (SM.getFilename(SM.getSpellingLoc(CastExpr->getLocStart())).endswith(".c"))
-    return;
 
   // Leave type spelling exactly as it was (unlike
   // getTypeAsWritten().getAsString() which would spell enum types 'enum X').
-  StringRef DestTypeString =
-      Lexer::getSourceText(CharSourceRange::getTokenRange(
-                               CastExpr->getLParenLoc().getLocWithOffset(1),
-                               CastExpr->getRParenLoc().getLocWithOffset(-1)),
-                           SM, Result.Context->getLangOpts());
+  StringRef DestTypeString = Lexer::getSourceText(
+      CharSourceRange::getTokenRange(
+      *Result.SourceManager, Result.Context->getLangOpts());
 
   auto diag_builder =
       diag(CastExpr->getLocStart(), "C-style casts are discouraged. %0");
 
   auto ReplaceWithCast = [&](StringRef CastType) {
     diag_builder << ("Use " + CastType).str();
 
     const Expr *SubExpr = CastExpr->getSubExprAsWritten()->IgnoreImpCasts();
     std::string CastText = (CastType + "<" + DestTypeString + ">").str();
     if (!isa<ParenExpr>(SubExpr)) {
       CastText.push_back('(');
       diag_builder << FixItHint::CreateInsertion(
-          Lexer::getLocForEndOfToken(SubExpr->getLocEnd(), 0, SM,
+          Lexer::getLocForEndOfToken(SubExpr->getLocEnd(), 0,
+                                     *Result.SourceManager,
                                      Result.Context->getLangOpts()),
           ")");
     }
     diag_builder << FixItHint::CreateReplacement(ParenRange, CastText);
   };
 
   // Suggest appropriate C++ cast. See [expr.cast] for cast notation semantics.
   switch (CastExpr->getCastKind()) {

clang-tidy/misc/AssertSideEffectCheck.cpp

return OpKind == OO_Equal || OpKind == OO_PlusEqual ||
            OpKind == OO_PlusPlus || OpKind == OO_MinusMinus ||
            OpKind == OO_PercentEqual || OpKind == OO_New ||
            OpKind == OO_Delete || OpKind == OO_Array_New ||
            OpKind == OO_Array_Delete;
   }
 
   if (const auto *CExpr = dyn_cast<CallExpr>(E)) {
     bool Result = CheckFunctionCalls;
-    if (const auto *FuncDecl = CExpr->getDirectCallee()) {
-      if (FuncDecl->getDeclName().isIdentifier() &&
-          FuncDecl->getName() == "__builtin_expect") // exceptions come here
-        Result = false;
-      else if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FuncDecl))
+    if (const auto *FuncDecl = CExpr->getDirectCallee())
+      if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FuncDecl))
         Result &= !MethodDecl->isConst();
-    }
     return Result;
   }
 
   return isa<CXXNewExpr>(E) || isa<CXXDeleteExpr>(E) || isa<CXXThrowExpr>(E);
 }
 
 } // namespace ast_matchers
 

clang-tidy/misc/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyMiscModule
   ArgumentCommentCheck.cpp
   AssertSideEffectCheck.cpp
   AssignOperatorSignatureCheck.cpp
   BoolPointerImplicitConversionCheck.cpp
   InaccurateEraseCheck.cpp
   InefficientAlgorithmCheck.cpp
   MiscTidyModule.cpp
-  NoexceptMoveConstructorCheck.cpp
   StaticAssertCheck.cpp
   SwappedArgumentsCheck.cpp
   UndelegatedConstructor.cpp
   UnusedRAIICheck.cpp
   UniqueptrResetReleaseCheck.cpp
   UseOverrideCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   )

clang-tidy/misc/MiscTidyModule.cpp

 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "ArgumentCommentCheck.h"
 #include "AssertSideEffectCheck.h"
 #include "AssignOperatorSignatureCheck.h"
 #include "BoolPointerImplicitConversionCheck.h"
 #include "InaccurateEraseCheck.h"
 #include "InefficientAlgorithmCheck.h"
-#include "NoexceptMoveConstructorCheck.h"
 #include "StaticAssertCheck.h"
 #include "SwappedArgumentsCheck.h"
 #include "UndelegatedConstructor.h"
 #include "UniqueptrResetReleaseCheck.h"
 #include "UnusedRAIICheck.h"
 #include "UseOverrideCheck.h"
 
 namespace clang {
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<AssignOperatorSignatureCheck>(
         "misc-assign-operator-signature");
     CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
         "misc-bool-pointer-implicit-conversion");
     CheckFactories.registerCheck<InaccurateEraseCheck>(
         "misc-inaccurate-erase");
     CheckFactories.registerCheck<InefficientAlgorithmCheck>(
         "misc-inefficient-algorithm");
-    CheckFactories.registerCheck<NoexceptMoveConstructorCheck>(
-        "misc-noexcept-move-constructor");
     CheckFactories.registerCheck<StaticAssertCheck>(
         "misc-static-assert");
     CheckFactories.registerCheck<SwappedArgumentsCheck>(
         "misc-swapped-arguments");
     CheckFactories.registerCheck<UndelegatedConstructorCheck>(
         "misc-undelegated-constructor");
     CheckFactories.registerCheck<UniqueptrResetReleaseCheck>(
         "misc-uniqueptr-reset-release");

clang-tidy/misc/NoexceptMoveConstructorCheck.h

-//===--- NoexceptMoveConstructorCheck.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_MISC_NOEXCEPTMOVECONSTRUCTORCHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NOEXCEPTMOVECONSTRUCTORCHECK_H
-
-#include "../ClangTidy.h"
-
-namespace clang {
-namespace tidy {
-
-/// \brief The check flags user-defined move constructors and assignment
-/// operators not marked with \c noexcept or marked with \c noexcept(expr) where
-/// \c expr evaluates to \c false (but is not a \c false literal itself).
-///
-/// Move constructors of all the types used with STL containers, for example,
-/// need to be declared \c noexcept. Otherwise STL will choose copy constructors
-/// instead. The same is valid for move assignment operations.
-class NoexceptMoveConstructorCheck : public ClangTidyCheck {
-public:
-  NoexceptMoveConstructorCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
-  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
-  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
-};
-
-} // namespace tidy
-} // namespace clang
-
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NOEXCEPTMOVECONSTRUCTORCHECK_H
-

clang-tidy/misc/NoexceptMoveConstructorCheck.cpp

-//===--- NoexceptMoveConstructorCheck.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 "NoexceptMoveConstructorCheck.h"
-#include "clang/AST/ASTContext.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
-
-using namespace clang::ast_matchers;
-
-namespace clang {
-namespace tidy {
-
-void NoexceptMoveConstructorCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(
-      methodDecl(anyOf(constructorDecl(), hasOverloadedOperatorName("=")),
-                 unless(isImplicit()), unless(isDeleted()))
-          .bind("decl"),
-      this);
-}
-
-void NoexceptMoveConstructorCheck::check(
-    const MatchFinder::MatchResult &Result) {
-  if (const auto *Decl = Result.Nodes.getNodeAs<CXXMethodDecl>("decl")) {
-    StringRef MethodType = "assignment operator";
-    if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(Decl)) {
-      if (!Ctor->isMoveConstructor())
-        return;
-      MethodType = "constructor";
-    } else if (!Decl->isMoveAssignmentOperator()) {
-      return;
-    }
-
-    const auto *ProtoType = Decl->getType()->getAs<FunctionProtoType>();
-    switch(ProtoType->getNoexceptSpec(*Result.Context)) {
-      case  FunctionProtoType::NR_NoNoexcept:
-        diag(Decl->getLocation(), "move %0s should be marked noexcept")
-            << MethodType;
-        // FIXME: Add a fixit.
-        break;
-      case FunctionProtoType::NR_Throw:
-        // Don't complain about nothrow(false), but complain on nothrow(expr)
-        // where expr evaluates to false.
-        if (const Expr *E = ProtoType->getNoexceptExpr()) {
-          if (isa<CXXBoolLiteralExpr>(E))
-            break;
-          diag(E->getExprLoc(),
-               "noexcept specifier on the move %0 evaluates to 'false'")
-              << MethodType;
-        }
-        break;
-      case FunctionProtoType::NR_Nothrow:
-      case FunctionProtoType::NR_Dependent:
-      case FunctionProtoType::NR_BadNoexcept:
-        break;
-    }
-  }
-}
-
-} // namespace tidy
-} // namespace clang
-

clang-tidy/misc/StaticAssertCheck.cpp

auto IsAlwaysFalseWithCast = ignoringParenImpCasts(anyOf(IsAlwaysFalse,
       cStyleCastExpr(has(IsAlwaysFalse)).bind("castExpr")));
   auto AssertExprRoot = anyOf(
       binaryOperator(
           anyOf(hasOperatorName("&&"), hasOperatorName("==")),
           hasEitherOperand(ignoringImpCasts(stringLiteral().bind("assertMSG"))),
           anyOf(binaryOperator(hasEitherOperand(IsAlwaysFalseWithCast)),
           anything())).bind("assertExprRoot"),
       IsAlwaysFalse);
-  auto NonConstexprFunctionCall =
-      callExpr(hasDeclaration(functionDecl(unless(isConstexpr()))));
-  auto AssertCondition = expr(anyOf(
+  auto Condition = expr(anyOf(
       expr(ignoringParenCasts(anyOf(
           AssertExprRoot,
           unaryOperator(hasUnaryOperand(ignoringParenCasts(AssertExprRoot)))))),
-      anything()), unless(findAll(NonConstexprFunctionCall))).bind("condition");
-  auto Condition = anyOf(ignoringParenImpCasts(callExpr(
-      hasDeclaration(functionDecl(hasName("__builtin_expect"))),
-      hasArgument(0, AssertCondition))), AssertCondition);
+      anything()));
 
   Finder->addMatcher(
-      stmt(anyOf(conditionalOperator(hasCondition(Condition)),
-                 ifStmt(hasCondition(Condition))),
+      stmt(anyOf(conditionalOperator(hasCondition(Condition.bind("condition"))),
+                 ifStmt(hasCondition(Condition.bind("condition")))),
            unless(isInTemplateInstantiation())).bind("condStmt"),
       this);
 }
 
 void StaticAssertCheck::check(const MatchFinder::MatchResult &Result) {
   const ASTContext *ASTCtx = Result.Context;
   const LangOptions &Opts = ASTCtx->getLangOpts();
   const SourceManager &SM = ASTCtx->getSourceManager();

clang-tidy/readability/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyReadabilityModule
   BracesAroundStatementsCheck.cpp
   ContainerSizeEmptyCheck.cpp
   ElseAfterReturnCheck.cpp
   FunctionSizeCheck.cpp
   NamedParameterCheck.cpp
   NamespaceCommentCheck.cpp
   ReadabilityTidyModule.cpp
   RedundantStringCStrCheck.cpp
   RedundantSmartptrGetCheck.cpp
+  RedundantVoidArgCheck.cpp
   ShrinkToFitCheck.cpp
   SimplifyBooleanExprCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTooling
   )

clang-tidy/readability/ReadabilityTidyModule.cpp

 #include "../ClangTidyModuleRegistry.h"
 #include "BracesAroundStatementsCheck.h"
 #include "ContainerSizeEmptyCheck.h"
 #include "ElseAfterReturnCheck.h"
 #include "FunctionSizeCheck.h"
 #include "NamedParameterCheck.h"
 #include "RedundantSmartptrGetCheck.h"
 #include "RedundantStringCStrCheck.h"
+#include "RedundantVoidArgCheck.h"
 #include "ShrinkToFitCheck.h"
 #include "SimplifyBooleanExprCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace readability {
 
 class ReadabilityModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<BracesAroundStatementsCheck>(
         "readability-braces-around-statements");
     CheckFactories.registerCheck<ContainerSizeEmptyCheck>(
         "readability-container-size-empty");
     CheckFactories.registerCheck<ElseAfterReturnCheck>(
         "readability-else-after-return");
     CheckFactories.registerCheck<FunctionSizeCheck>(
         "readability-function-size");
     CheckFactories.registerCheck<readability::NamedParameterCheck>(
         "readability-named-parameter");
     CheckFactories.registerCheck<RedundantSmartptrGetCheck>(
         "readability-redundant-smartptr-get");
     CheckFactories.registerCheck<RedundantStringCStrCheck>(
         "readability-redundant-string-cstr");
+    CheckFactories.registerCheck<RedundantVoidArgCheck>(
+        "readability-redundant-void-arg");
     CheckFactories.registerCheck<ShrinkToFitCheck>(
         "readability-shrink-to-fit");
     CheckFactories.registerCheck<SimplifyBooleanExprCheck>(
         "readability-simplify-boolean-expr");
+    CheckFactories.registerCheck<ShrinkToFitCheck>("readability-shrink-to-fit");
   }
 };
 
 // Register the ReadabilityModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add<ReadabilityModule>
     X("readability-module", "Adds readability-related checks.");
 
 } // namespace readability
 
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the ReadabilityModule.
 volatile int ReadabilityModuleAnchorSource = 0;
 
 } // namespace tidy
 } // namespace clang

clang-tidy/readability/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_READABILITY_REDUNDANT_VOID_ARG_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_VOID_ARG_CHECK_H
+
+#include "../ClangTidy.h"
+#include "clang/Lex/Token.h"
+
+#include <string>
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// \brief Find and remove redundant void argument lists.
+///
+/// Examples:
+///   int foo(void); ==> int foo();
+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
+  processCStyleCastExpr(const ast_matchers::MatchFinder::MatchResult &Result,
+                        const CStyleCastExpr *CStyleCast);
+
+  void
+  processFunctionalCast(const ast_matchers::MatchFinder::MatchResult &Result,
+                        const CXXFunctionalCastExpr *FunctionalCast);
+
+  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 readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_VOID_ARG_CHECK_H

clang-tidy/readability/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 FunctionalCastId[] = "fn-cast";
+const char LambdaId[] = "lambda";
+
+} // namespace
+
+namespace tidy {
+namespace readability {
+
+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(
+      staticCastExpr(isExpansionInMainFile(), CastDestinationIsFunction)
+          .bind(NamedCastId),
+      this);
+  Finder->addMatcher(
+      reinterpretCastExpr(isExpansionInMainFile(), CastDestinationIsFunction)
+          .bind(NamedCastId),
+      this);
+  Finder->addMatcher(constCastExpr(isExpansionInMainFile(),
+                                   CastDestinationIsFunction).bind(NamedCastId),
+                     this);
+  Finder->addMatcher(
+      functionalCastExpr(isExpansionInMainFile(), CastDestinationIsFunction)
+          .bind(FunctionalCastId),
+      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)) {
+    processCStyleCastExpr(Result, CStyleCast);
+  } else if (const auto *FunctionalCast =
+                 Nodes.getNodeAs<CXXFunctionalCastExpr>(FunctionalCastId)) {
+    processFunctionalCast(Result, FunctionalCast);
+  } 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::processCStyleCastExpr(
+    const MatchFinder::MatchResult &Result, const CStyleCastExpr *CStyleCast) {
+  if (protoTypeHasNoParms(CStyleCast->getTypeAsWritten())) {
+    removeVoidArgumentTokens(
+        Result,
+        CStyleCast->getTypeInfoAsWritten()->getTypeLoc().getSourceRange(),
+        "C style cast");
+  }
+}
+
+void RedundantVoidArgCheck::processFunctionalCast(
+    const MatchFinder::MatchResult &Result,
+    const CXXFunctionalCastExpr *FunctionalCast) {
+  if (protoTypeHasNoParms(FunctionalCast->getTypeAsWritten())) {
+    removeVoidArgumentTokens(
+        Result,
+        FunctionalCast->getTypeInfoAsWritten()->getTypeLoc().getSourceRange(),
+        "functional cast");
+  }
+}
+
+void RedundantVoidArgCheck::processExplicitCastExpr(
+    const MatchFinder::MatchResult &Result,
+    const ExplicitCastExpr *ExplicitCast) {
+  if (protoTypeHasNoParms(ExplicitCast->getTypeAsWritten())) {
+    removeVoidArgumentTokens(Result, ExplicitCast->getSourceRange(),
+                             "explicit cast");
+  }
+}
+
+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 readability
+} // namespace tidy
+} // namespace clang

clang-tidy/readability/SimplifyBooleanExprCheck.h

 /// `if (b && true)`                           becomes `if (b)`
 /// `if (b && false)`                          becomes `if (false)`
 /// `if (b || true)`                           becomes `if (true)`
 /// `if (b || false)`                          becomes `if (b)`
 /// `e ? true : false`                         becomes `e`
 /// `e ? false : true`                         becomes `!e`
 /// `if (true) t(); else f();`                 becomes `t();`
 /// `if (false) t(); else f();`                becomes `f();`
-/// `if (e) return true; else return false;`   becomes `return e;`
-/// `if (e) return false; else return true;`   becomes `return !e;`
+/// `if (e) return true; else return false;`   becomes `return (e);`
+/// `if (e) return false; else return true;`   becomes `return !(e);`
 /// `if (e) b = true; else b = false;`         becomes `b = e;`
-/// `if (e) b = false; else b = true;`         becomes `b = !e;`
-///
-/// Parenthesis from the resulting expression `e` are removed whenever
-/// possible.
-///
-/// When a conditional boolean return or assignment appears at the end of a
-/// chain of `if`, `else if` statements, the conditional statement is left
-/// unchanged unless the option `ChainedConditionalReturn` or
-/// `ChainedConditionalAssignment`, respectively, is specified as non-zero.
-/// The default value for both options is zero.
+/// `if (e) b = false; else b = true;`         becomes `b = !(e);`
 ///
 class SimplifyBooleanExprCheck : public ClangTidyCheck {
 public:
-  SimplifyBooleanExprCheck(StringRef Name, ClangTidyContext *Context);
-
-  void storeOptions(ClangTidyOptions::OptionMap &Options) override;
+  SimplifyBooleanExprCheck(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 matchBoolBinOpExpr(ast_matchers::MatchFinder *Finder, bool Value,
                           StringRef OperatorName, StringRef BooleanId);
 
   void matchExprBinOpBool(ast_matchers::MatchFinder *Finder, bool Value,
private:
 
   void replaceWithReturnCondition(
       const ast_matchers::MatchFinder::MatchResult &Result, const IfStmt *If,
       bool Negated = false);
 
   void
   replaceWithAssignment(const ast_matchers::MatchFinder::MatchResult &Result,
                         const IfStmt *If, bool Negated = false);
-
-  const bool ChainedConditionalReturn;
-  const bool ChainedConditionalAssignment;
 };
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SIMPLIFY_BOOLEAN_EXPR_H

clang-tidy/readability/SimplifyBooleanExprCheck.cpp

std::string replacementExpression(const MatchFinder::MatchResult &Result,
   while (const auto *Parenthesized = dyn_cast<ParenExpr>(E)) {
     E = Parenthesized->getSubExpr();
   }
   if (Negated) {
     if (const auto *BinOp = dyn_cast<BinaryOperator>(E)) {
       StringRef NegatedOperator = negatedOperator(BinOp);
       if (!NegatedOperator.empty()) {
         return (getText(Result, *BinOp->getLHS()) + " " + NegatedOperator +
-                " " + getText(Result, *BinOp->getRHS())).str();
+                " " + getText(Result, *BinOp->getRHS()))
+            .str();
       }
     }
   }
   StringRef Text = getText(Result, *E);
   return (Negated ? (needsParensAfterUnaryNegation(E) ? "!(" + Text + ")"
                                                       : "!" + Text)
-                  : Text).str();
+                  : Text)
+      .str();
 }
 
 } // namespace
 
-SimplifyBooleanExprCheck::SimplifyBooleanExprCheck(StringRef Name,
-                                                   ClangTidyContext *Context)
-    : ClangTidyCheck(Name, Context),
-      ChainedConditionalReturn(Options.get("ChainedConditionalReturn", 0U)),
-      ChainedConditionalAssignment(
-          Options.get("ChainedConditionalAssignment", 0U)) {}
-
 void SimplifyBooleanExprCheck::matchBoolBinOpExpr(MatchFinder *Finder,
                                                   bool Value,
                                                   StringRef OperatorName,
                                                   StringRef BooleanId) {
   Finder->addMatcher(
       binaryOperator(isExpansionInMainFile(), hasOperatorName(OperatorName),
                      hasLHS(allOf(expr().bind(LHSId),
                                   boolLiteral(equals(Value)).bind(BooleanId))),
Finder->addMatcher(
                           hasTrueExpression(boolLiteral(equals(Value))),
                           hasFalseExpression(boolLiteral(equals(!Value))))
           .bind(TernaryId),
       this);
 }
 
 void SimplifyBooleanExprCheck::matchIfReturnsBool(MatchFinder *Finder,
                                                   bool Value, StringRef Id) {
-  if (ChainedConditionalReturn) {
-    Finder->addMatcher(ifStmt(isExpansionInMainFile(),
-                              hasThen(ReturnsBool(Value, ThenLiteralId)),
-                              hasElse(ReturnsBool(!Value))).bind(Id),
-                       this);
-  } else {
-    Finder->addMatcher(ifStmt(isExpansionInMainFile(),
-                              unless(hasParent(ifStmt())),
-                              hasThen(ReturnsBool(Value, ThenLiteralId)),
-                              hasElse(ReturnsBool(!Value))).bind(Id),
-                       this);
-  }
-}
 
 void SimplifyBooleanExprCheck::matchIfAssignsBool(MatchFinder *Finder,
                                                   bool Value, StringRef Id) {
   auto SimpleThen = binaryOperator(
       hasOperatorName("="),
       hasLHS(declRefExpr(hasDeclaration(decl().bind(IfAssignObjId)))),
       hasLHS(expr().bind(IfAssignVariableId)),
       hasRHS(boolLiteral(equals(Value)).bind(IfAssignLocId)));
   auto Then = anyOf(SimpleThen, compoundStmt(statementCountIs(1),
                                              hasAnySubstatement(SimpleThen)));
   auto SimpleElse = binaryOperator(
       hasOperatorName("="),
       hasLHS(declRefExpr(hasDeclaration(equalsBoundNode(IfAssignObjId)))),
       hasRHS(boolLiteral(equals(!Value))));
   auto Else = anyOf(SimpleElse, compoundStmt(statementCountIs(1),
                                              hasAnySubstatement(SimpleElse)));
-  if (ChainedConditionalAssignment) {
-    Finder->addMatcher(
-        ifStmt(isExpansionInMainFile(), hasThen(Then), hasElse(Else)).bind(Id),
-        this);
-  } else {
-    Finder->addMatcher(ifStmt(isExpansionInMainFile(),
-                              unless(hasParent(ifStmt())), hasThen(Then),
-                              hasElse(Else)).bind(Id),
-                       this);
-  }
-}
-
-void SimplifyBooleanExprCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
-  Options.store(Opts, "ChainedConditionalReturn", ChainedConditionalReturn);
-  Options.store(Opts, "ChainedConditionalAssignment",
-                ChainedConditionalAssignment);
 }
 
 void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {
   matchBoolBinOpExpr(Finder, true, "&&", RightExpressionId);
   matchBoolBinOpExpr(Finder, false, "||", RightExpressionId);
   matchExprBinOpBool(Finder, false, "&&", RightExpressionId);
   matchExprBinOpBool(Finder, true, "||", RightExpressionId);
   matchBoolCompOpExpr(Finder, true, "==", RightExpressionId);

docs/ModernizerUsage.rst

 
 .. _-p:
 
 .. option:: -p=<build-path>
 
   ``<build-path>`` is the directory containing a *compilation databasefile*, a
   file named ``compile_commands.json``, which provides compiler arguments for
   building each source file. CMake can generate this file by specifying
-  ``-DCMAKE_EXPORT_COMPILE_COMMANDS=ON`` when running CMake. Ninja_, since v1.2
-  can also generate this file with ``ninja -t compdb``. If the compilation
-  database cannot be used for any reason, an error is reported.
+  ``-DCMAKE_EXPORT_COMPILE_COMMANDS`` when running CMake. Ninja_, since v1.2 can
+  also generate this file with ``ninja -t compdb``. If the compilation database
+  cannot be used for any reason, an error is reported.
 
   This option is ignored if ``--`` is present.
 
   Files in the compilation database will be transformed if no sources are
   provided and paths to files are explicitly included using ``-include`` or
   ``-include-from``.
   In order to transform all files in a compilation database the following
   command line can be used:

docs/clang-modernize.rst

 arguments, it might be easier to pass the compiler args on the command line
 after ``--``. If you don't have any compiler arguments then ``--`` is not needed.
 If you're working with multiple files or even a single file with many compiler
 args, it's probably best to use a *compilation database*.
 
 A `compilation database`_ contains the command-line arguments for multiple
 files. If the code you want to transform can be built with CMake, you can
 generate this database easily by running CMake with the
-``-DCMAKE_EXPORT_COMPILE_COMMANDS=ON`` option. The Ninja_ build system, since
+``-DCMAKE_EXPORT_COMPILE_COMMANDS`` option. The Ninja_ build system, since
 v1.2, can create this file too using the *compdb* tool: ``ninja -t compdb``. If
 you're not already using either of these tools or cannot easily make use of
 them you might consider looking into Bear_.
 
 In addition to the compiler arguments you usually build your code with, you must
 provide the option for enabling C++11 features. For clang and versions of gcc
 ≥ v4.8 this is ``-std=c++11``.
 

test/clang-modernize/HeaderReplacements/main.cpp

 //
 // The following are for FileCheck when used on output of 'ls'. See above.
 // MAIN_CPP: {{^main.cpp_.*.yaml$}}
 // MAIN_CPP-NOT: {{main.cpp_.*.yaml}}
 //
 // COMMON_CPP:     {{^common.cpp_.*.yaml$}}
 // COMMON_CPP-NOT: {{common.cpp_.*.yaml}}
 
-// REQUIRES: shell
-
 #include "common.h"
 
 void test_header_replacement() {
   dostuff();
   func2();
 }

test/clang-tidy/google-readability-casting.c

 // RUN: $(dirname %s)/check_clang_tidy.sh %s google-readability-casting %t -- -x c
-// The testing script always adds .cpp extension to the input file name, so we
-// need to run clang-tidy directly in order to verify handling of .c files:
-// RUN: clang-tidy --checks=-*,google-readability-casting %s -- -x c++ | FileCheck %s -check-prefix=CHECK-MESSAGES -implicit-check-not='{{warning|error}}:'
-// RUN: cp %s %t.main_file.cpp
-// RUN: clang-tidy --checks=-*,google-readability-casting -header-filter='.*' %t.main_file.cpp -- -I%S -DTEST_INCLUDE -x c++ | FileCheck %s -check-prefix=CHECK-MESSAGES -implicit-check-not='{{warning|error}}:'
 // REQUIRES: shell
 
-#ifdef TEST_INCLUDE
-
-#undef TEST_INCLUDE
-#include "google-readability-casting.c"
-
-#else
-
 void f(const char *cpc) {
   const char *cpc2 = (const char*)cpc;
   // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: redundant cast to the same type [google-readability-casting]
   // CHECK-FIXES: const char *cpc2 = cpc;
   char *pc = (char*)cpc;
 }
-
-#endif

test/clang-tidy/misc-assert-side-effect.cpp

-// RUN: $(dirname %s)/check_clang_tidy.sh %s misc-assert-side-effect %t -config="{CheckOptions: [{key: misc-assert-side-effect.CheckFunctionCalls, value: 1}, {key: misc-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert'}]}" -- -fexceptions
+// RUN: $(dirname %s)/check_clang_tidy.sh %s misc-assert-side-effect %t -config="{CheckOptions: [{key: misc-assert-side-effect.CheckFunctionCalls, value: 1}, {key: misc-assert-side-effect.AssertMacros, value: 'assert,my_assert'}]}" -- -fexceptions
 // REQUIRES: shell
 
 //===--- assert definition block ------------------------------------------===//
 int abort() { return 0; }
 
 #ifdef NDEBUG
 #define assert(x) 1
 #else
 #define assert(x)                                                              \
   if (!(x))                                                                    \
   (void)abort()
 #endif
 
-void print(...);
-#define assert2(e) (__builtin_expect(!(e), 0) ?                                \
-                       print (#e, __FILE__, __LINE__) : (void)0)
-
 #ifdef NDEBUG
 #define my_assert(x) 1
 #else
 #define my_assert(x)                                                           \
   ((void)((x) ? 1 : abort()))
 #endif
 
 #ifdef NDEBUG
int main() {
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() with side effect
 
   assert((delete mcp, false));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() with side effect
 
   assert((throw 1, false));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() with side effect
 
-  assert2(1 == 2 - 1);
-
   return 0;
 }

test/clang-tidy/misc-noexcept-move-constructor.cpp

-// RUN: $(dirname %s)/check_clang_tidy.sh %s misc-noexcept-move-constructor %t
-// REQUIRES: shell
-
-class A {
-  A(A &&);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [misc-noexcept-move-constructor]
-  A &operator=(A &&);
-  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should
-};
-
-struct B {
-  static constexpr bool kFalse = false;
-  B(B &&) noexcept(kFalse);
-  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [misc-noexcept-move-constructor]
-};
-
-class OK {};
-
-void f() {
-  OK a;
-  a = OK();
-}
-
-class OK1 {
- public:
-  OK1();
-  OK1(const OK1 &);
-  OK1(OK1&&) noexcept;
-  OK1 &operator=(OK1 &&) noexcept;
-  void f();
-  void g() noexcept;
-};
-
-class OK2 {
-  static constexpr bool kTrue = true;
-
-public:
-  OK2(OK2 &&) noexcept(true) {}
-  OK2 &operator=(OK2 &&) noexcept(kTrue) { return *this; }
-};
-
-struct OK3 {
-  OK3(OK3 &&) noexcept(false) {}
-  OK3 &operator=(OK3 &&) = delete;
-};

test/clang-tidy/misc-static-assert.cpp

 // RUN: $(dirname %s)/check_clang_tidy.sh %s misc-static-assert %t
 // REQUIRES: shell
 
 void abort() {}
 #ifdef NDEBUG
 #define assert(x) 1
 #else
 #define assert(x)                                                              \
   if (!(x))                                                                    \
   abort()
 #endif
 
-void print(...);
-
 #define ZERO_MACRO 0
 
 #define False false
 #define FALSE 0
 
 #define my_macro() assert(0 == 1)
 // CHECK-FIXES: #define my_macro() assert(0 == 1)
 
 constexpr bool myfunc(int a, int b) { return a * b == 0; }
 
-typedef __SIZE_TYPE__ size_t;
-extern "C" size_t strlen(const char *s);
-
 class A {
 public:
   bool method() { return true; }
 };
 
 class B {
 public:
   constexpr bool method() { return true; }

 #define false 0
   assert(false);
 #undef false
 
   assert(10==5 && "Report me!");
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be
   // CHECK-FIXES: {{^  }}static_assert(10==5 , "Report me!");
 
-  assert(strlen("12345") == 5);
-  // CHECK-FIXES: {{^  }}assert(strlen("12345") == 5);
-
-#define assert(e) (__builtin_expect(!(e), 0) ? print (#e, __FILE__, __LINE__) : (void)0)
-  assert(false);
-  // CHECK-FIXES: {{^  }}assert(false);
-
-  assert(10 == 5 + 5);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be
-  // CHECK-FIXES: {{^  }}static_assert(10 == 5 + 5, "");
-#undef assert
-
   return 0;
 }

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

+// RUN: $(dirname %s)/check_clang_tidy.sh %s readability-redundant-void-arg %t -- -x c
+// REQUIRES: shell
+
+#include <stdio.h>
+
+extern int i;
+
+int foo2() {
+  return 0;
+}
+
+int j = 1;
+
+int foo(void) {
+// CHECK-FIXES: {{^}}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);
+// CHECK-FIXES: {{^}}int (*returns_fn_void_int(void))(void);{{$}}
+
+typedef int (*returns_fn_void_int_t(void))(void);
+// CHECK-FIXES: {{^}}typedef int (*returns_fn_void_int_t(void))(void);{{$}}
+
+int (*returns_fn_void_int(void))(void) {
+// CHECK-FIXES: {{^}}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);
+// CHECK-FIXES: {{^}}void (*(*returns_fn_returns_fn_void_void(void))(void))(void);{{$}}
+
+typedef void (*(*returns_fn_returns_fn_void_void_t(void))(void))(void);
+// CHECK-FIXES: {{^}}typedef void (*(*returns_fn_returns_fn_void_void_t(void))(void))(void);{{$}}
+
+void (*(*returns_fn_returns_fn_void_void(void))(void))(void) {
+// CHECK-FIXES: {{^}}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);
+// CHECK-FIXES: {{^}}void (*f1)(void);{{$}}
+
+void (*f2)(void) = NULL;
+// CHECK-FIXES: {{^}}void (*f2)(void) = NULL;{{$}}
+
+void (*f3)(void) = bar;
+// CHECK-FIXES: {{^}}void (*f3)(void) = bar;{{$}}
+
+void (*fa)();
+
+void (*fb)() = NULL;
+
+void (*fc)() = bar;
+
+typedef void (function_ptr)(void);
+// CHECK-FIXES: {{^}}typedef void (function_ptr)(void);{{$}}
+
+// intentionally not LLVM style to check preservation of whitespace
+typedef void (function_ptr2)
+    (
+        void
+    );
+// CHECK-FIXES:      {{^}}typedef void (function_ptr2){{$}}
+// CHECK-FIXES-NEXT: {{^    \($}}
+// CHECK-FIXES-NEXT: {{^        void$}}
+// CHECK-FIXES-NEXT: {{^    \);$}}
+
+// intentionally not LLVM style to check preservation of whitespace
+typedef
+void
+(
+*
+(
+*
+returns_fn_returns_fn_void_void_t2
+(
+void
+)
+)
+(
+void
+)
+)
+(
+void
+)
+;
+// 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-NEXT: {{^void$}}
+// CHECK-FIXES-NEXT: {{^\)$}}
+// CHECK-FIXES-NEXT: {{^\)$}}
+// CHECK-FIXES-NEXT: {{^\($}}
+// CHECK-FIXES-NEXT: {{^void$}}
+// CHECK-FIXES-NEXT: {{^\)$}}
+// CHECK-FIXES-NEXT: {{^\)$}}
+// CHECK-FIXES-NEXT: {{^\($}}
+// CHECK-FIXES-NEXT: {{^void$}}
+// CHECK-FIXES-NEXT: {{^\)$}}
+// CHECK-FIXES-NEXT: {{^;$}}

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

+// RUN: $(dirname %s)/check_clang_tidy.sh %s readability-redundant-void-arg %t
+// REQUIRES: shell
+
+#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 [readability-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 C style cast
+  // 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 C style cast
+  // 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; };{{$}}
+}

test/clang-tidy/readability-simplify-bool-expr-chained-conditional-assignment.cpp

-// RUN: $(dirname %s)/check_clang_tidy.sh %s readability-simplify-boolean-expr %t -config="{CheckOptions: [{key: "readability-simplify-boolean-expr.ChainedConditionalAssignment", value: 1}]}" --
-// REQUIRES: shell
-
-void chained_conditional_compound_assignment(int i) {
-  bool b;
-  if (i < 0) {
-    b = true;
-  } else if (i < 10) {
-    b = false;
-  } else if (i > 20) {
-    b = true;
-  } else {
-    b = false;
-  }
-  // CHECK-MESSAGES: :[[@LINE-4]]:9: warning: redundant boolean literal in conditional assignment [readability-simplify-boolean-expr]
-  // CHECK-FIXES:      {{^}}  } else if (i < 10) {{{$}}
-  // CHECK-FIXES-NEXT: {{^}}    b = false;{{$}}
-  // CHECK-FIXES-NEXT: {{^}}  } else b = i > 20;{{$}}
-}
-
-void chained_conditional_assignment(int i) {
-  bool b;
-  if (i < 0)
-    b = true;
-  else if (i < 10)
-    b = false;
-  else if (i > 20)
-    b = true;
-  else
-    b = false;
-  // CHECK-MESSAGES: :[[@LINE-3]]:9: warning: {{.*}} in conditional assignment
-  // CHECK-FIXES:      {{^}}  else if (i < 10)
-  // CHECK-FIXES-NEXT: {{^}}    b = false;
-  // CHECK-FIXES-NEXT: {{^}}  else b = i > 20;
-}

test/clang-tidy/readability-simplify-bool-expr-chained-conditional-return.cpp

-// RUN: $(dirname %s)/check_clang_tidy.sh %s readability-simplify-boolean-expr %t -config="{CheckOptions: [{key: "readability-simplify-boolean-expr.ChainedConditionalReturn", value: 1}]}" --
-// REQUIRES: shell
-
-bool chained_conditional_compound_return(int i) {
-  if (i < 0) {
-    return true;
-  } else if (i < 10) {
-    return false;
-  } else if (i > 20) {
-    return true;
-  } else {
-    return false;
-  }
-  // CHECK-MESSAGES: :[[@LINE-4]]:12: warning: redundant boolean literal in conditional return statement [readability-simplify-boolean-expr]
-  // CHECK-FIXES:      {{^}}  } else if (i < 10) {{{$}}
-  // CHECK-FIXES-NEXT: {{^}}    return false;{{$}}
-  // CHECK-FIXES-NEXT: {{^}}  } else return i > 20;{{$}}
-}
-
-bool chained_conditional_return(int i) {
-  if (i < 0)
-    return true;
-  else if (i < 10)
-    return false;
-  else if (i > 20)
-    return true;
-  else
-    return false;
-  // CHECK-MESSAGES: :[[@LINE-3]]:12: warning: {{.*}} in conditional return statement
-  // CHECK-FIXES:      {{^}}  else if (i < 10)
-  // CHECK-FIXES-NEXT: {{^}}    return false;
-  // CHECK-FIXES-NEXT: {{^}}  else return i > 20;
-}

test/clang-tidy/readability-simplify-bool-expr.cpp

void complex_conditional_assignment_statements(int i) {
   // unchanged; multiple statements
   bool h;
   if (j > 10) {
     j = 10;
     f = true;
   } else
     f = false;
 }
-
-// unchanged: chained return statements, but ChainedConditionalReturn not set
-bool chained_conditional_compound_return(int i) {
-  if (i < 0) {
-    return true;
-  } else if (i < 10) {
-    return false;
-  } else if (i > 20) {
-    return true;
-  } else {
-    return false;
-  }
-}
-
-// unchanged: chained return statements, but ChainedConditionalReturn not set
-bool chained_conditional_return(int i) {
-  if (i < 0)
-    return true;
-  else if (i < 10)
-    return false;
-  else if (i > 20)
-    return true;
-  else
-    return false;
-}
-
-// unchanged: chained assignments, but ChainedConditionalAssignment not set
-void chained_conditional_compound_assignment(int i) {
-  bool b;
-  if (i < 0) {
-    b = true;
-  } else if (i < 10) {
-    b = false;
-  } else if (i > 20) {
-    b = true;
-  } else {
-    b = false;
-  }
-}
-
-// unchanged: chained return statements, but ChainedConditionalReturn not set
-void chained_conditional_assignment(int i) {
-  bool b;
-  if (i < 0)
-    b = true;
-  else if (i < 10)
-    b = false;
-  else if (i > 20)
-    b = true;
-  else
-    b = false;
-}

test/lit.cfg

if platform.system() == 'Windows':
     path = lit_config.getToolsPath(path,
                                    config.environment['PATH'],
                                    ['cmp.exe', 'grep.exe', 'sed.exe'])
     if path is not None:
         path = os.path.pathsep.join((path,
                                      config.environment['PATH']))
         config.environment['PATH'] = path
 
-# Choose between lit's internal shell pipeline runner and a real shell.  If
-# LIT_USE_INTERNAL_SHELL is in the environment, we use that as an override.
-use_lit_shell = os.environ.get("LIT_USE_INTERNAL_SHELL")
-if use_lit_shell:
-    # 0 is external, "" is default, and everything else is internal.
-    execute_external = (use_lit_shell == "0")
-else:
-    # Otherwise we default to internal on Windows and external elsewhere, as
-    # bash on Windows is usually very slow.
-    execute_external = (not sys.platform in ['win32'])
-
 # testFormat: The test format to use to interpret tests.
 #
 # For now we require '&&' between commands, until they get globally killed and
 # the test runner updated.
+execute_external = (platform.system() != 'Windows'
+                    or lit_config.getBashPath() not in [None, ""])
 config.test_format = lit.formats.ShTest(execute_external)
 
 # suffixes: A list of file extensions to treat as test files.
 config.suffixes = ['.c', '.cpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s', '.modularize', '.module-map-checker']
 
 # Test-time dependencies located in directories called 'Inputs' are excluded
 # from test suites; there won't be any lit tests within them.
 config.excludes = ['Inputs']

 
 # Set available features we allow tests to conditionalize on.
 #
 # As of 2011.08, crash-recovery tests still do not pass on FreeBSD.
 if platform.system() not in ['FreeBSD']:
     config.available_features.add('crash-recovery')
 
 # Shell execution
-if execute_external:
+if platform.system() not in ['Windows'] or lit_config.getBashPath() != '':
     config.available_features.add('shell')
 
-# Exclude MSYS due to transforming '/' to 'X:/mingwroot/'.
-if not platform.system() in ['Windows'] or not execute_external:
-    config.available_features.add('shell-preserves-root')
-
 # ANSI escape sequences in non-dumb terminal
 if platform.system() not in ['Windows']:
     config.available_features.add('ansi-escape-sequences')
 
 config.substitutions.append( ('%python', config.python_executable) )
 
 import sys
 # Scripts using argparse need Python 2.7.
 if sys.version_info >= (2, 7):
   config.available_features.add('python27')