[PATCH] D18914: [clang-tidy] new readability-redundant-inline

Izaron added a comment. In D18914#2399705 , @chfast wrote: > This check can be useful in other case like this: > > inline constexpr const int x = 1; > > where `inline` and `const` are redundant. In your case `inline` is not actually redundant (at least

[PATCH] D18914: [clang-tidy] new readability-redundant-inline

chfast added a comment. This check can be useful in other case like this: inline constexpr const int x = 1; where `inline` and `const` are redundant. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D18914/new/ https://reviews.llvm.org/D18914

[PATCH] D18914: [clang-tidy] new readability-redundant-inline

mgehre added a comment. Thank you for the review input! After learning about `inlinehint`, I don't feel its worthwhile for me to continue this check. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D18914/new/ https://reviews.llvm.org/D18914

[PATCH] D18914: [clang-tidy] new readability-redundant-inline

sbenza added a comment. In D18914#396149 , @mgehre wrote: > ... > I personally think that 1) should be used, because late one could move the > function definition to a source file (removing the inline) without having to > touch > the class declaration.

[PATCH] D18914: [clang-tidy] new readability-redundant-inline

aaron.ballman added inline comments. Comment at: clang-tidy/readability/RedundantInlineCheck.cpp:24 +AST_MATCHER(FriendDecl, isInlineAndHasBody) { + NamedDecl *D = Node.getFriendDecl(); + if (!D) const NamedDecl (same for const auto * below).

[PATCH] D18914: [clang-tidy] new readability-redundant-inline

mgehre added a comment. Friendly ping https://reviews.llvm.org/D18914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18914: [clang-tidy] new readability-redundant-inline

mgehre updated this revision to Diff 56800. mgehre added a comment. Update for review comment: - Return Optional when looking for 'inline' - Add test that hides 'inline' in a macro http://reviews.llvm.org/D18914 Files: clang-tidy/readability/CMakeLists.txt

Re: [PATCH] D18914: [clang-tidy] new readability-redundant-inline

sbenza added inline comments. Comment at: clang-tidy/readability/RedundantInlineCheck.cpp:68 @@ +67,3 @@ + } + llvm_unreachable("InlineTok() did not encounter the 'inline' token"); +} This is still reachable. The token might be hidden through macros, for

Re: [PATCH] D18914: [clang-tidy] new readability-redundant-inline

mgehre added inline comments. Comment at: test/clang-tidy/readability-redundant-inline.cpp:6 @@ +5,3 @@ +// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 'inline' is redundant because method body is defined inside class [readability-redundant-inline] +// CHECK-FIXES: {{^}} int f1()

Re: [PATCH] D18914: [clang-tidy] new readability-redundant-inline

rsmith added a subscriber: rsmith. Comment at: test/clang-tidy/readability-redundant-inline.cpp:5 @@ +4,3 @@ + inline int f1() { +// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 'inline' is redundant because method body is defined inside class [readability-redundant-inline] +//

Re: [PATCH] D18914: [clang-tidy] new readability-redundant-inline

mgehre updated this revision to Diff 53317. mgehre added a comment. Removed debug output http://reviews.llvm.org/D18914 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp clang-tidy/readability/RedundantInlineCheck.cpp

Re: [PATCH] D18914: [clang-tidy] new readability-redundant-inline

mgehre updated this revision to Diff 53315. mgehre added a comment. Update for review comments; simplified lexing, added proposed test case http://reviews.llvm.org/D18914 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp

Re: [PATCH] D18914: [clang-tidy] new readability-redundant-inline

mgehre added a comment. I'm thinking about extending the check to the following issue and would like to hear your opinion. In C++, the following three code snippets all have identical meaning 1: struct S { int f(); }; inline int S::f() { return 0; } 2: struct S {

Re: [PATCH] D18914: [clang-tidy] new readability-redundant-inline

mgehre updated this revision to Diff 53118. mgehre added a comment. Update for review comments http://reviews.llvm.org/D18914 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp clang-tidy/readability/RedundantInlineCheck.cpp

Re: [PATCH] D18914: [clang-tidy] new readability-redundant-inline

Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/readability-redundant-inline.rst:6 @@ +5,3 @@ + +This check flags redundant 'inline' specifiers. +It flags 'inline' on member functions defined inside a class definition like Please use ``

Re: [PATCH] D18914: [clang-tidy] new readability-redundant-inline

Eugene.Zelenko added a subscriber: Eugene.Zelenko. Eugene.Zelenko added a comment. Please mention this check in docs/ReleaseNotes.rst (in alphabetical order). http://reviews.llvm.org/D18914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D18914: [clang-tidy] new readability-redundant-inline

mgehre created this revision. mgehre added reviewers: sbenza, bkramer, aaron.ballman, alexfh. mgehre added a subscriber: cfe-commits. This check flags redundant 'inline' specifiers. It flags 'inline' on member functions defined inside a class definition like .. code-block:: c++ struct S {