firolino added a comment.
In https://reviews.llvm.org/D27621#1248849, @JonasToth wrote:
> Hi @firolino,
> I implemented a less general version of you check
> https://reviews.llvm.org/D51949 as I wanted to achieve a slightly different
> goal (and this revision seems to be abonded). My aim is to
JonasToth added a comment.
Hi @firolino,
I implemented a less general version of you check
https://reviews.llvm.org/D51949 as I wanted to achieve a slightly different
goal (and this revision seems to be abonded). My aim is to have general
transformation capabilities to separate declarations (va
firolino marked an inline comment as done.
firolino added a comment.
In https://reviews.llvm.org/D27621#1061562, @lebedev.ri wrote:
> @firolino do you plan on finishing this?
Sometimes the universe is really funny! I have just re-started working on it
today.
https://reviews.llvm.org/D27621
lebedev.ri added a comment.
@firolino do you plan on finishing this?
https://reviews.llvm.org/D27621
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
JonasToth added a comment.
Still work on this one?
https://reviews.llvm.org/D27621
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
alexfh added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:130
+if (isa(*(FirstVarIt + 1)))
+ return "typedef " + TypeString;
+
Should we suggest `using x = ...;` in C++11 code?
Comment at: clang-ti
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:37-38
+ const auto *DeclStatement = Result.Nodes.getNodeAs("declstmt");
+ if (!DeclStatement)
+return;
+
Is there a case where this could happen? I would
firolino marked 7 inline comments as done.
firolino added a comment.
- nothing special, just went through some open comments and marked them as Done.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:246
+
+static std::string getCurrentLineIndent(SourceLocation
firolino added a comment.
@aaron.ballman I am going to implement `CppCore`, `Cert` and `Everything` and
test it on some projects and provide the results next week, to find out which
one to set `Default`.
https://reviews.llvm.org/D27621
___
cfe-com
firolino added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.h:25
+class OneNamePerDeclarationCheck : public ClangTidyCheck {
+private:
+ std::string getUserWrittenType(const DeclStmt *DeclStmt, SourceManager &SM);
firolino wrote
firolino added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:153
+ const SourceRange FVLoc(DeclStmt->getLocStart(), Location);
+ std::string UserWrittenType =
+ Lexer::getSourceText(CharSourceRange::getCharRange(FVLoc), SM,
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
A few more comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:153
+ const SourceRange FVLoc(DeclStmt->getLocStart(), Location);
+ std::st
firolino added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:34
+ // a tag declaration (e.g. struct, class etc.):
+ // class A { } Object1, Object2; <-- won't be matched
+ Finder->addMatcher(
aaron.ballman wrote:
> firolin
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:34
+ // a tag declaration (e.g. struct, class etc.):
+ // class A { } Object1, Object2; <-- won't be matched
+ Finder->addMatcher(
firolino wrote:
> aaron.b
firolino added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:34
+ // a tag declaration (e.g. struct, class etc.):
+ // class A { } Object1, Object2; <-- won't be matched
+ Finder->addMatcher(
aaron.ballman wrote:
> firolin
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:34
+ // a tag declaration (e.g. struct, class etc.):
+ // class A { } Object1, Object2; <-- won't be matched
+ Finder->addMatcher(
firolino wrote:
> aaron.b
firolino added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:34
+ // a tag declaration (e.g. struct, class etc.):
+ // class A { } Object1, Object2; <-- won't be matched
+ Finder->addMatcher(
aaron.ballman wrote:
> firolin
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:34
+ // a tag declaration (e.g. struct, class etc.):
+ // class A { } Object1, Object2; <-- won't be matched
+ Finder->addMatcher(
firolino wrote:
> aaron.b
firolino updated the summary for this revision.
firolino updated this revision to Diff 82855.
firolino marked an inline comment as done.
firolino added a comment.
- applied suggestions from Aaron
- added support and test cases for TagDecl e.g.
struct S {} S1, S2;
https://reviews.llvm.org/D276
firolino marked 5 inline comments as done.
firolino added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:34
+ // a tag declaration (e.g. struct, class etc.):
+ // class A { } Object1, Object2; <-- won't be matched
+ Finder->addMatcher(
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:34
+ // a tag declaration (e.g. struct, class etc.):
+ // class A { } Object1, Object2; <-- won't be matched
+ Finder->addMatcher(
firolino wrote:
> firolin
firolino added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:34
+ // a tag declaration (e.g. struct, class etc.):
+ // class A { } Object1, Object2; <-- won't be matched
+ Finder->addMatcher(
firolino wrote:
> firolino wro
firolino added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:34
+ // a tag declaration (e.g. struct, class etc.):
+ // class A { } Object1, Object2; <-- won't be matched
+ Finder->addMatcher(
firolino wrote:
> firolino wro
firolino marked 3 inline comments as done.
firolino added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:34
+ // a tag declaration (e.g. struct, class etc.):
+ // class A { } Object1, Object2; <-- won't be matched
+ Finder->addMatcher(
firolino marked 27 inline comments as done.
firolino added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:34
+ // a tag declaration (e.g. struct, class etc.):
+ // class A { } Object1, Object2; <-- won't be matched
+ Finder->addMatcher(
---
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:22
+
+const internal::VariadicDynCastAllOfMatcher tagDecl;
+
firolino wrote:
> aaron.ballman wrote:
> > We may want to consider adding this to ASTMatchers.h at
firolino marked 18 inline comments as done.
firolino added a comment.
Addressed suggestions from @aaron.ballman Thanks!
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:22
+
+const internal::VariadicDynCastAllOfMatcher tagDecl;
+
aaron.ballman
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:22
+
+const internal::VariadicDynCastAllOfMatcher tagDecl;
+
We may want to consider adding this to ASTMatchers.h at some point (can be done
in a future patch)
firolino updated this revision to Diff 81964.
firolino added a comment.
Added support for new test cases:
int S::*mdpa1[2] = {&S::a, &S::a}, var1 = 0;
// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration statement can be split
// CHECK-FIXES: {{^}}int S::*mdpa1[2] = {&S::a, &S::a
firolino updated this revision to Diff 81731.
firolino added a comment.
further cleanup in getUserWrittenType
https://reviews.llvm.org/D27621
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/OneNamePerDeclarationCheck.cpp
clang-tidy/readability/OneNamePerDeclarationChec
firolino updated this revision to Diff 81730.
firolino added a comment.
little cleanup in getUserWrittenType
https://reviews.llvm.org/D27621
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/OneNamePerDeclarationCheck.cpp
clang-tidy/readability/OneNamePerDeclarationCheck
firolino updated this revision to Diff 81729.
firolino added a comment.
small improvement
https://reviews.llvm.org/D27621
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/OneNamePerDeclarationCheck.cpp
clang-tidy/readability/OneNamePerDeclarationCheck.h
clang-tidy/rea
firolino updated this revision to Diff 81605.
firolino added a comment.
Added fix and test case for `const int * const`.
https://reviews.llvm.org/D27621
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/OneNamePerDeclarationCheck.cpp
clang-tidy/readability/OneNamePerDecl
firolino added a comment.
Unfortunately, I am not able to make getUserWrittenType easier right now.
TypeLoc does not give me the desired range. See also:
- http://lists.llvm.org/pipermail/cfe-dev/2013-March/028467.html
- http://lists.llvm.org/pipermail/cfe-dev/2010-April/008664.html
- https://re
firolino updated this revision to Diff 81591.
firolino added a comment.
- small improvements
- re-added lost handling for:
bool defPre = false,
#ifdef IS_ENABLED
defTest = false;
#else
defTest = true;
#endif
- added support for new test cases:
int /* :: */ S::*pp2 =
Eugene.Zelenko added a comment.
I think will be good idea to extend check for class members.
Comment at: test/clang-tidy/readability-one-name-per-declaration-modern.cpp:13
+ public:
+vector() {}
+vector(initializer_list init) {}
Please use
firolino marked an inline comment as done.
firolino added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:154
+ if (const auto *MemberPointerT = Type->getAs()) {
+auto Pos = UserWrittenType.find("::");
+if (Pos != std::string::npos) { /
firolino added a comment.
In https://reviews.llvm.org/D27621#620464, @rsmith wrote:
> Please add a test to make sure this does not fire on C++17 decomposition
> declarations:
>
> void f() {
> struct X { int a, b, c; };
> auto [a, b, c] = X();
> }
Done. Thanks!
https://reviews.llv
malcolm.parsons added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:154
+ if (const auto *MemberPointerT = Type->getAs()) {
+auto Pos = UserWrittenType.find("::");
+if (Pos != std::string::npos) { // might be hidden behind typedef etc
rsmith added a comment.
Please add a test to make sure this does not fire on C++17 decomposition
declarations:
void f() {
struct X { int a, b, c; };
auto [a, b, c] = X();
}
https://reviews.llvm.org/D27621
___
cfe-commits mailing list
c
firolino updated this revision to Diff 81154.
firolino marked an inline comment as done.
firolino added a comment.
Small improvments:
- SourceRange -> SourceLocation (malcolms last comment)
- added a few consts
https://reviews.llvm.org/D27621
Files:
clang-tidy/readability/CMakeLists.txt
cl
firolino marked 2 inline comments as done.
firolino added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:86
+
+ Diag << FixItHint::CreateReplacement(CommaRange, ";")
+ << FixItHint::CreateReplacement(AfterCommaToVarNameRange,
---
firolino marked an inline comment as done.
firolino added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:87
+ Diag << FixItHint::CreateReplacement(CommaRange, ";")
+ << FixItHint::CreateReplacement(AfterCommaToVarNameRange,
+
malcolm.parsons added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:86
+
+ Diag << FixItHint::CreateReplacement(CommaRange, ";")
+ << FixItHint::CreateReplacement(AfterCommaToVarNameRange,
A `SourceLocation` can
firolino updated this revision to Diff 81101.
firolino marked an inline comment as done.
firolino added a comment.
Simplified fix-it hint generation.
https://reviews.llvm.org/D27621
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/OneNamePerDeclarationCheck.cpp
clang-ti
firolino marked 2 inline comments as done.
firolino added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:133
+VariableLocation.setBegin(tidy::utils::lexer::findLocationAfterToken(
+VariableLocation.getEnd(), Tokens, *Result.Context)
arphaman added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:133
+VariableLocation.setBegin(tidy::utils::lexer::findLocationAfterToken(
+VariableLocation.getEnd(), Tokens, *Result.Context));
+ }
If you use `ArrayR
firolino added a comment.
In https://reviews.llvm.org/D27621#619649, @malcolm.parsons wrote:
> The fixit construction looks overly complicated.
Yes, you were right. I have changed a couple of things. It looks better now.
Still working on a better getUserWrittenType.
https://reviews.llvm.org/
firolino added a comment.
In https://reviews.llvm.org/D27621#619649, @malcolm.parsons wrote:
> The fixit construction looks overly complicated.
> All you need to do is change a `,` to a `;` and insert a copy of the type:
>
> << FixItHint::CreateReplacement(CommaRange, ";")
> << FixItHint::Cr
malcolm.parsons added a comment.
The fixit construction looks overly complicated.
All you need to do is change a `,` to a `;` and insert a copy of the type:
<< FixItHint::CreateReplacement(CommaRange, ";")
<< FixItHint::CreateInsertionFromRange(VarLocation, TypeRange)
and insert some whitesp
firolino updated this revision to Diff 81039.
firolino added a comment.
A few namespace cleanups
- Removed explicit llvm::'s, since the used names are already introduced into
the clang namespace by llvm.h
https://reviews.llvm.org/D27621
Files:
clang-tidy/readability/CMakeLists.txt
clang-t
firolino added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:246
+
+static std::string getCurrentLineIndent(SourceLocation Loc,
+const SourceManager &SM) {
malcolm.parsons wrote:
> Shoul
firolino updated this revision to Diff 81029.
firolino marked an inline comment as done.
firolino added a comment.
Little typo fix.
https://reviews.llvm.org/D27621
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/OneNamePerDeclarationCheck.cpp
clang-tidy/readability/One
firolino updated this revision to Diff 81023.
firolino added a comment.
typdef -> typedef
https://reviews.llvm.org/D27621
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/OneNamePerDeclarationCheck.cpp
clang-tidy/readability/OneNamePerDeclarationCheck.h
clang-tidy/rea
firolino updated this revision to Diff 81022.
firolino marked 7 inline comments as done.
firolino added a comment.
Applied suggestions from malcolm.
https://reviews.llvm.org/D27621
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/OneNamePerDeclarationCheck.cpp
clang-tid
malcolm.parsons added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:48
+ // Single declarations and macros will be ignored
+ if (DeclStmt->isSingleDecl() || DeclStmt->getLocStart().isMacroID())
+return;
Can you reject si
firolino updated this revision to Diff 81001.
firolino added a comment.
Added test case with struct keyword:
struct StructOne cs1, cs2( 42 );
https://reviews.llvm.org/D27621
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/OneNamePerDeclarationCheck.cpp
clang-tidy/re
firolino added a comment.
@Eugene.Zelenko No modernize-* check was reported. I applied //most// of the
readability-* checks. For example, I did not put braces around:
if (DeclStmt == nullptr)
return;
if (DeclStmt->isSingleDecl() || DeclStmt->getLocStart().isMacroID())
return;
firolino updated this revision to Diff 81000.
firolino marked 9 inline comments as done.
firolino added a comment.
Applied readability-* checks.
https://reviews.llvm.org/D27621
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/OneNamePerDeclarationCheck.cpp
clang-tidy/re
firolino added a comment.
In https://reviews.llvm.org/D27621#618379, @JDevlieghere wrote:
> Some small stuff I noticed while reading through the code, I didn't check it
> in much detail though.
Thanks, fixed it.
https://reviews.llvm.org/D27621
_
firolino updated this revision to Diff 80916.
firolino added a comment.
Addressed comment from JDevlieghere
https://reviews.llvm.org/D27621
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/OneNamePerDeclarationCheck.cpp
clang-tidy/readability/OneNamePerDeclarationCheck.
Eugene.Zelenko added inline comments.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:47
+// Single declarations and macros will be ignored
+if (DeclStmt->isSingleDecl() == false &&
+DeclStmt->getLocStart().isMacroID() == false) {
---
firolino removed rL LLVM as the repository for this revision.
firolino updated this revision to Diff 80915.
firolino added a comment.
Updated docs/ReleaseNotes.txt
https://reviews.llvm.org/D27621
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/OneNamePerDeclarationCheck.
JDevlieghere added a comment.
Some small stuff I noticed while reading through the code, I didn't check it in
much detail though.
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:43
+void OneNamePerDeclarationCheck::check(const MatchFinder::MatchResult &Result
firolino added a comment.
I have agreed with Kirill to continue this. See
http://lists.llvm.org/pipermail/cfe-dev/2016-October/051270.html
I would suggest to close https://reviews.llvm.org/D25024.
Repository:
rL LLVM
https://reviews.llvm.org/D27621
Eugene.Zelenko added a comment.
Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).
This is duplicate of https://reviews.llvm.org/D25024. Will be goo idea to
combine code in one. Please note that there are multiple coding guidelines
which suggest this style.
https://r
firolino added a comment.
I have tested it on the whole llvm/clang/extra source code without any problems.
https://reviews.llvm.org/D27621
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
firolino created this revision.
firolino added reviewers: alexfh, aaron.ballman, klimek, malcolm.parsons.
firolino added a subscriber: cfe-commits.
Herald added subscribers: JDevlieghere, mgorny.
This check can be used to find declarations, which declare more than one name.
It helps improving rea
68 matches
Mail list logo