This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Splits fuchsia-default-arguments
ClosedPublic

Authored by DiegoAstiazaran on May 24 2019, 4:27 PM.

Details

Summary

Splits fuchsia-default-arguments check into two checks. fuchsia-default-arguments-calls warns if a function or method is called with default arguments. fuchsia-default-arguments-declarations warns if a function or method is declared with default parameters.

Resolves b38051

Diff Detail

Event Timeline

Please mention renaming of existing check in Release Notes (after list of new checks).

clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsDeclarationsCheck.cpp
24

You could use auto instead of type because type is spelled in cast.

25

Please run clang-format.

Herald added a project: Restricted Project. · View Herald TranscriptMay 24 2019, 4:45 PM
Eugene.Zelenko added inline comments.May 24 2019, 4:47 PM
clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsCallsCheck.cpp
2

Please add space after clang-tidy.

clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsCallsCheck.h
1

Please narrow to 80 symbols and add space after clang-tidy.

clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsDeclarationsCheck.h
2

Please narrow to 80 symbols.

clang-tools-extra/clang-tidy/fuchsia/FuchsiaTidyModule.cpp
1

Please add space after clang-tidy.

Reduces lines to 80 characters. The files have been also formatted by clang-format.
Types are replaced by auto in declarations where the type is spelled in cast.

aaron.ballman requested changes to this revision.May 27 2019, 8:05 AM

It seems that the functional changes are missing from the patch -- all I see are formatting changes; have I missed something?

clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsCallsCheck.cpp
24–25

Given that the only change in the file is a formatting change, I would revert this. Feel free to commit separately as an NFC commit if you plan to do future work in the file.

clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsDeclarationsCheck.h
2

This should similarly be a separate NFC commit.

This revision now requires changes to proceed.May 27 2019, 8:05 AM
DiegoAstiazaran updated this revision to Diff 201799.EditedMay 28 2019, 5:34 PM

Fix patch submitted.
Patch submitted with the last update was a new commit, functional changes were not included in that diff.
New patch includes functional changes (first patch) and format changes (second patch / first update).

hokein added a comment.Jun 6 2019, 5:53 AM

@juliehockett could you take a look on this patch? I think you have the best knowledge about this check and fuchsia stuff.

Sorry for the delay!

clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsCallsCheck.cpp
24

Just if (!S) should be sufficient here.

clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsDeclarationsCheck.cpp
25

Same here for the if as above.

clang-tools-extra/docs/ReleaseNotes.rst
118–122

Also include a line about the addition of fuchsia-default-arguments-calls and the removal of fuchsia-default-arguments.

clang-tools-extra/test/clang-tidy/fuchsia-default-arguments-declarations.cpp
59–60

Remove this

Complete documentation in ReleaseNotes

DiegoAstiazaran marked 12 inline comments as done.Jun 6 2019, 2:31 PM
Eugene.Zelenko added inline comments.Jun 6 2019, 2:36 PM
clang-tools-extra/docs/ReleaseNotes.rst
122

Please highlight fuchsia-default-arguments with single back-ticks.

129

Please highlight fuchsia-default-arguments with single back-ticks.

167

Please add links to fuchsia-default-arguments-calls and fuchsia-default-arguments-declarations.

Improve format of documentation in ReleaseNotes.

juliehockett accepted this revision.Jun 11 2019, 2:53 PM
DiegoAstiazaran marked 3 inline comments as done.Jun 11 2019, 3:44 PM
This revision was not accepted when it landed; it landed in state Needs Review.Jun 18 2019, 11:08 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 18 2019, 11:08 AM
clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsCallsCheck.cpp