Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes
berenm added a comment. Hello, I had a little bit of look into the NestingLevel field. I understand that it only indicates the nesting level of the token inside the current unwrapped line, which could very well be the same as the nesting level of another token in the previous or next unwrapped line. Right now, it doesn't include the information on the nested level of the unwrapped line itself (this is in the Level field of the line itself). I'm not sure if the value of IndentLevel comes directly from the unwrapped line's Level, but I believe that combining them would give an equivalent of a "global" nesting level of the token. In order to be cleaner, I think it could be done in the AnnotatedLineParser class, that already fills the NestingLevel field. I wrote a patch that demonstrates this here: https://reviews.llvm.org/D24859. All the unit tests are passing, although I'm not 100% sure about all the implications this changes has. With this patch, I believed NestingLevel can be used directly to determine begin and end of alignment sequences, without requiring the ScopeLevel helper function. https://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes
berenm added a comment. In https://reviews.llvm.org/D21279#550565, @djasper wrote: > Are we talking completely past each other? I specifically think we should > *NOT* combine NestingLevel and IndentLevel into one value. Not in > ScopeLevel() and not anywhere else. Ok, I probably misunderstood the situation, sorry. https://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12369: [clang-format] Fix alignConsecutiveAssignments not working properly
berenm added a comment. I don't have credentials to commit it, anybody could do it for me if the diff is ok? Thanks! http://reviews.llvm.org/D12369 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12362: [clang-format] Add support of consecutive declarations alignment
berenm added a comment. Ping? The unit tests should work fine, now that http://reviews.llvm.org/D12369 has been merged. http://reviews.llvm.org/D12362 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
berenm changed the visibility of this Differential Revision from "berenm (Beren
Minor)" to "Public (No Login Required)".
berenm updated this revision to Diff 35465.
berenm added a comment.
Remove remaining commented code.
http://reviews.llvm.org/D13081
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -104,6 +104,9 @@
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_class'
// CHECK-FIXES: {{^}}class CMyClass {{{$}}
my_class();
+// CHECK-FIXES: {{^}}CMyClass();{{$}}
+~my_class();
+// CHECK-FIXES: {{^}}~CMyClass();{{$}}
const int MEMBER_one_1 = ConstExpr_variable;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: invalid case style for constant member 'MEMBER_one_1'
@@ -137,15 +140,36 @@
const int my_class::classConstant = 4;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class constant 'classConstant'
-// CHECK-FIXES: {{^}}const int my_class::kClassConstant = 4;{{$}}
-// FIXME: The fixup should reflect class name fixups as well:
-// FIXME: {{^}}const int CMyClass::kClassConstant = 4;{{$}}
+// CHECK-FIXES: {{^}}const int CMyClass::kClassConstant = 4;{{$}}
int my_class::ClassMember_2 = 5;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class member 'ClassMember_2'
-// CHECK-FIXES: {{^}}int my_class::ClassMember2 = 5;{{$}}
-// FIXME: The fixup should reflect class name fixups as well:
-// FIXME: {{^}}int CMyClass::ClassMember2 = 5;{{$}}
+// CHECK-FIXES: {{^}}int CMyClass::ClassMember2 = 5;{{$}}
+
+class my_derived_class : public virtual my_class {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_derived_class'
+// CHECK-FIXES: {{^}}class CMyDerivedClass : public virtual CMyClass {};{{$}}
+
+class CMyWellNamedClass {};
+// No warning expected as this class is well named.
+
+template
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for type template parameter 'T'
+// CHECK-FIXES: {{^}}template{{$}}
+class my_templated_class : CMyWellNamedClass {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_templated_class'
+// CHECK-FIXES: {{^}}class CMyTemplatedClass : CMyWellNamedClass {};{{$}}
+
+template
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for type template parameter 'T'
+// CHECK-FIXES: {{^}}template{{$}}
+class my_other_templated_class : my_templated_class, private my_derived_class {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_other_templated_class'
+// CHECK-FIXES: {{^}}class CMyOtherTemplatedClass : CMyTemplatedClass, private CMyDerivedClass {};{{$}}
+
+template
+using MYSUPER_TPL = my_other_templated_class<::FOO_NS::my_class>;
+// CHECK-FIXES: {{^}}using MYSUPER_TPL = CMyOtherTemplatedClass<::foo_ns::CMyClass>;{{$}}
const int global_Constant = 6;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global constant 'global_Constant'
@@ -186,7 +210,7 @@
void Global_Fun(TYPE_parameters... PARAMETER_PACK) {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global function 'Global_Fun'
// CHECK-MESSAGES: :[[@LINE-2]]:17: warning: invalid case style for parameter pack 'PARAMETER_PACK'
-// CHECK-FIXES: {{^}}void GlobalFun(TYPE_parameters... parameterPack) {{{$}}
+// CHECK-FIXES: {{^}}void GlobalFun(typeParameters_t... parameterPack) {{{$}}
global_function(1, 2);
// CHECK-FIXES: {{^}}GlobalFunction(1, 2);{{$}}
FOO_bar = Global_variable;
@@ -214,6 +238,8 @@
void non_Virtual_METHOD() {}
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for private method 'non_Virtual_METHOD'
// CHECK-FIXES: {{^}}void __non_Virtual_METHOD() {}{{$}}
+
+public:
static void CLASS_METHOD() {}
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for class method 'CLASS_METHOD'
// CHECK-FIXES: {{^}}static void classMethod() {}{{$}}
@@ -242,10 +268,8 @@
struct THIS___Structure {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for struct 'THIS___Structure'
-// CHECK-FIXES: {{^}}struct this_structure {{{$}}
+// CHECK-FIXES: {{^}}this_structure();{{$}}
THIS___Structure();
-// FIXME: There should be a fixup for the constructor as well
-// FIXME: {{^}}this_structure();{{$}}
union __MyUnion_is_wonderful__ {};
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: invalid case style for union '__MyUnion_is_wonderful__'
@@ -254,13 +278,19 @@
typedef THIS___Structure struct_type;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for typedef 'struct_type'
-// CHECK-FIXES: {{^}}typedef THIS___Structure struct_type_t;{{$}}
-// FIXME: The fixup should reflect structure name fixups as well:
-// F
Re: [PATCH] D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
berenm updated this revision to Diff 35466.
berenm added a comment.
Fix incorrect line removal in unit test.
http://reviews.llvm.org/D13081
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -104,6 +104,9 @@
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_class'
// CHECK-FIXES: {{^}}class CMyClass {{{$}}
my_class();
+// CHECK-FIXES: {{^}}CMyClass();{{$}}
+~my_class();
+// CHECK-FIXES: {{^}}~CMyClass();{{$}}
const int MEMBER_one_1 = ConstExpr_variable;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: invalid case style for constant member 'MEMBER_one_1'
@@ -137,15 +140,36 @@
const int my_class::classConstant = 4;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class constant 'classConstant'
-// CHECK-FIXES: {{^}}const int my_class::kClassConstant = 4;{{$}}
-// FIXME: The fixup should reflect class name fixups as well:
-// FIXME: {{^}}const int CMyClass::kClassConstant = 4;{{$}}
+// CHECK-FIXES: {{^}}const int CMyClass::kClassConstant = 4;{{$}}
int my_class::ClassMember_2 = 5;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class member 'ClassMember_2'
-// CHECK-FIXES: {{^}}int my_class::ClassMember2 = 5;{{$}}
-// FIXME: The fixup should reflect class name fixups as well:
-// FIXME: {{^}}int CMyClass::ClassMember2 = 5;{{$}}
+// CHECK-FIXES: {{^}}int CMyClass::ClassMember2 = 5;{{$}}
+
+class my_derived_class : public virtual my_class {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_derived_class'
+// CHECK-FIXES: {{^}}class CMyDerivedClass : public virtual CMyClass {};{{$}}
+
+class CMyWellNamedClass {};
+// No warning expected as this class is well named.
+
+template
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for type template parameter 'T'
+// CHECK-FIXES: {{^}}template{{$}}
+class my_templated_class : CMyWellNamedClass {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_templated_class'
+// CHECK-FIXES: {{^}}class CMyTemplatedClass : CMyWellNamedClass {};{{$}}
+
+template
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for type template parameter 'T'
+// CHECK-FIXES: {{^}}template{{$}}
+class my_other_templated_class : my_templated_class, private my_derived_class {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_other_templated_class'
+// CHECK-FIXES: {{^}}class CMyOtherTemplatedClass : CMyTemplatedClass, private CMyDerivedClass {};{{$}}
+
+template
+using MYSUPER_TPL = my_other_templated_class<::FOO_NS::my_class>;
+// CHECK-FIXES: {{^}}using MYSUPER_TPL = CMyOtherTemplatedClass<::foo_ns::CMyClass>;{{$}}
const int global_Constant = 6;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global constant 'global_Constant'
@@ -186,7 +210,7 @@
void Global_Fun(TYPE_parameters... PARAMETER_PACK) {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global function 'Global_Fun'
// CHECK-MESSAGES: :[[@LINE-2]]:17: warning: invalid case style for parameter pack 'PARAMETER_PACK'
-// CHECK-FIXES: {{^}}void GlobalFun(TYPE_parameters... parameterPack) {{{$}}
+// CHECK-FIXES: {{^}}void GlobalFun(typeParameters_t... parameterPack) {{{$}}
global_function(1, 2);
// CHECK-FIXES: {{^}}GlobalFunction(1, 2);{{$}}
FOO_bar = Global_variable;
@@ -214,6 +238,8 @@
void non_Virtual_METHOD() {}
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for private method 'non_Virtual_METHOD'
// CHECK-FIXES: {{^}}void __non_Virtual_METHOD() {}{{$}}
+
+public:
static void CLASS_METHOD() {}
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for class method 'CLASS_METHOD'
// CHECK-FIXES: {{^}}static void classMethod() {}{{$}}
@@ -244,8 +270,7 @@
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for struct 'THIS___Structure'
// CHECK-FIXES: {{^}}struct this_structure {{{$}}
THIS___Structure();
-// FIXME: There should be a fixup for the constructor as well
-// FIXME: {{^}}this_structure();{{$}}
+// CHECK-FIXES: {{^}}this_structure();{{$}}
union __MyUnion_is_wonderful__ {};
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: invalid case style for union '__MyUnion_is_wonderful__'
@@ -254,13 +279,19 @@
typedef THIS___Structure struct_type;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for typedef 'struct_type'
-// CHECK-FIXES: {{^}}typedef THIS___Structure struct_type_t;{{$}}
-// FIXME: The fixup should reflect structure name fixups as well:
-// FIXME: {{^}}typedef this_structure struct_type_t;{{$}}
+// CHECK-FIXES: {{^}}typedef this_structure struct_type_t;{{$}}
static void static_Fun
[PATCH] D13090: [clang-tidy] IdentifierNamingCheck should only emit warnings when declaration or usage is outside of macros
berenm created this revision.
berenm added a reviewer: alexfh.
berenm added a subscriber: cfe-commits.
This fixes https://llvm.org/bugs/show_bug.cgi?id=24835.
The patch might be slightly different after http://reviews.llvm.org/D13079 and
http://reviews.llvm.org/D13081, but the idea is the same.
http://reviews.llvm.org/D13090
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -567,10 +567,11 @@
SourceRange DeclRange =
DeclarationNameInfo(Decl.getDeclName(), Decl.getLocation())
.getSourceRange();
-auto Diag = diag(Decl.getLocStart(), "invalid case style for %0 '%1'")
-<< Failure.KindName << Decl.getName();
if (Failure.ShouldFix) {
+ auto Diag = diag(Decl.getLocStart(), "invalid case style for %0 '%1'")
+ << Failure.KindName << Decl.getName();
+
Diag << FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(DeclRange), Failure.Fixup);
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -567,10 +567,11 @@
SourceRange DeclRange =
DeclarationNameInfo(Decl.getDeclName(), Decl.getLocation())
.getSourceRange();
-auto Diag = diag(Decl.getLocStart(), "invalid case style for %0 '%1'")
-<< Failure.KindName << Decl.getName();
if (Failure.ShouldFix) {
+ auto Diag = diag(Decl.getLocStart(), "invalid case style for %0 '%1'")
+ << Failure.KindName << Decl.getName();
+
Diag << FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(DeclRange), Failure.Fixup);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13090: [clang-tidy] IdentifierNamingCheck should only emit warnings when declaration or usage is outside of macros
berenm updated this revision to Diff 35468.
berenm added a comment.
Even better with the corresponding unit test fix.
http://reviews.llvm.org/D13090
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -77,8 +79,7 @@
#define BLA int FOO_bar
BLA;
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global
variable 'FOO_bar'
-// NO fix expected as FOO_bar is from macro expansion
+// NO warnings or fixes expected as FOO_bar is from macro expansion
enum my_enumeration {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for enum
'my_enumeration'
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -567,10 +567,11 @@
SourceRange DeclRange =
DeclarationNameInfo(Decl.getDeclName(), Decl.getLocation())
.getSourceRange();
-auto Diag = diag(Decl.getLocStart(), "invalid case style for %0 '%1'")
-<< Failure.KindName << Decl.getName();
if (Failure.ShouldFix) {
+ auto Diag = diag(Decl.getLocStart(), "invalid case style for %0 '%1'")
+ << Failure.KindName << Decl.getName();
+
Diag << FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(DeclRange), Failure.Fixup);
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -77,8 +79,7 @@
#define BLA int FOO_bar
BLA;
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global variable 'FOO_bar'
-// NO fix expected as FOO_bar is from macro expansion
+// NO warnings or fixes expected as FOO_bar is from macro expansion
enum my_enumeration {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for enum 'my_enumeration'
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -567,10 +567,11 @@
SourceRange DeclRange =
DeclarationNameInfo(Decl.getDeclName(), Decl.getLocation())
.getSourceRange();
-auto Diag = diag(Decl.getLocStart(), "invalid case style for %0 '%1'")
-<< Failure.KindName << Decl.getName();
if (Failure.ShouldFix) {
+ auto Diag = diag(Decl.getLocStart(), "invalid case style for %0 '%1'")
+ << Failure.KindName << Decl.getName();
+
Diag << FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(DeclRange), Failure.Fixup);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13079: [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck
berenm added inline comments.
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:508
@@ +507,3 @@
+ auto &Failure = Failures[Decl];
+ for (const auto &R : Failure.Usages) {
+if (R == Range)
Hopefully the number of ranges in Usages will stay low. If not, `Usages` should
be an hash table instead, but it looks like SourceRange isn't hashable as is.
http://reviews.llvm.org/D13079
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk
berenm added a comment. Ping? Just to be sure it's not going to fall to oblivion. :) http://reviews.llvm.org/D12407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk
berenm added a comment. Well, the format on save setting is disabled by default, do you mean you had to enable it twice? Or you had it enabled without the C++ development tools, and after the installation you had to disable and enable it again? http://reviews.llvm.org/D12407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13079: [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck
berenm updated this revision to Diff 35651.
berenm added a comment.
Changed Usages to RawUsageLocs, as DenseMap storing the raw encoding of
SourceLocation instead of a vector or SourceRange.
http://reviews.llvm.org/D13079
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tidy/readability/IdentifierNamingCheck.h
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -68,6 +68,8 @@
// FIXME: name, declaration contexts, forward declarations, etc, are correctly
// FIXME: checked and renamed
+// clang-format off
+
namespace FOO_NS {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for namespace 'FOO_NS' [readability-identifier-naming]
// CHECK-FIXES: {{^}}namespace foo_ns {{{$}}
Index: clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tidy/readability/IdentifierNamingCheck.h
@@ -62,20 +62,20 @@
}
};
-private:
- std::vector NamingStyles;
- bool IgnoreFailedSplit;
-
struct NamingCheckFailure {
std::string KindName;
std::string Fixup;
bool ShouldFix;
-std::vector Usages;
+llvm::DenseSet RawUsageLocs;
NamingCheckFailure() : ShouldFix(true) {}
};
+ typedef llvm::DenseMap NamingCheckFailureMap;
- llvm::DenseMap NamingCheckFailures;
+private:
+ std::vector NamingStyles;
+ bool IgnoreFailedSplit;
+ NamingCheckFailureMap NamingCheckFailures;
};
} // namespace readability
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -21,6 +21,7 @@
namespace tidy {
namespace readability {
+// clang-format off
#define NAMING_KEYS(m) \
m(Namespace) \
m(InlineNamespace) \
@@ -80,6 +81,7 @@
};
#undef NAMING_KEYS
+// clang-format on
IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
ClangTidyContext *Context)
@@ -134,10 +136,10 @@
}
void IdentifierNamingCheck::registerMatchers(MatchFinder *Finder) {
-// FIXME: For now, only Decl and DeclRefExpr nodes are visited for checking and
-// replacement. There is a lot of missing cases, such as references to a class
-// name (as in 'const int CMyClass::kClassConstant = 4;'), to an enclosing
-// context (namespace, class, etc).
+ // FIXME: For now, only Decl and DeclRefExpr nodes are visited for checking
+ // and replacement. There is a lot of missing cases, such as references to a
+ // class name (as in 'const int CMyClass::kClassConstant = 4;'), to an
+ // enclosing context (namespace, class, etc).
Finder->addMatcher(namedDecl().bind("decl"), this);
Finder->addMatcher(declRefExpr().bind("declref"), this);
@@ -499,23 +501,24 @@
return SK_Invalid;
}
+static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
+ const NamedDecl *Decl, SourceRange Range,
+ const SourceManager *SM) {
+ auto &Failure = Failures[Decl];
+ if (!Failure.RawUsageLocs.insert(Range.getBegin().getRawEncoding()).second)
+return;
+
+ Failure.ShouldFix = Failure.ShouldFix && SM->isInMainFile(Range.getBegin()) &&
+ SM->isInMainFile(Range.getEnd()) &&
+ !Range.getBegin().isMacroID() &&
+ !Range.getEnd().isMacroID();
+}
+
void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) {
if (const auto *DeclRef = Result.Nodes.getNodeAs("declref")) {
-auto It = NamingCheckFailures.find(DeclRef->getDecl());
-if (It == NamingCheckFailures.end())
- return;
-
-NamingCheckFailure &Failure = It->second;
SourceRange Range = DeclRef->getNameInfo().getSourceRange();
-
-Failure.Usages.push_back(Range);
-Failure.ShouldFix = Failure.ShouldFix &&
-Result.SourceManager->isInMainFile(Range.getBegin()) &&
-Result.SourceManager->isInMainFile(Range.getEnd()) &&
-!Range.getBegin().isMacroID() &&
-!Range.getEnd().isMacroID();
-
-return;
+return addUsage(NamingCheckFailures, DeclRef->getDecl(), Range,
+Result.SourceManager);
}
if (const auto *Decl = Result.Nodes.getNodeAs("decl")) {
@@ -550,11 +553,7 @@
Failure.Fixup = std::move(Fixup);
Failure.KindName = std::move(KindName);
- Failure.ShouldFix =
- Failure.ShouldFix &&
- Result.SourceManager->isInMainFile(Range.getBegin()) &&
- Result.SourceManager->isInMainFile(Range.getEnd()) &&
- !Range.getBegin().isMacroID() && !Range
Re: [PATCH] D13079: [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck
berenm marked 2 inline comments as done. berenm added a comment. http://reviews.llvm.org/D13079 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13090: [clang-tidy] IdentifierNamingCheck should only emit warnings when declaration or usage is outside of macros
berenm updated this revision to Diff 35652.
berenm added a comment.
Update the diff with more context, thanks to arcanist.
http://reviews.llvm.org/D13090
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -77,8 +77,7 @@
#define BLA int FOO_bar
BLA;
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global
variable 'FOO_bar'
-// NO fix expected as FOO_bar is from macro expansion
+// NO warnings or fixes expected as FOO_bar is from macro expansion
enum my_enumeration {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for enum
'my_enumeration'
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -567,10 +567,11 @@
SourceRange DeclRange =
DeclarationNameInfo(Decl.getDeclName(), Decl.getLocation())
.getSourceRange();
-auto Diag = diag(Decl.getLocStart(), "invalid case style for %0 '%1'")
-<< Failure.KindName << Decl.getName();
if (Failure.ShouldFix) {
+ auto Diag = diag(Decl.getLocStart(), "invalid case style for %0 '%1'")
+ << Failure.KindName << Decl.getName();
+
Diag << FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(DeclRange), Failure.Fixup);
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -77,8 +77,7 @@
#define BLA int FOO_bar
BLA;
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global variable 'FOO_bar'
-// NO fix expected as FOO_bar is from macro expansion
+// NO warnings or fixes expected as FOO_bar is from macro expansion
enum my_enumeration {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for enum 'my_enumeration'
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -567,10 +567,11 @@
SourceRange DeclRange =
DeclarationNameInfo(Decl.getDeclName(), Decl.getLocation())
.getSourceRange();
-auto Diag = diag(Decl.getLocStart(), "invalid case style for %0 '%1'")
-<< Failure.KindName << Decl.getName();
if (Failure.ShouldFix) {
+ auto Diag = diag(Decl.getLocStart(), "invalid case style for %0 '%1'")
+ << Failure.KindName << Decl.getName();
+
Diag << FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(DeclRange), Failure.Fixup);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
berenm updated this revision to Diff 35655.
berenm added a comment.
Update the diff with more context.
http://reviews.llvm.org/D13081
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -104,6 +104,9 @@
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_class'
// CHECK-FIXES: {{^}}class CMyClass {{{$}}
my_class();
+// CHECK-FIXES: {{^}}CMyClass();{{$}}
+~my_class();
+// CHECK-FIXES: {{^}}~CMyClass();{{$}}
const int MEMBER_one_1 = ConstExpr_variable;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: invalid case style for constant member 'MEMBER_one_1'
@@ -137,15 +140,36 @@
const int my_class::classConstant = 4;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class constant 'classConstant'
-// CHECK-FIXES: {{^}}const int my_class::kClassConstant = 4;{{$}}
-// FIXME: The fixup should reflect class name fixups as well:
-// FIXME: {{^}}const int CMyClass::kClassConstant = 4;{{$}}
+// CHECK-FIXES: {{^}}const int CMyClass::kClassConstant = 4;{{$}}
int my_class::ClassMember_2 = 5;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class member 'ClassMember_2'
-// CHECK-FIXES: {{^}}int my_class::ClassMember2 = 5;{{$}}
-// FIXME: The fixup should reflect class name fixups as well:
-// FIXME: {{^}}int CMyClass::ClassMember2 = 5;{{$}}
+// CHECK-FIXES: {{^}}int CMyClass::ClassMember2 = 5;{{$}}
+
+class my_derived_class : public virtual my_class {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_derived_class'
+// CHECK-FIXES: {{^}}class CMyDerivedClass : public virtual CMyClass {};{{$}}
+
+class CMyWellNamedClass {};
+// No warning expected as this class is well named.
+
+template
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for type template parameter 'T'
+// CHECK-FIXES: {{^}}template{{$}}
+class my_templated_class : CMyWellNamedClass {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_templated_class'
+// CHECK-FIXES: {{^}}class CMyTemplatedClass : CMyWellNamedClass {};{{$}}
+
+template
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for type template parameter 'T'
+// CHECK-FIXES: {{^}}template{{$}}
+class my_other_templated_class : my_templated_class, private my_derived_class {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_other_templated_class'
+// CHECK-FIXES: {{^}}class CMyOtherTemplatedClass : CMyTemplatedClass, private CMyDerivedClass {};{{$}}
+
+template
+using MYSUPER_TPL = my_other_templated_class<::FOO_NS::my_class>;
+// CHECK-FIXES: {{^}}using MYSUPER_TPL = CMyOtherTemplatedClass<::foo_ns::CMyClass>;{{$}}
const int global_Constant = 6;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global constant 'global_Constant'
@@ -186,7 +210,7 @@
void Global_Fun(TYPE_parameters... PARAMETER_PACK) {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global function 'Global_Fun'
// CHECK-MESSAGES: :[[@LINE-2]]:17: warning: invalid case style for parameter pack 'PARAMETER_PACK'
-// CHECK-FIXES: {{^}}void GlobalFun(TYPE_parameters... parameterPack) {{{$}}
+// CHECK-FIXES: {{^}}void GlobalFun(typeParameters_t... parameterPack) {{{$}}
global_function(1, 2);
// CHECK-FIXES: {{^}}GlobalFunction(1, 2);{{$}}
FOO_bar = Global_variable;
@@ -214,6 +238,8 @@
void non_Virtual_METHOD() {}
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for private method 'non_Virtual_METHOD'
// CHECK-FIXES: {{^}}void __non_Virtual_METHOD() {}{{$}}
+
+public:
static void CLASS_METHOD() {}
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for class method 'CLASS_METHOD'
// CHECK-FIXES: {{^}}static void classMethod() {}{{$}}
@@ -244,23 +270,28 @@
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for struct 'THIS___Structure'
// CHECK-FIXES: {{^}}struct this_structure {{{$}}
THIS___Structure();
-// FIXME: There should be a fixup for the constructor as well
-// FIXME: {{^}}this_structure();{{$}}
+// CHECK-FIXES: {{^}}this_structure();{{$}}
union __MyUnion_is_wonderful__ {};
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: invalid case style for union '__MyUnion_is_wonderful__'
// CHECK-FIXES: {{^}} union UMyUnionIsWonderful {};{{$}}
};
typedef THIS___Structure struct_type;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for typedef 'struct_type'
-// CHECK-FIXES: {{^}}typedef THIS___Structure struct_type_t;{{$}}
-// FIXME: The fixup should reflect structure name fixups as well:
-// FIXME: {{^}}typedef this_structure struct_type_t;{{$}}
+// CHECK-FIXES: {{^}}typedef this_structure struct_t
Re: [PATCH] D13079: [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck
berenm updated this revision to Diff 35669.
berenm added a comment.
Reformatting with clang-format
http://reviews.llvm.org/D13079
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tidy/readability/IdentifierNamingCheck.h
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -68,6 +68,8 @@
// FIXME: name, declaration contexts, forward declarations, etc, are correctly
// FIXME: checked and renamed
+// clang-format off
+
namespace FOO_NS {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for namespace 'FOO_NS' [readability-identifier-naming]
// CHECK-FIXES: {{^}}namespace foo_ns {{{$}}
Index: clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tidy/readability/IdentifierNamingCheck.h
@@ -62,20 +62,20 @@
}
};
-private:
- std::vector NamingStyles;
- bool IgnoreFailedSplit;
-
struct NamingCheckFailure {
std::string KindName;
std::string Fixup;
bool ShouldFix;
-std::vector Usages;
+llvm::DenseSet RawUsageLocs;
NamingCheckFailure() : ShouldFix(true) {}
};
+ typedef llvm::DenseMap NamingCheckFailureMap;
- llvm::DenseMap NamingCheckFailures;
+private:
+ std::vector NamingStyles;
+ bool IgnoreFailedSplit;
+ NamingCheckFailureMap NamingCheckFailures;
};
} // namespace readability
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -21,6 +21,7 @@
namespace tidy {
namespace readability {
+// clang-format off
#define NAMING_KEYS(m) \
m(Namespace) \
m(InlineNamespace) \
@@ -80,6 +81,7 @@
};
#undef NAMING_KEYS
+// clang-format on
IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
ClangTidyContext *Context)
@@ -134,10 +136,10 @@
}
void IdentifierNamingCheck::registerMatchers(MatchFinder *Finder) {
-// FIXME: For now, only Decl and DeclRefExpr nodes are visited for checking and
-// replacement. There is a lot of missing cases, such as references to a class
-// name (as in 'const int CMyClass::kClassConstant = 4;'), to an enclosing
-// context (namespace, class, etc).
+ // FIXME: For now, only Decl and DeclRefExpr nodes are visited for checking
+ // and replacement. There is a lot of missing cases, such as references to a
+ // class name (as in 'const int CMyClass::kClassConstant = 4;'), to an
+ // enclosing context (namespace, class, etc).
Finder->addMatcher(namedDecl().bind("decl"), this);
Finder->addMatcher(declRefExpr().bind("declref"), this);
@@ -499,23 +501,24 @@
return SK_Invalid;
}
+static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
+ const NamedDecl *Decl, SourceRange Range,
+ const SourceManager *SM) {
+ auto &Failure = Failures[Decl];
+ if (!Failure.RawUsageLocs.insert(Range.getBegin().getRawEncoding()).second)
+return;
+
+ Failure.ShouldFix = Failure.ShouldFix && SM->isInMainFile(Range.getBegin()) &&
+ SM->isInMainFile(Range.getEnd()) &&
+ !Range.getBegin().isMacroID() &&
+ !Range.getEnd().isMacroID();
+}
+
void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) {
if (const auto *DeclRef = Result.Nodes.getNodeAs("declref")) {
-auto It = NamingCheckFailures.find(DeclRef->getDecl());
-if (It == NamingCheckFailures.end())
- return;
-
-NamingCheckFailure &Failure = It->second;
SourceRange Range = DeclRef->getNameInfo().getSourceRange();
-
-Failure.Usages.push_back(Range);
-Failure.ShouldFix = Failure.ShouldFix &&
-Result.SourceManager->isInMainFile(Range.getBegin()) &&
-Result.SourceManager->isInMainFile(Range.getEnd()) &&
-!Range.getBegin().isMacroID() &&
-!Range.getEnd().isMacroID();
-
-return;
+return addUsage(NamingCheckFailures, DeclRef->getDecl(), Range,
+Result.SourceManager);
}
if (const auto *Decl = Result.Nodes.getNodeAs("decl")) {
@@ -550,11 +553,7 @@
Failure.Fixup = std::move(Fixup);
Failure.KindName = std::move(KindName);
- Failure.ShouldFix =
- Failure.ShouldFix &&
- Result.SourceManager->isInMainFile(Range.getBegin()) &&
- Result.SourceManager->isInMainFile(Range.getEnd()) &&
- !Range.getBegin().isMacroID() && !Range.getEnd().isMacroID();
+ addUsage(NamingCheckFailures, Decl, Range, Result.SourceManager
Re: [PATCH] D13090: [clang-tidy] IdentifierNamingCheck should only emit warnings when declaration or usage is outside of macros
berenm added a comment. This will also disable all warnings for declaration / usages outside of the main file. It might be better to disable the warnings and fixes whenever a macro is involved (in the declaration or any usage), but at least keep the warning across files, even if we don't offer fixes. I think the most common use-case will be to have some declaration in a header file and usages in multiple source files, and in this case warnings are interesting. I'm assuming here that "main file" in clang tooling framework means the .cpp file from which the ast is built. http://reviews.llvm.org/D13090 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
berenm updated this revision to Diff 35672.
berenm added a comment.
- Do not check for identifier names from system headers
- Check for SourceLocation validity before modifying them
http://reviews.llvm.org/D13081
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -64,12 +64,11 @@
// RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0} \
// RUN: ]}' -- -std=c++11 -fno-delayed-template-parsing
-// FIXME: There should be more test cases for checking that references to class
-// FIXME: name, declaration contexts, forward declarations, etc, are correctly
-// FIXME: checked and renamed
-
// clang-format off
+#include
+// Expect NO warnings or errors from system headers, it shouldn't even be checked
+
namespace FOO_NS {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for namespace 'FOO_NS' [readability-identifier-naming]
// CHECK-FIXES: {{^}}namespace foo_ns {{{$}}
@@ -104,6 +103,9 @@
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_class'
// CHECK-FIXES: {{^}}class CMyClass {{{$}}
my_class();
+// CHECK-FIXES: {{^}}CMyClass();{{$}}
+~my_class();
+// CHECK-FIXES: {{^}}~CMyClass();{{$}}
const int MEMBER_one_1 = ConstExpr_variable;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: invalid case style for constant member 'MEMBER_one_1'
@@ -137,15 +139,36 @@
const int my_class::classConstant = 4;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class constant 'classConstant'
-// CHECK-FIXES: {{^}}const int my_class::kClassConstant = 4;{{$}}
-// FIXME: The fixup should reflect class name fixups as well:
-// FIXME: {{^}}const int CMyClass::kClassConstant = 4;{{$}}
+// CHECK-FIXES: {{^}}const int CMyClass::kClassConstant = 4;{{$}}
int my_class::ClassMember_2 = 5;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class member 'ClassMember_2'
-// CHECK-FIXES: {{^}}int my_class::ClassMember2 = 5;{{$}}
-// FIXME: The fixup should reflect class name fixups as well:
-// FIXME: {{^}}int CMyClass::ClassMember2 = 5;{{$}}
+// CHECK-FIXES: {{^}}int CMyClass::ClassMember2 = 5;{{$}}
+
+class my_derived_class : public virtual my_class {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_derived_class'
+// CHECK-FIXES: {{^}}class CMyDerivedClass : public virtual CMyClass {};{{$}}
+
+class CMyWellNamedClass {};
+// No warning expected as this class is well named.
+
+template
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for type template parameter 'T'
+// CHECK-FIXES: {{^}}template{{$}}
+class my_templated_class : CMyWellNamedClass {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_templated_class'
+// CHECK-FIXES: {{^}}class CMyTemplatedClass : CMyWellNamedClass {};{{$}}
+
+template
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for type template parameter 'T'
+// CHECK-FIXES: {{^}}template{{$}}
+class my_other_templated_class : my_templated_class, private my_derived_class {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_other_templated_class'
+// CHECK-FIXES: {{^}}class CMyOtherTemplatedClass : CMyTemplatedClass, private CMyDerivedClass {};{{$}}
+
+template
+using MYSUPER_TPL = my_other_templated_class<::FOO_NS::my_class>;
+// CHECK-FIXES: {{^}}using MYSUPER_TPL = CMyOtherTemplatedClass<::foo_ns::CMyClass>;{{$}}
const int global_Constant = 6;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global constant 'global_Constant'
@@ -186,7 +209,7 @@
void Global_Fun(TYPE_parameters... PARAMETER_PACK) {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global function 'Global_Fun'
// CHECK-MESSAGES: :[[@LINE-2]]:17: warning: invalid case style for parameter pack 'PARAMETER_PACK'
-// CHECK-FIXES: {{^}}void GlobalFun(TYPE_parameters... parameterPack) {{{$}}
+// CHECK-FIXES: {{^}}void GlobalFun(typeParameters_t... parameterPack) {{{$}}
global_function(1, 2);
// CHECK-FIXES: {{^}}GlobalFunction(1, 2);{{$}}
FOO_bar = Global_variable;
@@ -214,6 +237,8 @@
void non_Virtual_METHOD() {}
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for private method 'non_Virtual_METHOD'
// CHECK-FIXES: {{^}}void __non_Virtual_METHOD() {}{{$}}
+
+public:
static void CLASS_METHOD() {}
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for class method 'CLASS_METHOD'
// CHECK-FIXES: {{^}}static void classMethod() {}{{$}}
@@ -244,23 +269,28 @@
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for struct 'THIS___Structure'
// CHECK-FIXES: {{^}}struct this_structure {{{$}}
THIS___Structure()
Re: [PATCH] D13079: [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck
berenm updated this revision to Diff 35818.
berenm added a comment.
Added comments and reformated NamingCheckIdentifer.h with clang-format.
http://reviews.llvm.org/D13079
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tidy/readability/IdentifierNamingCheck.h
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -68,6 +68,8 @@
// FIXME: name, declaration contexts, forward declarations, etc, are correctly
// FIXME: checked and renamed
+// clang-format off
+
namespace FOO_NS {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for namespace 'FOO_NS' [readability-identifier-naming]
// CHECK-FIXES: {{^}}namespace foo_ns {{{$}}
Index: clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tidy/readability/IdentifierNamingCheck.h
@@ -62,20 +62,32 @@
}
};
-private:
- std::vector NamingStyles;
- bool IgnoreFailedSplit;
-
+ /// \brief Holds an identifier name check failure, tracking the kind of the
+ /// identifer, its possible fixup and the starting locations of all the
+ /// idenfiier usages.
struct NamingCheckFailure {
std::string KindName;
std::string Fixup;
+
+/// \brief Whether the failure should be fixed or not.
+///
+/// ie: if the identifier was used or declared within a macro we won't offer
+/// a fixup for safety reasons.
bool ShouldFix;
-std::vector Usages;
+
+/// \brief A set of all the identifier usages starting SourceLocation, in
+/// their encoded form.
+llvm::DenseSet RawUsageLocs;
NamingCheckFailure() : ShouldFix(true) {}
};
+ typedef llvm::DenseMap
+ NamingCheckFailureMap;
- llvm::DenseMap NamingCheckFailures;
+private:
+ std::vector NamingStyles;
+ bool IgnoreFailedSplit;
+ NamingCheckFailureMap NamingCheckFailures;
};
} // namespace readability
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -21,6 +21,7 @@
namespace tidy {
namespace readability {
+// clang-format off
#define NAMING_KEYS(m) \
m(Namespace) \
m(InlineNamespace) \
@@ -80,6 +81,7 @@
};
#undef NAMING_KEYS
+// clang-format on
IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
ClangTidyContext *Context)
@@ -134,10 +136,10 @@
}
void IdentifierNamingCheck::registerMatchers(MatchFinder *Finder) {
-// FIXME: For now, only Decl and DeclRefExpr nodes are visited for checking and
-// replacement. There is a lot of missing cases, such as references to a class
-// name (as in 'const int CMyClass::kClassConstant = 4;'), to an enclosing
-// context (namespace, class, etc).
+ // FIXME: For now, only Decl and DeclRefExpr nodes are visited for checking
+ // and replacement. There is a lot of missing cases, such as references to a
+ // class name (as in 'const int CMyClass::kClassConstant = 4;'), to an
+ // enclosing context (namespace, class, etc).
Finder->addMatcher(namedDecl().bind("decl"), this);
Finder->addMatcher(declRefExpr().bind("declref"), this);
@@ -499,23 +501,24 @@
return SK_Invalid;
}
+static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
+ const NamedDecl *Decl, SourceRange Range,
+ const SourceManager *SM) {
+ auto &Failure = Failures[Decl];
+ if (!Failure.RawUsageLocs.insert(Range.getBegin().getRawEncoding()).second)
+return;
+
+ Failure.ShouldFix = Failure.ShouldFix && SM->isInMainFile(Range.getBegin()) &&
+ SM->isInMainFile(Range.getEnd()) &&
+ !Range.getBegin().isMacroID() &&
+ !Range.getEnd().isMacroID();
+}
+
void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) {
if (const auto *DeclRef = Result.Nodes.getNodeAs("declref")) {
-auto It = NamingCheckFailures.find(DeclRef->getDecl());
-if (It == NamingCheckFailures.end())
- return;
-
-NamingCheckFailure &Failure = It->second;
SourceRange Range = DeclRef->getNameInfo().getSourceRange();
-
-Failure.Usages.push_back(Range);
-Failure.ShouldFix = Failure.ShouldFix &&
-Result.SourceManager->isInMainFile(Range.getBegin()) &&
-Result.SourceManager->isInMainFile(Range.getEnd()) &&
-!Range.getBegin().isMacroID() &&
-!Range.getEnd().isMacroID();
-
-return;
+return addUsage(NamingCheckFailures, DeclRef->getDecl(), Range,
+Result.S
Re: [PATCH] D13079: [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck
berenm marked 3 inline comments as done. berenm added a comment. In http://reviews.llvm.org/D13079#253512, @alexfh wrote: > Please tell me, if you need me to commit the patch for you after you address > the comments. Yes please, I cannot commit it myself. http://reviews.llvm.org/D13079 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
berenm updated this revision to Diff 35819.
berenm added a comment.
Reworked the patches a little bit, addressing the comments and adding
better handling of checks in header files with corresponding unit test cases.
http://reviews.llvm.org/D13081
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/Inputs/readability-identifier-naming/system/system-header.h
test/clang-tidy/Inputs/readability-identifier-naming/user-header.h
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -62,25 +62,44 @@
// RUN: {key: readability-identifier-naming.VirtualMethodCase, value: UPPER_CASE}, \
// RUN: {key: readability-identifier-naming.VirtualMethodPrefix, value: 'v_'}, \
// RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0} \
-// RUN: ]}' -- -std=c++11 -fno-delayed-template-parsing
-
-// FIXME: There should be more test cases for checking that references to class
-// FIXME: name, declaration contexts, forward declarations, etc, are correctly
-// FIXME: checked and renamed
+// RUN: ]}' -- -std=c++11 -fno-delayed-template-parsing \
+// RUN: -I%S/Inputs/readability-identifier-naming \
+// RUN: -isystem %S/Inputs/readability-identifier-naming/system
// clang-format off
+#include
+#include "user-header.h"
+// NO warnings or fixes expected from declarations within header files without
+// the -header-filter= option
+
namespace FOO_NS {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for namespace 'FOO_NS' [readability-identifier-naming]
// CHECK-FIXES: {{^}}namespace foo_ns {{{$}}
inline namespace InlineNamespace {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for inline namespace 'InlineNamespace'
// CHECK-FIXES: {{^}}inline namespace inline_namespace {{{$}}
+SYSTEM_NS::structure g_s1;
+// NO warnings or fixes expected as SYSTEM_NS and structure are declared in a header file
+
+USER_NS::object g_s2;
+// NO warnings or fixes expected as USER_NS and object are declared in a header file
+
+SYSTEM_MACRO(var1);
+// NO warnings or fixes expected as var1 is from macro expansion
+
+USER_MACRO(var2);
+// NO warnings or fixes expected as var2 is declared in a macro expansion
+
+int global;
+#define USE_IN_MACRO(m) auto use_##m = m
+USE_IN_MACRO(global);
+// NO warnings or fixes expected as global is used in a macro expansion
+
#define BLA int FOO_bar
BLA;
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global variable 'FOO_bar'
-// NO fix expected as FOO_bar is from macro expansion
+// NO warnings or fixes expected as FOO_bar is from macro expansion
enum my_enumeration {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for enum 'my_enumeration'
@@ -104,6 +123,9 @@
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_class'
// CHECK-FIXES: {{^}}class CMyClass {{{$}}
my_class();
+// CHECK-FIXES: {{^}}CMyClass();{{$}}
+~my_class();
+// CHECK-FIXES: {{^}}~CMyClass();{{$}}
const int MEMBER_one_1 = ConstExpr_variable;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: invalid case style for constant member 'MEMBER_one_1'
@@ -137,15 +159,36 @@
const int my_class::classConstant = 4;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class constant 'classConstant'
-// CHECK-FIXES: {{^}}const int my_class::kClassConstant = 4;{{$}}
-// FIXME: The fixup should reflect class name fixups as well:
-// FIXME: {{^}}const int CMyClass::kClassConstant = 4;{{$}}
+// CHECK-FIXES: {{^}}const int CMyClass::kClassConstant = 4;{{$}}
int my_class::ClassMember_2 = 5;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class member 'ClassMember_2'
-// CHECK-FIXES: {{^}}int my_class::ClassMember2 = 5;{{$}}
-// FIXME: The fixup should reflect class name fixups as well:
-// FIXME: {{^}}int CMyClass::ClassMember2 = 5;{{$}}
+// CHECK-FIXES: {{^}}int CMyClass::ClassMember2 = 5;{{$}}
+
+class my_derived_class : public virtual my_class {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_derived_class'
+// CHECK-FIXES: {{^}}class CMyDerivedClass : public virtual CMyClass {};{{$}}
+
+class CMyWellNamedClass {};
+// No warning expected as this class is well named.
+
+template
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for type template parameter 'T'
+// CHECK-FIXES: {{^}}template{{$}}
+class my_templated_class : CMyWellNamedClass {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_templated_class'
+// CHECK-FIXES: {{^}}class CMyTemplatedClass : CMyWellNamedClass {};{{$}}
+
+template
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for type template parameter 'T'
+// CHECK-FIXES: {{^}}template{{$}}
+class my_other_t
Re: [PATCH] D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
berenm marked 3 inline comments as done.
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:141
@@ +140,3 @@
+ Finder->addMatcher(usingDecl().bind("using"), this);
+ Finder->addMatcher(declRefExpr().bind("declRef"), this);
+ Finder->addMatcher(cxxConstructorDecl().bind("classRef"), this);
I wasn't sure how system headers were handled, and I thought it was a waste of
time to check identifiers in them. But as you say it could be useful, I rolled
back the change.
From what I've seen, clang-tidy will display the warnings from incorrect
declaration names in system header files when using the `-system-headers` and
`-header-filter=.*` flags, but will not offer fixes for the declaration or
usages in user's code. I guess editing system headers isn't going to work fine?
For user headers, warnings and fixes are emitted when `-header-filter=.*` is
used.
Without the flag, no warnings or fixes are emitted, which is expected.
http://reviews.llvm.org/D13081
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
berenm marked an inline comment as done. berenm added a comment. I think the latest version also addresses http://reviews.llvm.org/D13079 in a better way. The check will now let clang-tidy decide whether warnings and fixes are displayed, whether it is in or from system/user header files or in main file, and will only worry about not emitting warnings and fixes if a macro is involved in the declaration or any usage. http://reviews.llvm.org/D13081 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
berenm marked an inline comment as done. berenm added a comment. http://reviews.llvm.org/D13081 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13090: [clang-tidy] IdentifierNamingCheck should only emit warnings when declaration or usage is outside of macros
berenm abandoned this revision. berenm added a comment. Actually I believe it's better fixed by http://reviews.llvm.org/D13081. http://reviews.llvm.org/D13090 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk
berenm updated this revision to Diff 35946.
berenm added a comment.
Update the diff with the ProvideAutoLoad attribute.
Thanks @MyDeveloperDay for the review and tips!
http://reviews.llvm.org/D12407
Files:
tools/clang-format-vs/ClangFormat/ClangFormat.csproj
tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
Index: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
===
--- tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
+++ tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
@@ -12,12 +12,14 @@
//
//===--===//
+using Microsoft.VisualStudio;
using Microsoft.VisualStudio.Editor;
using Microsoft.VisualStudio.Shell;
using Microsoft.VisualStudio.Shell.Interop;
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.TextManager.Interop;
+using Microsoft.VisualStudio.ComponentModelHost;
using System;
using System.ComponentModel;
using System.ComponentModel.Design;
@@ -53,28 +55,54 @@
get { return style; }
set { style = value; }
}
+
+[Category("LLVM/Clang")]
+[DisplayName("Reformat On Save")]
+[Description("Reformat code when the source file is saved on disk")]
+public bool ReformatOnSave
+{
+get;
+set;
+}
}
[PackageRegistration(UseManagedResourcesOnly = true)]
[InstalledProductRegistration("#110", "#112", "1.0", IconResourceID = 400)]
[ProvideMenuResource("Menus.ctmenu", 1)]
[Guid(GuidList.guidClangFormatPkgString)]
[ProvideOptionPage(typeof(OptionPageGrid), "LLVM/Clang", "ClangFormat", 0, 0, true)]
-public sealed class ClangFormatPackage : Package
+[ProvideAutoLoad(UIContextGuids80.NoSolution)]
+public sealed class ClangFormatPackage : Package, IVsRunningDocTableEvents3
{
+private IVsEditorAdaptersFactoryService editorAdaptersFactoryService;
+private IVsRunningDocumentTable runningDocumentTable;
+private uint adviseCookie;
+
#region Package Members
protected override void Initialize()
{
base.Initialize();
+IComponentModel componentModel = GetService(typeof(SComponentModel)) as IComponentModel;
+editorAdaptersFactoryService = componentModel.GetService();
+runningDocumentTable = GetService(typeof(IVsRunningDocumentTable)) as IVsRunningDocumentTable;
+runningDocumentTable.AdviseRunningDocTableEvents(this, out adviseCookie);
+
var commandService = GetService(typeof(IMenuCommandService)) as OleMenuCommandService;
if (commandService != null)
{
var menuCommandID = new CommandID(GuidList.guidClangFormatCmdSet, (int)PkgCmdIDList.cmdidClangFormat);
var menuItem = new MenuCommand(MenuItemCallback, menuCommandID);
commandService.AddCommand(menuItem);
}
}
+
+protected override void Dispose(bool disposing)
+{
+runningDocumentTable.UnadviseRunningDocTableEvents(adviseCookie);
+
+base.Dispose(disposing);
+}
#endregion
private void MenuItemCallback(object sender, EventArgs args)
@@ -92,10 +120,16 @@
if (start >= text.Length && text.Length > 0)
start = text.Length - 1;
string path = GetDocumentParent(view);
+FormatTextBuffer(view.TextBuffer, start, length, path);
+}
+
+private void FormatTextBuffer(ITextBuffer textBuffer, int offset, int length, string path)
+{
try
{
-var root = XElement.Parse(RunClangFormat(text, start, length, path));
-var edit = view.TextBuffer.CreateEdit();
+string text = textBuffer.CurrentSnapshot.GetText();
+var root = XElement.Parse(RunClangFormat(text, offset, length, path));
+var edit = textBuffer.CreateEdit();
foreach (XElement replacement in root.Descendants("replacement"))
{
var span = new Span(
@@ -199,7 +233,7 @@
var userData = (IVsUserData)textView;
if (userData == null)
return null;
-Guid guidWpfViewHost = DefGuidList.guidIWpfTextViewHost;
+Guid guidWpfViewHost = Microsoft.VisualStudio.Editor.DefGuidList.guidIWpfTextViewHost;
object host;
userData.GetData(ref guidWpfViewHost, out host);
return ((IWpfTextViewHost)host).TextView;
@@ -211,6 +245,12 @@
return page.Style;
}
+private bool ShouldReformatOnSave()
+{
+var page = (OptionPageGrid)GetDialogPage(typeof(OptionPageGrid));
+return page.ReformatOnSave;
+}
+
private
Re: [PATCH] D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
berenm added inline comments.
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:545
@@ +544,3 @@
+ if (const auto *Loc = Result.Nodes.getNodeAs("typeLoc")) {
+if (const auto &Ref = Loc->getAs()) {
+ addUsage(NamingCheckFailures, Ref.getDecl(), Loc->getSourceRange(),
alexfh wrote:
> The four cases are too similar. It should be possible to write the code much
> shorter. This might work:
>
> if (isa(Loc) || isa(Loc) || ...)
> addUsage(NamingCheckFailures, Loc->getType()->getDecl(),
> Loc->getSourceRange(), Result.SourceManager);
>
It doesn't look to work well with `TypeLoc`. This looks to be a convoluted
class hierarchy (http://clang.llvm.org/doxygen/classclang_1_1TypeLoc.html) and
getDecl() is only available on some derived `TypeLoc` classes.
I can change to something smaller, is that any better?
```
NamedDecl *Decl = nullptr;
if (const auto &Ref = Loc->getAs()) {
Decl = Ref.getDecl();
} else if (const auto &Ref = Loc->getAs()) {
Decl = Ref.getDecl();
} else if (const auto &Ref = Loc->getAs()) {
Decl = Ref.getDecl();
} else if (const auto &Ref = Loc->getAs()) {
Decl = Ref.getDecl();
}
if (Decl) {
addUsage(NamingCheckFailures, Decl, Loc->getSourceRange(),
Result.SourceManager);
return;
}
```
http://reviews.llvm.org/D13081
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
berenm updated this revision to Diff 35972.
berenm added a comment.
Address the latest comments, as well as useless code removal.
- Only the start of the token range is stored, so there is no need to care
about the end of the range. Remove the code that was trying to match the end of
the range (template, nested name).
- Add some spaces in test cases (destructors, templates) just to be sure that
everything works as expected.
- Add a missing CHECK-FIXES in the test case on the abstract_class destructor.
http://reviews.llvm.org/D13081
Files:
clang-query/tool/CMakeLists.txt
clang-tidy/misc/UnusedRAIICheck.h
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/MakeUniqueCheck.cpp
clang-tidy/modernize/MakeUniqueCheck.h
clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tidy/readability/IdentifierNamingCheck.h
clang-tidy/tool/run-clang-tidy.py
docs/clang-tidy/checks/google-build-explicit-make-pair.rst
docs/clang-tidy/checks/misc-unused-raii.rst
docs/modularize.rst
test/clang-tidy/Inputs/readability-identifier-naming/system/system-header.h
test/clang-tidy/Inputs/readability-identifier-naming/user-header.h
test/clang-tidy/modernize-make-unique.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -62,23 +62,44 @@
// RUN: {key: readability-identifier-naming.VirtualMethodCase, value: UPPER_CASE}, \
// RUN: {key: readability-identifier-naming.VirtualMethodPrefix, value: 'v_'}, \
// RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0} \
-// RUN: ]}' -- -std=c++11 -fno-delayed-template-parsing
+// RUN: ]}' -- -std=c++11 -fno-delayed-template-parsing \
+// RUN: -I%S/Inputs/readability-identifier-naming \
+// RUN: -isystem %S/Inputs/readability-identifier-naming/system
-// FIXME: There should be more test cases for checking that references to class
-// FIXME: name, declaration contexts, forward declarations, etc, are correctly
-// FIXME: checked and renamed
+// clang-format off
+
+#include
+#include "user-header.h"
+// NO warnings or fixes expected from declarations within header files without
+// the -header-filter= option
namespace FOO_NS {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for namespace 'FOO_NS' [readability-identifier-naming]
// CHECK-FIXES: {{^}}namespace foo_ns {{{$}}
inline namespace InlineNamespace {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for inline namespace 'InlineNamespace'
// CHECK-FIXES: {{^}}inline namespace inline_namespace {{{$}}
+SYSTEM_NS::structure g_s1;
+// NO warnings or fixes expected as SYSTEM_NS and structure are declared in a header file
+
+USER_NS::object g_s2;
+// NO warnings or fixes expected as USER_NS and object are declared in a header file
+
+SYSTEM_MACRO(var1);
+// NO warnings or fixes expected as var1 is from macro expansion
+
+USER_MACRO(var2);
+// NO warnings or fixes expected as var2 is declared in a macro expansion
+
+int global;
+#define USE_IN_MACRO(m) auto use_##m = m
+USE_IN_MACRO(global);
+// NO warnings or fixes expected as global is used in a macro expansion
+
#define BLA int FOO_bar
BLA;
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global variable 'FOO_bar'
-// NO fix expected as FOO_bar is from macro expansion
+// NO warnings or fixes expected as FOO_bar is from macro expansion
enum my_enumeration {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for enum 'my_enumeration'
@@ -102,6 +123,13 @@
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_class'
// CHECK-FIXES: {{^}}class CMyClass {{{$}}
my_class();
+// CHECK-FIXES: {{^}}CMyClass();{{$}}
+
+~
+ my_class();
+// (space in destructor token test, we could check trigraph but they will be deprecated)
+// CHECK-FIXES: {{^}}~{{$}}
+// CHECK-FIXES: {{^}} CMyClass();{{$}}
const int MEMBER_one_1 = ConstExpr_variable;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: invalid case style for constant member 'MEMBER_one_1'
@@ -135,15 +163,36 @@
const int my_class::classConstant = 4;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class constant 'classConstant'
-// CHECK-FIXES: {{^}}const int my_class::kClassConstant = 4;{{$}}
-// FIXME: The fixup should reflect class name fixups as well:
-// FIXME: {{^}}const int CMyClass::kClassConstant = 4;{{$}}
+// CHECK-FIXES: {{^}}const int CMyClass::kClassConstant = 4;{{$}}
int my_class::ClassMember_2 = 5;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class member 'ClassMember_2'
-// CHECK-FIXES: {{^}}int my_class::ClassMember2 = 5;{{$}}
-// FIXME: The fixup should reflect class name fixups as well:
-// FIXME: {{^}}int CMyCla
Re: [PATCH] D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
berenm updated this revision to Diff 35974.
berenm added a comment.
Fix arcanist messup...
http://reviews.llvm.org/D13081
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/Inputs/readability-identifier-naming/system/system-header.h
test/clang-tidy/Inputs/readability-identifier-naming/user-header.h
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -62,25 +62,44 @@
// RUN: {key: readability-identifier-naming.VirtualMethodCase, value: UPPER_CASE}, \
// RUN: {key: readability-identifier-naming.VirtualMethodPrefix, value: 'v_'}, \
// RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0} \
-// RUN: ]}' -- -std=c++11 -fno-delayed-template-parsing
-
-// FIXME: There should be more test cases for checking that references to class
-// FIXME: name, declaration contexts, forward declarations, etc, are correctly
-// FIXME: checked and renamed
+// RUN: ]}' -- -std=c++11 -fno-delayed-template-parsing \
+// RUN: -I%S/Inputs/readability-identifier-naming \
+// RUN: -isystem %S/Inputs/readability-identifier-naming/system
// clang-format off
+#include
+#include "user-header.h"
+// NO warnings or fixes expected from declarations within header files without
+// the -header-filter= option
+
namespace FOO_NS {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for namespace 'FOO_NS' [readability-identifier-naming]
// CHECK-FIXES: {{^}}namespace foo_ns {{{$}}
inline namespace InlineNamespace {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for inline namespace 'InlineNamespace'
// CHECK-FIXES: {{^}}inline namespace inline_namespace {{{$}}
+SYSTEM_NS::structure g_s1;
+// NO warnings or fixes expected as SYSTEM_NS and structure are declared in a header file
+
+USER_NS::object g_s2;
+// NO warnings or fixes expected as USER_NS and object are declared in a header file
+
+SYSTEM_MACRO(var1);
+// NO warnings or fixes expected as var1 is from macro expansion
+
+USER_MACRO(var2);
+// NO warnings or fixes expected as var2 is declared in a macro expansion
+
+int global;
+#define USE_IN_MACRO(m) auto use_##m = m
+USE_IN_MACRO(global);
+// NO warnings or fixes expected as global is used in a macro expansion
+
#define BLA int FOO_bar
BLA;
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global variable 'FOO_bar'
-// NO fix expected as FOO_bar is from macro expansion
+// NO warnings or fixes expected as FOO_bar is from macro expansion
enum my_enumeration {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for enum 'my_enumeration'
@@ -104,6 +123,13 @@
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_class'
// CHECK-FIXES: {{^}}class CMyClass {{{$}}
my_class();
+// CHECK-FIXES: {{^}}CMyClass();{{$}}
+
+~
+ my_class();
+// (space in destructor token test, we could check trigraph but they will be deprecated)
+// CHECK-FIXES: {{^}}~{{$}}
+// CHECK-FIXES: {{^}} CMyClass();{{$}}
const int MEMBER_one_1 = ConstExpr_variable;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: invalid case style for constant member 'MEMBER_one_1'
@@ -137,15 +163,36 @@
const int my_class::classConstant = 4;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class constant 'classConstant'
-// CHECK-FIXES: {{^}}const int my_class::kClassConstant = 4;{{$}}
-// FIXME: The fixup should reflect class name fixups as well:
-// FIXME: {{^}}const int CMyClass::kClassConstant = 4;{{$}}
+// CHECK-FIXES: {{^}}const int CMyClass::kClassConstant = 4;{{$}}
int my_class::ClassMember_2 = 5;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class member 'ClassMember_2'
-// CHECK-FIXES: {{^}}int my_class::ClassMember2 = 5;{{$}}
-// FIXME: The fixup should reflect class name fixups as well:
-// FIXME: {{^}}int CMyClass::ClassMember2 = 5;{{$}}
+// CHECK-FIXES: {{^}}int CMyClass::ClassMember2 = 5;{{$}}
+
+class my_derived_class : public virtual my_class {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_derived_class'
+// CHECK-FIXES: {{^}}class CMyDerivedClass : public virtual CMyClass {};{{$}}
+
+class CMyWellNamedClass {};
+// No warning expected as this class is well named.
+
+template
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for type template parameter 'T'
+// CHECK-FIXES: {{^}}template{{$}}
+class my_templated_class : CMyWellNamedClass {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_templated_class'
+// CHECK-FIXES: {{^}}class CMyTemplatedClass : CMyWellNamedClass {};{{$}}
+
+template
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for type template parameter 'T'
+// CHECK-FIXES: {{^}}template{{$}}
+class
Re: [PATCH] D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
berenm marked 3 inline comments as done. berenm added a comment. http://reviews.llvm.org/D13081 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12362: [clang-format] Add support of consecutive declarations alignment
berenm added inline comments.
Comment at: unittests/Format/FormatTest.cpp:8699
@@ +8698,3 @@
+ Alignment));
+ Alignment.AlignConsecutiveAssignments = true;
+ verifyFormat("float something = 2000;\n"
djasper wrote:
> Can you add a case (unless I missed it) where aligning both consecutive
> assignments and consecutive declarations exceed the column limit? What should
> happen in that case? I am thinking of something like:
>
> int lngName = 1;
> LngType i = bbb;
AFAIR, the alignment doesn't work very well with the column limit at the
moment. This is already true wrt the assignment alignment. The column limit is
enforced before the alignment is done and aligning variable names and / or
assignment will expand beyond that limit.
I will add the test case but I haven't tried to fix this issue yet.
Should test cases check the current behaviour or the ideal expected behaviour
(that doesn't work) ?
http://reviews.llvm.org/D12362
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
berenm added a comment. Alright, thank you for your time! Repository: rL LLVM http://reviews.llvm.org/D13081 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12362: [clang-format] Add support of consecutive declarations alignment
berenm updated this revision to Diff 36207.
berenm added a comment.
Fix arcanist insisting on creating the diff against old revision...
http://reviews.llvm.org/D12362
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8645,6 +8645,189 @@
Alignment);
}
+TEST_F(FormatTest, AlignConsecutiveDeclarations) {
+ FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveDeclarations = false;
+ verifyFormat("float const a = 5;\n"
+ "int oneTwoThree = 123;",
+ Alignment);
+ verifyFormat("int a = 5;\n"
+ "float const oneTwoThree = 123;",
+ Alignment);
+
+ Alignment.AlignConsecutiveDeclarations = true;
+ verifyFormat("float const a = 5;\n"
+ "int oneTwoThree = 123;",
+ Alignment);
+ verifyFormat("int a = method();\n"
+ "float const oneTwoThree = 133;",
+ Alignment);
+ verifyFormat("int i = 1, j = 10;\n"
+ "something = 2000;",
+ Alignment);
+ verifyFormat("something = 2000;\n"
+ "int i = 1, j = 10;\n",
+ Alignment);
+ verifyFormat("float something = 2000;\n"
+ "double another = 911;\n"
+ "inti = 1, j = 10;\n"
+ "const int *oneMore = 1;\n"
+ "unsigned i = 2;",
+ Alignment);
+ verifyFormat("float a = 5;\n"
+ "int one = 1;\n"
+ "method();\n"
+ "const double oneTwoThree = 123;\n"
+ "const unsigned int oneTwo = 12;",
+ Alignment);
+ verifyFormat("int oneTwoThree{0}; // comment\n"
+ "unsigned oneTwo; // comment",
+ Alignment);
+ EXPECT_EQ("float const a = 5;\n"
+"\n"
+"int oneTwoThree = 123;",
+format("float const a = 5;\n"
+ "\n"
+ "int oneTwoThree= 123;",
+ Alignment));
+ EXPECT_EQ("float a = 5;\n"
+"int one = 1;\n"
+"\n"
+"unsigned oneTwoThree = 123;",
+format("floata = 5;\n"
+ "int one = 1;\n"
+ "\n"
+ "unsigned oneTwoThree = 123;",
+ Alignment));
+ EXPECT_EQ("float a = 5;\n"
+"int one = 1;\n"
+"\n"
+"unsigned oneTwoThree = 123;\n"
+"int oneTwo = 12;",
+format("floata = 5;\n"
+ "int one = 1;\n"
+ "\n"
+ "unsigned oneTwoThree = 123;\n"
+ "int oneTwo = 12;",
+ Alignment));
+ Alignment.AlignConsecutiveAssignments = true;
+ verifyFormat("float something = 2000;\n"
+ "double another = 911;\n"
+ "inti = 1, j = 10;\n"
+ "const int *oneMore = 1;\n"
+ "unsigned i = 2;",
+ Alignment);
+ verifyFormat("int oneTwoThree = {0}; // comment\n"
+ "unsigned oneTwo = 0; // comment",
+ Alignment);
+ EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
+" int const i = 1;\n"
+" int * j = 2;\n"
+" int big = 1;\n"
+"\n"
+" unsigned oneTwoThree = 123;\n"
+" int oneTwo = 12;\n"
+" method();\n"
+" float k = 2;\n"
+" int ll = 1;\n"
+"}",
+format("void SomeFunction(int parameter= 0) {\n"
+ " int const i= 1;\n"
+ " int *j=2;\n"
+ " int big = 1;\n"
+ "\n"
+ "unsigned oneTwoThree =123;\n"
+ "int oneTwo = 12;\n"
+ " method();\n"
+ "float k= 2;\n"
+ "int ll=1;\n"
+ "}",
+ Alignment));
+ Alignment.AlignConsecutiveAssignments = false;
+ Alignment.AlignEscapedNewlinesLeft = true;
+ verifyFormat("#define A \\\n"
+ " int = 12; \\\n"
+ " float b = 23;\\\n"
+ " const int ccc = 234; \\\n"
+ " unsigned dd = 2345;",
+ Alignment);
+ Alignment.AlignEscapedNewlinesLeft = false;
+ Alignment.ColumnLimit = 30;
+ verifyFormat("#define A\\\n"
+ " int = 12; \\\n"
+ " float b = 23; \\\n"
+ " const int ccc = 234; \\\n"
+ " i
Re: [PATCH] D12362: [clang-format] Add support of consecutive declarations alignment
berenm marked 7 inline comments as done. Comment at: unittests/Format/FormatTest.cpp:8699 @@ +8698,3 @@ + "int one = 1;\n" + "\n" + "unsigned oneTwoThree = 123;", I think I actually managed to do something about that. I added a test case as well to check that AlignConsecutive* respect the column limit. http://reviews.llvm.org/D12362 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12362: [clang-format] Add support of consecutive declarations alignment
berenm marked 2 inline comments as done. berenm added a comment. http://reviews.llvm.org/D12362 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12362: [clang-format] Add support of consecutive declarations alignment
berenm added a comment. Thanks, I don't have commit access. http://reviews.llvm.org/D12362 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D13513: [clang-format] Stop alignment sequences on open braces and parens when aligning assignments.
berenm created this revision.
berenm added a reviewer: djasper.
berenm added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
This was done correctly when aligning the declarations, but not when aligning
assignments.
FIXME: The code between assignments and declarations alignment is roughly
duplicated and
would benefit from factorization.
Bug 25090: https://llvm.org/bugs/show_bug.cgi?id=25090
http://reviews.llvm.org/D13513
Files:
lib/Format/WhitespaceManager.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8649,6 +8649,19 @@
"someLongFunction();\n"
"int j = 2;",
Alignment);
+
+ verifyFormat("auto lambda = []() {\n"
+ " auto i = 0;\n"
+ " return 0;\n"
+ "};\n"
+ "int i = 0;\n"
+ "auto v = type{\n"
+ "i = 1, //\n"
+ "(i = 2), //\n"
+ "i = 3//\n"
+ "};",
+ Alignment);
+
// FIXME: Should align all three assignments
verifyFormat(
"int i = 1;\n"
@@ -8817,6 +8830,23 @@
"someLongFunction();\n"
"int j = 2;",
Alignment);
+
+ Alignment.AlignConsecutiveAssignments = true;
+ verifyFormat("auto lambda = []() {\n"
+ " auto ii = 0;\n"
+ " float j = 0;\n"
+ " return 0;\n"
+ "};\n"
+ "int i = 0;\n"
+ "float i2 = 0;\n"
+ "auto v = type{\n"
+ "i = 1, //\n"
+ "(i = 2), //\n"
+ "i = 3//\n"
+ "};",
+ Alignment);
+ Alignment.AlignConsecutiveAssignments = false;
+
// FIXME: Should align all three declarations
verifyFormat(
"int i = 1;\n"
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -153,6 +153,9 @@
// a "=" is found on a line, extend the current sequence. If the current line
// cannot be part of a sequence, e.g. because there is an empty line before it
// or it contains non-assignments, finalize the previous sequence.
+//
+// FIXME: The code between assignment and declaration alignment is mostly
+// duplicated and would benefit from factorization.
void WhitespaceManager::alignConsecutiveAssignments() {
if (!Style.AlignConsecutiveAssignments)
return;
@@ -162,6 +165,7 @@
unsigned StartOfSequence = 0;
unsigned EndOfSequence = 0;
bool FoundAssignmentOnLine = false;
+ bool FoundLeftBraceOnLine = false;
bool FoundLeftParenOnLine = false;
// Aligns a sequence of assignment tokens, on the MinColumn column.
@@ -181,18 +185,21 @@
};
for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
-if (Changes[i].NewlinesBefore > 0) {
+if (Changes[i].NewlinesBefore != 0) {
EndOfSequence = i;
- // If there is a blank line or if the last line didn't contain any
- // assignment, the sequence ends here.
- if (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine) {
+ // If there is a blank line, if the last line didn't contain any
+ // assignment, or if we found an open brace or paren, the sequence ends
+ // here.
+ if (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine ||
+ FoundLeftBraceOnLine || FoundLeftParenOnLine) {
// NB: In the latter case, the sequence should end at the beggining of
// the previous line, but it doesn't really matter as there is no
// assignment on it
AlignSequence();
}
FoundAssignmentOnLine = false;
+ FoundLeftBraceOnLine = false;
FoundLeftParenOnLine = false;
}
@@ -202,14 +209,24 @@
(FoundAssignmentOnLine || Changes[i].NewlinesBefore > 0 ||
Changes[i + 1].NewlinesBefore > 0)) {
AlignSequence();
-} else if (!FoundLeftParenOnLine && Changes[i].Kind == tok::r_paren) {
- AlignSequence();
+} else if (Changes[i].Kind == tok::r_brace) {
+ if (!FoundLeftBraceOnLine)
+AlignSequence();
+ FoundLeftBraceOnLine = false;
+} else if (Changes[i].Kind == tok::l_brace) {
+ FoundLeftBraceOnLine = true;
+ if (!FoundAssignmentOnLine)
+AlignSequence();
+} else if (Changes[i].Kind == tok::r_paren) {
+ if (!FoundLeftParenOnLine)
+AlignSequence();
+ FoundLeftParenOnLine = false;
} else if (Changes[i].Kind == tok::l_paren) {
FoundLeftParenOnLine = true;
if (!FoundAssignmentOnLine)
AlignSequence();
-} else if (!FoundAssignmentOnLine && !FoundLeftParenOnLine &&
- Changes[i].Kind == tok::eq
Re: [PATCH] D13513: [clang-format] Stop alignment sequences on open braces and parens when aligning assignments.
berenm added inline comments.
Comment at: unittests/Format/FormatTest.cpp:8657
@@ +8656,3 @@
+ "};\n"
+ "int i = 0;\n"
+ "auto v = type{\n"
djasper wrote:
> So, it is kind of interesting that we (would) align with a lambda on the next
> line here, but not with the lambda declaration a few lines above.
>
> I guess this makes sense, but I would also vager that somebody is going to
> file a bug report about it ;-).
Yes, this would require to handle nested levels of alignment sequences and push
the current one when a new scope is detected. It's just not implemented yet :)
http://reviews.llvm.org/D13513
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10933: Add new IdentifierNaming check
berenm marked 2 inline comments as done. Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:166 @@ +165,3 @@ + static llvm::Regex Splitter( + "(([a-z0-9A-Z]*)(_+)|([A-Z]?[a-z0-9]+)([A-Z]|$)|([A-Z]+)([A-Z]|$))"); + alexfh wrote: > Why do you need the outermost parentheses? For some unknown reason, I always thought parentheses were required around regex alternation... Well, I now know they are not. http://reviews.llvm.org/D10933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10933: Add new IdentifierNaming check
berenm updated this revision to Diff 32277.
berenm added a comment.
Here is an updated version with the latest comments fixed.
http://reviews.llvm.org/D10933
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tidy/readability/IdentifierNamingCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -0,0 +1,260 @@
+// RUN: $(dirname %s)/check_clang_tidy.sh %s readability-identifier-naming %t \
+// RUN: -config='{CheckOptions: [ \
+// RUN: {key: readability-identifier-naming.AbstractClassCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.AbstractClassPrefix, value: 'A'}, \
+// RUN: {key: readability-identifier-naming.ClassCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ClassPrefix, value: 'C'}, \
+// RUN: {key: readability-identifier-naming.ClassConstantCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ClassConstantPrefix, value: 'k'}, \
+// RUN: {key: readability-identifier-naming.ClassMemberCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ClassMethodCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.ConstantCase, value: UPPER_CASE}, \
+// RUN: {key: readability-identifier-naming.ConstantSuffix, value: '_CST'}, \
+// RUN: {key: readability-identifier-naming.ConstexprFunctionCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.ConstexprMethodCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.ConstexprVariableCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.EnumCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.EnumPrefix, value: 'E'}, \
+// RUN: {key: readability-identifier-naming.EnumConstantCase, value: UPPER_CASE}, \
+// RUN: {key: readability-identifier-naming.FunctionCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.GlobalConstantCase, value: UPPER_CASE}, \
+// RUN: {key: readability-identifier-naming.GlobalFunctionCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.GlobalVariableCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.GlobalVariablePrefix, value: 'g_'}, \
+// RUN: {key: readability-identifier-naming.InlineNamespaceCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.LocalConstantCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.LocalConstantPrefix, value: 'k'}, \
+// RUN: {key: readability-identifier-naming.LocalVariableCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.MemberCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.MemberPrefix, value: 'm_'}, \
+// RUN: {key: readability-identifier-naming.ConstantMemberCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.PrivateMemberPrefix, value: '__'}, \
+// RUN: {key: readability-identifier-naming.ProtectedMemberPrefix, value: '_'}, \
+// RUN: {key: readability-identifier-naming.PublicMemberCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.MethodCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.PrivateMethodPrefix, value: '__'}, \
+// RUN: {key: readability-identifier-naming.ProtectedMethodPrefix, value: '_'}, \
+// RUN: {key: readability-identifier-naming.NamespaceCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.ParameterCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.ParameterPrefix, value: 'a_'}, \
+// RUN: {key: readability-identifier-naming.ConstantParameterCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.ConstantParameterPrefix, value: 'i_'}, \
+// RUN: {key: readability-identifier-naming.ParameterPackCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.PureFunctionCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.PureMethodCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.StaticConstantCase, value: UPPER_CASE}, \
+// RUN: {key: readability-identifier-naming.StaticVariableCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.StaticVariablePrefix, value: 's_'}, \
+// RUN: {key: readability-identifier-naming.StructCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.TemplateParameterCase, value: UPPER_CASE}, \
+// RUN: {key: readability-identifier-naming.TemplateTemplateParameterCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.TemplateUsingCase,
Re: [PATCH] D10933: Add new IdentifierNaming check
berenm marked 6 inline comments as done. berenm added a comment. http://reviews.llvm.org/D10933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10933: Add new IdentifierNaming check
berenm added a comment. In http://reviews.llvm.org/D10933#225671, @alexfh wrote: > Looks good with a couple of comments. Tell me if you need me to submit the > patch for you, once you address the comments. If you tell me how I should proceed, I can maybe do it myself (do I just have to send the patch to the cfe-commits list ?). http://reviews.llvm.org/D10933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10933: Add new IdentifierNaming check
berenm updated this revision to Diff 32294.
berenm added a comment.
Indeed, I probably don't have rights on Clang/LLVM repositories.
I have updated the patch with stricter tests - and found a missing break
statement (`clang-tidy/readability/IdentifierNamingCheck.cpp:201`).
I also realized that the DeclRefExpr matcher used to find references to
declared identifiers is only matching references to values and not to classes.
For example, when referencing the class in the out-of-line class static member
declaration (`test/clang-tidy/readability-identifier-naming.cpp:133` and
`test/clang-tidy/readability-identifier-naming.cpp:136`), or when using a
class identifier as the type of a typedef,
(`test/clang-tidy/readability-identifier-naming.cpp:243`). Or even when
declaring a variable of a type whose name must be fixed.
Is there any matcher that could be used for finding references to a TypeDecl?
Should I go through calls to `getDeclContext`, `getType`, or
`getUnderlyingType` depending on the context? Any easier way I don't know about?
http://reviews.llvm.org/D10933
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tidy/readability/IdentifierNamingCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -0,0 +1,253 @@
+// RUN: $(dirname %s)/check_clang_tidy.sh %s readability-identifier-naming %t \
+// RUN: -config='{CheckOptions: [ \
+// RUN: {key: readability-identifier-naming.AbstractClassCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.AbstractClassPrefix, value: 'A'}, \
+// RUN: {key: readability-identifier-naming.ClassCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ClassPrefix, value: 'C'}, \
+// RUN: {key: readability-identifier-naming.ClassConstantCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ClassConstantPrefix, value: 'k'}, \
+// RUN: {key: readability-identifier-naming.ClassMemberCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ClassMethodCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.ConstantCase, value: UPPER_CASE}, \
+// RUN: {key: readability-identifier-naming.ConstantSuffix, value: '_CST'}, \
+// RUN: {key: readability-identifier-naming.ConstexprFunctionCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.ConstexprMethodCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.ConstexprVariableCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.EnumCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.EnumPrefix, value: 'E'}, \
+// RUN: {key: readability-identifier-naming.EnumConstantCase, value: UPPER_CASE}, \
+// RUN: {key: readability-identifier-naming.FunctionCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.GlobalConstantCase, value: UPPER_CASE}, \
+// RUN: {key: readability-identifier-naming.GlobalFunctionCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.GlobalVariableCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.GlobalVariablePrefix, value: 'g_'}, \
+// RUN: {key: readability-identifier-naming.InlineNamespaceCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.LocalConstantCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.LocalConstantPrefix, value: 'k'}, \
+// RUN: {key: readability-identifier-naming.LocalVariableCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.MemberCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.MemberPrefix, value: 'm_'}, \
+// RUN: {key: readability-identifier-naming.ConstantMemberCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.PrivateMemberPrefix, value: '__'}, \
+// RUN: {key: readability-identifier-naming.ProtectedMemberPrefix, value: '_'}, \
+// RUN: {key: readability-identifier-naming.PublicMemberCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.MethodCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.PrivateMethodPrefix, value: '__'}, \
+// RUN: {key: readability-identifier-naming.ProtectedMethodPrefix, value: '_'}, \
+// RUN: {key: readability-identifier-naming.NamespaceCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.ParameterCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.ParameterPrefix, value: 'a_'}, \
+// RUN: {key: readability-identifier-naming.ConstantParameterCase, value: camelBack}, \
+// RUN: {key: readability-identif
Re: [PATCH] D10933: Add new IdentifierNaming check
berenm updated this revision to Diff 32304.
berenm added a comment.
Alright!
Here is the latest revision then, with FIXMEs comments mentioning this missing
feature. I will work on implementing it and come back to you when i've got
something working.
Thanks for your time and your advices!
http://reviews.llvm.org/D10933
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tidy/readability/IdentifierNamingCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -0,0 +1,266 @@
+// RUN: $(dirname %s)/check_clang_tidy.sh %s readability-identifier-naming %t \
+// RUN: -config='{CheckOptions: [ \
+// RUN: {key: readability-identifier-naming.AbstractClassCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.AbstractClassPrefix, value: 'A'}, \
+// RUN: {key: readability-identifier-naming.ClassCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ClassPrefix, value: 'C'}, \
+// RUN: {key: readability-identifier-naming.ClassConstantCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ClassConstantPrefix, value: 'k'}, \
+// RUN: {key: readability-identifier-naming.ClassMemberCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ClassMethodCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.ConstantCase, value: UPPER_CASE}, \
+// RUN: {key: readability-identifier-naming.ConstantSuffix, value: '_CST'}, \
+// RUN: {key: readability-identifier-naming.ConstexprFunctionCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.ConstexprMethodCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.ConstexprVariableCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.EnumCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.EnumPrefix, value: 'E'}, \
+// RUN: {key: readability-identifier-naming.EnumConstantCase, value: UPPER_CASE}, \
+// RUN: {key: readability-identifier-naming.FunctionCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.GlobalConstantCase, value: UPPER_CASE}, \
+// RUN: {key: readability-identifier-naming.GlobalFunctionCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.GlobalVariableCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.GlobalVariablePrefix, value: 'g_'}, \
+// RUN: {key: readability-identifier-naming.InlineNamespaceCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.LocalConstantCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.LocalConstantPrefix, value: 'k'}, \
+// RUN: {key: readability-identifier-naming.LocalVariableCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.MemberCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.MemberPrefix, value: 'm_'}, \
+// RUN: {key: readability-identifier-naming.ConstantMemberCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.PrivateMemberPrefix, value: '__'}, \
+// RUN: {key: readability-identifier-naming.ProtectedMemberPrefix, value: '_'}, \
+// RUN: {key: readability-identifier-naming.PublicMemberCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.MethodCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.PrivateMethodPrefix, value: '__'}, \
+// RUN: {key: readability-identifier-naming.ProtectedMethodPrefix, value: '_'}, \
+// RUN: {key: readability-identifier-naming.NamespaceCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.ParameterCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.ParameterPrefix, value: 'a_'}, \
+// RUN: {key: readability-identifier-naming.ConstantParameterCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.ConstantParameterPrefix, value: 'i_'}, \
+// RUN: {key: readability-identifier-naming.ParameterPackCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.PureFunctionCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.PureMethodCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.StaticConstantCase, value: UPPER_CASE}, \
+// RUN: {key: readability-identifier-naming.StaticVariableCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.StaticVariablePrefix, value: 's_'}, \
+// RUN: {key: readability-identifier-naming.StructCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.TemplateParameterCase, value: UPPER_CASE}, \
+//
[PATCH] D12362: [clang-format] Add support of consecutive declarations alignment
berenm created this revision.
berenm added a reviewer: djasper.
berenm added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
This allows clang-format to align identifiers in consecutive declarations.
This is, arguably, a feature useful for increasing the readability of the code,
in the same way the alignment of assignations is. It is also present in other
tools such as uncrustify for example.
The code is a slightly modified version of the consecutive assignment
alignment code. Currently only the identifiers are aligned, and there is no
support of alignment of the pointer star or reference symbol.
http://reviews.llvm.org/D12362
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8616,6 +8616,152 @@
Alignment);
}
+TEST_F(FormatTest, AlignConsecutiveDeclarations) {
+ FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveDeclarations = false;
+ verifyFormat("float const a = 5;\n"
+ "int oneTwoThree = 123;",
+ Alignment);
+ verifyFormat("int a = 5;\n"
+ "float const oneTwoThree = 123;",
+ Alignment);
+
+ Alignment.AlignConsecutiveDeclarations = true;
+ verifyFormat("float const a = 5;\n"
+ "int oneTwoThree = 123;",
+ Alignment);
+ verifyFormat("int a = method();\n"
+ "float const oneTwoThree = 133;",
+ Alignment);
+ verifyFormat("a &= 5;\n"
+ "bcd *= 5;\n"
+ "ghtyf += 5;\n"
+ "dvfvdb -= 5;\n"
+ "a /= 5;\n"
+ "vdsvsv %= 5;\n"
+ "sfdbddfbdfbb ^= 5;\n"
+ "dvsdsv |= 5;\n"
+ "int dsvvdvsdvvv = 123;",
+ Alignment);
+ verifyFormat("int i = 1, j = 10;\n"
+ "something = 2000;",
+ Alignment);
+ verifyFormat("something = 2000;\n"
+ "int i = 1, j = 10;\n",
+ Alignment);
+ verifyFormat("float something = 2000;\n"
+ "double another = 911;\n"
+ "inti = 1, j = 10;\n"
+ "const int *oneMore = 1;\n"
+ "unsigned i = 2;",
+ Alignment);
+ verifyFormat("float a = 5;\n"
+ "int one = 1;\n"
+ "method();\n"
+ "const double oneTwoThree = 123;\n"
+ "const unsigned int oneTwo = 12;",
+ Alignment);
+ verifyFormat("int oneTwoThree{0}; // comment\n"
+ "unsigned oneTwo; // comment",
+ Alignment);
+ EXPECT_EQ("float const a = 5;\n"
+"\n"
+"int oneTwoThree = 123;",
+format("float const a = 5;\n"
+ "\n"
+ "int oneTwoThree= 123;",
+ Alignment));
+ EXPECT_EQ("float a = 5;\n"
+"int one = 1;\n"
+"\n"
+"unsigned oneTwoThree = 123;",
+format("floata = 5;\n"
+ "int one = 1;\n"
+ "\n"
+ "unsigned oneTwoThree = 123;",
+ Alignment));
+ EXPECT_EQ("float a = 5;\n"
+"int one = 1;\n"
+"\n"
+"unsigned oneTwoThree = 123;\n"
+"int oneTwo = 12;",
+format("floata = 5;\n"
+ "int one = 1;\n"
+ "\n"
+ "unsigned oneTwoThree = 123;\n"
+ "int oneTwo = 12;",
+ Alignment));
+ Alignment.AlignEscapedNewlinesLeft = true;
+ verifyFormat("#define A \\\n"
+ " int = 12; \\\n"
+ " float b = 23;\\\n"
+ " const int ccc = 234; \\\n"
+ " unsigned dd = 2345;",
+ Alignment);
+ Alignment.AlignEscapedNewlinesLeft = false;
+ verifyFormat("#define A "
+ "\\\n"
+ " int = 12; "
+ "\\\n"
+ " float b = 23;"
+ "\\\n"
+ " const int ccc = 234; "
+ "\\\n"
+ " int dd = 2345;",
+ Alignment);
+ verifyFormat("void SomeFunction(int parameter = 1, int i = 2, int j = 3, int "
+ "k = 4, int l = 5,\n"
+ " int m = 6) {\n"
+ " const int j = 10;\n"
+ " otherThing = 1;\n"
+
Re: [PATCH] D12362: [clang-format] Add support of consecutive declarations alignment
berenm added a comment.
Actually, I think there is a bug in the assignment alignment code. Even without
this patch, this code doesn't align properly:
int oneTwoThree = 123;
int oneTwo = 12;
method();
I don't think I completely understand the assignment alignment code, and that's
why mine is slightly rewritten. I will open another request for fixes in the
assignment alignment code.
Comment at: unittests/Format/FormatTest.cpp:8619
@@ -8618,1 +8618,3 @@
+TEST_F(FormatTest, AlignConsecutiveDeclarations) {
+ FormatStyle Alignment = getLLVMStyle();
djasper wrote:
> This needs tests that check what happens if both declarations and assignments
> are aligned.
Indeed, I will add such tests.
http://reviews.llvm.org/D12362
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D12369: [clang-format] Fix alignConsecutiveAssignments not working properly
berenm created this revision.
berenm added a reviewer: djasper.
berenm added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
This simple test case (added to the unit tests) was failing to align,
apparently due to the ``method();`` on the last line:
```
int oneTwoThree = 123;
int oneTwo = 12;
method();
```
http://reviews.llvm.org/D12369
Files:
lib/Format/WhitespaceManager.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8530,6 +8530,10 @@
"int oneTwoThree = 123;\n"
"int oneTwo = 12;\n",
Alignment);
+ verifyFormat("int oneTwoThree = 123;\n"
+ "int oneTwo = 12;\n"
+ "method();\n",
+ Alignment);
verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo = 12; // comment",
Alignment);
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -162,41 +162,53 @@
unsigned EndOfSequence = 0;
bool FoundAssignmentOnLine = false;
bool FoundLeftParenOnLine = false;
- unsigned CurrentLine = 0;
+ // Aligns a sequence of assignment tokens, on the MinColumn column.
+ //
+ // Sequences start from the first assignment token to align, and end at the
+ // first token of the first line that doesn't need to be aligned.
+ //
+ // Wee need to adjust the StartOfTokenColumn of each Change that is on a line
+ // containing any assignment to be aligned and located after such assignment
auto AlignSequence = [&] {
-alignConsecutiveAssignments(StartOfSequence, EndOfSequence, MinColumn);
+if (StartOfSequence > 0 && StartOfSequence < EndOfSequence)
+ alignConsecutiveAssignments(StartOfSequence, EndOfSequence, MinColumn);
MinColumn = 0;
StartOfSequence = 0;
EndOfSequence = 0;
};
for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
-if (Changes[i].NewlinesBefore != 0) {
- CurrentLine += Changes[i].NewlinesBefore;
- if (StartOfSequence > 0 &&
- (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine)) {
-EndOfSequence = i;
+if (Changes[i].NewlinesBefore > 0) {
+ EndOfSequence = i;
+ // If there is a blank line or if the last line didn't contain any
+ // assignment, the sequence ends here.
+ if (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine) {
+// NB: In the latter case, the sequence should end at the beggining of
+// the previous line, but it doesn't really matter as there is no
+// assignment on it
AlignSequence();
}
+
FoundAssignmentOnLine = false;
FoundLeftParenOnLine = false;
}
-if ((Changes[i].Kind == tok::equal &&
- (FoundAssignmentOnLine || ((Changes[i].NewlinesBefore > 0 ||
- Changes[i + 1].NewlinesBefore > 0 ||
-(!FoundLeftParenOnLine && Changes[i].Kind == tok::r_paren)) {
- if (StartOfSequence > 0)
-AlignSequence();
+// If there is more than one "=" per line, or if the "=" appears first on
+// the line of if it appears last, end the sequence
+if (Changes[i].Kind == tok::equal &&
+(FoundAssignmentOnLine || Changes[i].NewlinesBefore > 0 ||
+ Changes[i + 1].NewlinesBefore > 0)) {
+ AlignSequence();
+} else if (!FoundLeftParenOnLine && Changes[i].Kind == tok::r_paren) {
+ AlignSequence();
} else if (Changes[i].Kind == tok::l_paren) {
FoundLeftParenOnLine = true;
- if (!FoundAssignmentOnLine && StartOfSequence > 0)
+ if (!FoundAssignmentOnLine)
AlignSequence();
} else if (!FoundAssignmentOnLine && !FoundLeftParenOnLine &&
Changes[i].Kind == tok::equal) {
FoundAssignmentOnLine = true;
- EndOfSequence = i;
if (StartOfSequence == 0)
StartOfSequence = i;
@@ -205,36 +217,34 @@
}
}
- if (StartOfSequence > 0) {
-EndOfSequence = Changes.size();
-AlignSequence();
- }
+ EndOfSequence = Changes.size();
+ AlignSequence();
}
void WhitespaceManager::alignConsecutiveAssignments(unsigned Start,
unsigned End,
unsigned Column) {
- bool AlignedAssignment = false;
- int PreviousShift = 0;
+ bool FoundAssignmentOnLine = false;
+ int Shift = 0;
for (unsigned i = Start; i != End; ++i) {
-int Shift = 0;
-if (Changes[i].NewlinesBefore > 0)
- AlignedAssignment = false;
-if (!AlignedAssignment && Changes[i].Kind == tok::equal) {
+if (Changes[i].NewlinesBefore > 0) {
+ FoundAssignmentOnLine = false;
+ Shift = 0;
+}
+
+// If this is the first assignme
Re: [PATCH] D12369: [clang-format] Fix alignConsecutiveAssignments not working properly
berenm added a comment.
I'm not entirely sure of what was going on, so I rewrote the code a bit, and
hopefully, more clearly.
Comment at: lib/Format/WhitespaceManager.cpp:218
@@ -217,3 +229,1 @@
- bool AlignedAssignment = false;
- int PreviousShift = 0;
for (unsigned i = Start; i != End; ++i) {
What was the purpose of `PreviousShift`?
Comment at: lib/Format/WhitespaceManager.cpp:229
@@ -228,3 +244,3 @@
assert(Shift >= 0);
-Changes[i].Spaces += Shift;
if (i + 1 != Changes.size())
Why shifting by `Shift` //and then// also by `PreviousShift`?
http://reviews.llvm.org/D12369
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12362: [clang-format] Add support of consecutive declarations alignment
berenm updated this revision to Diff 33212.
berenm added a comment.
Added some test cases in combined usage with AlignConsecutiveAssignments.
The tests should pass with http://reviews.llvm.org/D12369 applied.
http://reviews.llvm.org/D12362
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8631,6 +8631,174 @@
Alignment);
}
+TEST_F(FormatTest, AlignConsecutiveDeclarations) {
+ FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveDeclarations = false;
+ verifyFormat("float const a = 5;\n"
+ "int oneTwoThree = 123;",
+ Alignment);
+ verifyFormat("int a = 5;\n"
+ "float const oneTwoThree = 123;",
+ Alignment);
+
+ Alignment.AlignConsecutiveDeclarations = true;
+ verifyFormat("float const a = 5;\n"
+ "int oneTwoThree = 123;",
+ Alignment);
+ verifyFormat("int a = method();\n"
+ "float const oneTwoThree = 133;",
+ Alignment);
+ verifyFormat("int i = 1, j = 10;\n"
+ "something = 2000;",
+ Alignment);
+ verifyFormat("something = 2000;\n"
+ "int i = 1, j = 10;\n",
+ Alignment);
+ verifyFormat("float something = 2000;\n"
+ "double another = 911;\n"
+ "inti = 1, j = 10;\n"
+ "const int *oneMore = 1;\n"
+ "unsigned i = 2;",
+ Alignment);
+ verifyFormat("float a = 5;\n"
+ "int one = 1;\n"
+ "method();\n"
+ "const double oneTwoThree = 123;\n"
+ "const unsigned int oneTwo = 12;",
+ Alignment);
+ verifyFormat("int oneTwoThree{0}; // comment\n"
+ "unsigned oneTwo; // comment",
+ Alignment);
+ EXPECT_EQ("float const a = 5;\n"
+"\n"
+"int oneTwoThree = 123;",
+format("float const a = 5;\n"
+ "\n"
+ "int oneTwoThree= 123;",
+ Alignment));
+ EXPECT_EQ("float a = 5;\n"
+"int one = 1;\n"
+"\n"
+"unsigned oneTwoThree = 123;",
+format("floata = 5;\n"
+ "int one = 1;\n"
+ "\n"
+ "unsigned oneTwoThree = 123;",
+ Alignment));
+ EXPECT_EQ("float a = 5;\n"
+"int one = 1;\n"
+"\n"
+"unsigned oneTwoThree = 123;\n"
+"int oneTwo = 12;",
+format("floata = 5;\n"
+ "int one = 1;\n"
+ "\n"
+ "unsigned oneTwoThree = 123;\n"
+ "int oneTwo = 12;",
+ Alignment));
+ Alignment.AlignConsecutiveAssignments = true;
+ verifyFormat("float something = 2000;\n"
+ "double another = 911;\n"
+ "inti = 1, j = 10;\n"
+ "const int *oneMore = 1;\n"
+ "unsigned i = 2;",
+ Alignment);
+ verifyFormat("int oneTwoThree = {0}; // comment\n"
+ "unsigned oneTwo = 0; // comment",
+ Alignment);
+ EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
+" int const i = 1;\n"
+" int * j = 2;\n"
+" int big = 1;\n"
+"\n"
+" unsigned oneTwoThree = 123;\n"
+" int oneTwo = 12;\n"
+" method();\n"
+" float k = 2;\n"
+" int ll = 1;\n"
+"}",
+format("void SomeFunction(int parameter= 0) {\n"
+ " int const i= 1;\n"
+ " int *j=2;\n"
+ " int big = 1;\n"
+ "\n"
+ "unsigned oneTwoThree =123;\n"
+ "int oneTwo = 12;\n"
+ " method();\n"
+ "float k= 2;\n"
+ "int ll=1;\n"
+ "}",
+ Alignment));
+ Alignment.AlignConsecutiveAssignments = false;
+ Alignment.AlignEscapedNewlinesLeft = true;
+ verifyFormat("#define A \\\n"
+ " int = 12; \\\n"
+ " float b = 23;\\\n"
+ " const int ccc = 234; \\\n"
+ " unsigned dd = 2345;",
+ Alignment);
+ Alignment.AlignEscapedNewlinesLeft = false;
+ Alignment.ColumnLimit = 30;
+ verifyFormat("#define A\\\n"
+ " int = 12; \\\n"
+ " float b = 23; \\\n
[PATCH] D12405: [clang-format-vs] Format the whole document if nothing is selected
berenm created this revision.
berenm added a reviewer: djasper.
berenm added a subscriber: cfe-commits.
By default, clang-format VS plugin only reformats the selected code.
To reformat the whole document, the user has to select everything before
calling the reformat shortcut.
http://reviews.llvm.org/D12405
Files:
tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
Index: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
===
--- tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
+++ tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
@@ -84,8 +84,13 @@
// We're not in a text view.
return;
string text = view.TextBuffer.CurrentSnapshot.GetText();
-int start =
view.Selection.Start.Position.GetContainingLine().Start.Position;
-int end =
view.Selection.End.Position.GetContainingLine().End.Position;
+int start = 0;
+int end = text.Length;
+if (!view.Selection.IsEmpty)
+{
+start =
view.Selection.Start.Position.GetContainingLine().Start.Position;
+end =
view.Selection.End.Position.GetContainingLine().End.Position;
+}
int length = end - start;
// clang-format doesn't support formatting a range that starts at
the end
// of the file.
Index: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
===
--- tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
+++ tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
@@ -84,8 +84,13 @@
// We're not in a text view.
return;
string text = view.TextBuffer.CurrentSnapshot.GetText();
-int start = view.Selection.Start.Position.GetContainingLine().Start.Position;
-int end = view.Selection.End.Position.GetContainingLine().End.Position;
+int start = 0;
+int end = text.Length;
+if (!view.Selection.IsEmpty)
+{
+start = view.Selection.Start.Position.GetContainingLine().Start.Position;
+end = view.Selection.End.Position.GetContainingLine().End.Position;
+}
int length = end - start;
// clang-format doesn't support formatting a range that starts at the end
// of the file.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12405: [clang-format-vs] Format the whole document if nothing is selected
Alright, my bad. It does indeed.
I was trying to add a "Reformat all on save" feature in the plugin, and
after struggling with VSSDK I thought this would be an easy first step.
--
Beren Minor
On Thu, Aug 27, 2015 at 3:36 PM, Aaron Ballman
wrote:
> On Thu, Aug 27, 2015 at 9:34 AM, Daniel Jasper via cfe-commits
> wrote:
> > If nothing is selected, clang-format should format the current line.. At
> > least that's the intended behavior. Doesn't it do that?
>
> It currently reformats the current line (possibly extended if the
> expression spans multiple lines) for me.
>
> ~Aaron
>
> >
> > On Aug 27, 2015 3:21 PM, "Beren Minor"
> wrote:
> >>
> >> berenm created this revision.
> >> berenm added a reviewer: djasper.
> >> berenm added a subscriber: cfe-commits.
> >>
> >> By default, clang-format VS plugin only reformats the selected code.
> >>
> >> To reformat the whole document, the user has to select everything before
> >> calling the reformat shortcut.
> >>
> >>
> >> http://reviews.llvm.org/D12405
> >>
> >> Files:
> >> tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
> >>
> >> Index: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
> >> ===
> >> --- tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
> >> +++ tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
> >> @@ -84,8 +84,13 @@
> >> // We're not in a text view.
> >> return;
> >> string text = view.TextBuffer.CurrentSnapshot.GetText();
> >> -int start =
> >> view.Selection.Start.Position.GetContainingLine().Start.Position;
> >> -int end =
> >> view.Selection.End.Position.GetContainingLine().End.Position;
> >> +int start = 0;
> >> +int end = text.Length;
> >> +if (!view.Selection.IsEmpty)
> >> +{
> >> +start =
> >> view.Selection.Start.Position.GetContainingLine().Start.Position;
> >> +end =
> >> view.Selection.End.Position.GetContainingLine().End.Position;
> >> +}
> >> int length = end - start;
> >> // clang-format doesn't support formatting a range that
> >> starts at the end
> >> // of the file.
> >>
> >>
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk
berenm added a comment. Sure, no worries. http://reviews.llvm.org/D12407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12369: [clang-format] Fix alignConsecutiveAssignments not working properly
berenm updated this revision to Diff 33783.
berenm added a comment.
Here it is, with the typos in comments corrected, and rebased over the latest
trunk.
Thanks for the review!
http://reviews.llvm.org/D12369
Files:
lib/Format/WhitespaceManager.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8537,6 +8537,10 @@
"int oneTwoThree = 123;\n"
"int oneTwo = 12;",
Alignment);
+ verifyFormat("int oneTwoThree = 123;\n"
+ "int oneTwo = 12;\n"
+ "method();\n",
+ Alignment);
verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo = 12; // comment",
Alignment);
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -156,41 +156,53 @@
unsigned EndOfSequence = 0;
bool FoundAssignmentOnLine = false;
bool FoundLeftParenOnLine = false;
- unsigned CurrentLine = 0;
+ // Aligns a sequence of assignment tokens, on the MinColumn column.
+ //
+ // Sequences start from the first assignment token to align, and end at the
+ // first token of the first line that doesn't need to be aligned.
+ //
+ // We need to adjust the StartOfTokenColumn of each Change that is on a line
+ // containing any assignment to be aligned and located after such assignment
auto AlignSequence = [&] {
-alignConsecutiveAssignments(StartOfSequence, EndOfSequence, MinColumn);
+if (StartOfSequence > 0 && StartOfSequence < EndOfSequence)
+ alignConsecutiveAssignments(StartOfSequence, EndOfSequence, MinColumn);
MinColumn = 0;
StartOfSequence = 0;
EndOfSequence = 0;
};
for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
-if (Changes[i].NewlinesBefore != 0) {
- CurrentLine += Changes[i].NewlinesBefore;
- if (StartOfSequence > 0 &&
- (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine)) {
-EndOfSequence = i;
+if (Changes[i].NewlinesBefore > 0) {
+ EndOfSequence = i;
+ // If there is a blank line or if the last line didn't contain any
+ // assignment, the sequence ends here.
+ if (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine) {
+// NB: In the latter case, the sequence should end at the beggining of
+// the previous line, but it doesn't really matter as there is no
+// assignment on it
AlignSequence();
}
+
FoundAssignmentOnLine = false;
FoundLeftParenOnLine = false;
}
-if ((Changes[i].Kind == tok::equal &&
- (FoundAssignmentOnLine || ((Changes[i].NewlinesBefore > 0 ||
- Changes[i + 1].NewlinesBefore > 0 ||
-(!FoundLeftParenOnLine && Changes[i].Kind == tok::r_paren)) {
- if (StartOfSequence > 0)
-AlignSequence();
+// If there is more than one "=" per line, or if the "=" appears first on
+// the line of if it appears last, end the sequence
+if (Changes[i].Kind == tok::equal &&
+(FoundAssignmentOnLine || Changes[i].NewlinesBefore > 0 ||
+ Changes[i + 1].NewlinesBefore > 0)) {
+ AlignSequence();
+} else if (!FoundLeftParenOnLine && Changes[i].Kind == tok::r_paren) {
+ AlignSequence();
} else if (Changes[i].Kind == tok::l_paren) {
FoundLeftParenOnLine = true;
- if (!FoundAssignmentOnLine && StartOfSequence > 0)
+ if (!FoundAssignmentOnLine)
AlignSequence();
} else if (!FoundAssignmentOnLine && !FoundLeftParenOnLine &&
Changes[i].Kind == tok::equal) {
FoundAssignmentOnLine = true;
- EndOfSequence = i;
if (StartOfSequence == 0)
StartOfSequence = i;
@@ -199,36 +211,34 @@
}
}
- if (StartOfSequence > 0) {
-EndOfSequence = Changes.size();
-AlignSequence();
- }
+ EndOfSequence = Changes.size();
+ AlignSequence();
}
void WhitespaceManager::alignConsecutiveAssignments(unsigned Start,
unsigned End,
unsigned Column) {
- bool AlignedAssignment = false;
- int PreviousShift = 0;
+ bool FoundAssignmentOnLine = false;
+ int Shift = 0;
for (unsigned i = Start; i != End; ++i) {
-int Shift = 0;
-if (Changes[i].NewlinesBefore > 0)
- AlignedAssignment = false;
-if (!AlignedAssignment && Changes[i].Kind == tok::equal) {
+if (Changes[i].NewlinesBefore > 0) {
+ FoundAssignmentOnLine = false;
+ Shift = 0;
+}
+
+// If this is the first assignment to be aligned, remember by how many
+// spaces it has to be shifted, so the rest of the changes on the line are
+// shifted by the same amoun
Re: [PATCH] D12369: [clang-format] Fix alignConsecutiveAssignments not working properly
berenm marked 5 inline comments as done. berenm added a comment. http://reviews.llvm.org/D12369 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329)
berenm created this revision.
berenm added a reviewer: djasper.
berenm added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
This fixes bug #25329, as well as misalignments like the following:
int a, b = 2;
int c= 3;
http://reviews.llvm.org/D14325
Files:
lib/Format/WhitespaceManager.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8703,6 +8703,16 @@
" loongParameterB);\n"
"int j = 2;",
Alignment);
+
+ verifyFormat("template \n"
+ "auto foo() {}\n",
+ Alignment);
+ verifyFormat("int a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
}
TEST_F(FormatTest, AlignConsecutiveDeclarations) {
@@ -8903,6 +8913,23 @@
"int myvar = 1;",
Alignment);
Alignment.ColumnLimit = 80;
+ Alignment.AlignConsecutiveAssignments = false;
+
+ verifyFormat(
+ "template \n"
+ "auto foo() {}\n",
+ Alignment);
+ verifyFormat("float a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ Alignment.AlignConsecutiveAssignments = true;
+ verifyFormat("float a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ Alignment.AlignConsecutiveAssignments = false;
}
TEST_F(FormatTest, LinuxBraceBreaking) {
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -167,6 +167,8 @@
bool FoundAssignmentOnLine = false;
bool FoundLeftBraceOnLine = false;
bool FoundLeftParenOnLine = false;
+ unsigned CommasOnPrevLine = 0;
+ unsigned CommasOnLine = 0;
// Aligns a sequence of assignment tokens, on the MinColumn column.
//
@@ -186,6 +188,8 @@
for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
if (Changes[i].NewlinesBefore != 0) {
+ CommasOnPrevLine = CommasOnLine;
+ CommasOnLine = 0;
EndOfSequence = i;
// If there is a blank line, if the last line didn't contain any
// assignment, or if we found an open brace or paren, the sequence ends
@@ -225,6 +229,9 @@
FoundLeftParenOnLine = true;
if (!FoundAssignmentOnLine)
AlignSequence();
+} else if (Changes[i].Kind == tok::comma) {
+ if (!FoundAssignmentOnLine)
+CommasOnLine++;
} else if (!FoundAssignmentOnLine && !FoundLeftBraceOnLine &&
!FoundLeftParenOnLine && Changes[i].Kind == tok::equal) {
FoundAssignmentOnLine = true;
@@ -237,7 +244,8 @@
LineLengthAfter += Changes[j].Spaces + Changes[j].TokenLength;
unsigned ChangeMaxColumn = Style.ColumnLimit - LineLengthAfter;
- if (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) {
+ if (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn ||
+ CommasOnPrevLine != CommasOnLine) {
AlignSequence();
StartOfSequence = i;
}
@@ -298,6 +306,8 @@
bool FoundDeclarationOnLine = false;
bool FoundLeftBraceOnLine = false;
bool FoundLeftParenOnLine = false;
+ unsigned CommasOnPrevLine = 0;
+ unsigned CommasOnLine = 0;
auto AlignSequence = [&] {
if (StartOfSequence > 0 && StartOfSequence < EndOfSequence)
@@ -310,6 +320,8 @@
for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
if (Changes[i].NewlinesBefore != 0) {
+ CommasOnPrevLine = CommasOnLine;
+ CommasOnLine = 0;
EndOfSequence = i;
if (Changes[i].NewlinesBefore > 1 || !FoundDeclarationOnLine ||
FoundLeftBraceOnLine || FoundLeftParenOnLine)
@@ -335,6 +347,9 @@
FoundLeftParenOnLine = true;
if (!FoundDeclarationOnLine)
AlignSequence();
+} else if (Changes[i].Kind == tok::comma) {
+ if (!FoundDeclarationOnLine)
+CommasOnLine++;
} else if (!FoundDeclarationOnLine && !FoundLeftBraceOnLine &&
!FoundLeftParenOnLine && Changes[i].IsStartOfDeclName) {
FoundDeclarationOnLine = true;
@@ -347,7 +362,8 @@
LineLengthAfter += Changes[j].Spaces + Changes[j].TokenLength;
unsigned ChangeMaxColumn = Style.ColumnLimit - LineLengthAfter;
- if (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) {
+ if (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn ||
+ CommasOnPrevLine != CommasOnLine) {
AlignSequence();
StartOfSequence = i;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329)
berenm updated this revision to Diff 39201.
berenm added a comment.
[clang-format] Count the number of braces and parens on the line instead of
remembering only one.
http://reviews.llvm.org/D14325
Files:
lib/Format/WhitespaceManager.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8703,6 +8703,19 @@
" loongParameterB);\n"
"int j = 2;",
Alignment);
+
+ verifyFormat("template \n"
+ "auto foo() {}\n",
+ Alignment);
+ verifyFormat("int a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n"
+ "float b[1][] = {{3.f}};\n",
+ Alignment);
}
TEST_F(FormatTest, AlignConsecutiveDeclarations) {
@@ -8903,6 +8916,29 @@
"int myvar = 1;",
Alignment);
Alignment.ColumnLimit = 80;
+ Alignment.AlignConsecutiveAssignments = false;
+
+ verifyFormat(
+ "template \n"
+ "auto foo() {}\n",
+ Alignment);
+ verifyFormat("float a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n"
+ "float b[1][] = {{3.f}};\n",
+ Alignment);
+ Alignment.AlignConsecutiveAssignments = true;
+ verifyFormat("float a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n"
+ "float b[1][] = {{3.f}};\n",
+ Alignment);
+ Alignment.AlignConsecutiveAssignments = false;
}
TEST_F(FormatTest, LinuxBraceBreaking) {
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -165,8 +165,10 @@
unsigned StartOfSequence = 0;
unsigned EndOfSequence = 0;
bool FoundAssignmentOnLine = false;
- bool FoundLeftBraceOnLine = false;
- bool FoundLeftParenOnLine = false;
+ unsigned LeftBracesOnLine = 0;
+ unsigned LeftParensOnLine = 0;
+ unsigned CommasOnPrevLine = 0;
+ unsigned CommasOnLine = 0;
// Aligns a sequence of assignment tokens, on the MinColumn column.
//
@@ -186,21 +188,23 @@
for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
if (Changes[i].NewlinesBefore != 0) {
+ CommasOnPrevLine = CommasOnLine;
+ CommasOnLine = 0;
EndOfSequence = i;
// If there is a blank line, if the last line didn't contain any
// assignment, or if we found an open brace or paren, the sequence ends
// here.
if (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine ||
- FoundLeftBraceOnLine || FoundLeftParenOnLine) {
+ LeftBracesOnLine > 0 || LeftParensOnLine > 0) {
// NB: In the latter case, the sequence should end at the beggining of
// the previous line, but it doesn't really matter as there is no
// assignment on it
AlignSequence();
}
FoundAssignmentOnLine = false;
- FoundLeftBraceOnLine = false;
- FoundLeftParenOnLine = false;
+ LeftBracesOnLine = 0;
+ LeftParensOnLine = 0;
}
// If there is more than one "=" per line, or if the "=" appears first on
@@ -210,23 +214,28 @@
Changes[i + 1].NewlinesBefore > 0)) {
AlignSequence();
} else if (Changes[i].Kind == tok::r_brace) {
- if (!FoundLeftBraceOnLine)
+ if (LeftBracesOnLine == 0)
AlignSequence();
- FoundLeftBraceOnLine = false;
+ else
+LeftBracesOnLine--;
} else if (Changes[i].Kind == tok::l_brace) {
- FoundLeftBraceOnLine = true;
+ LeftBracesOnLine++;
if (!FoundAssignmentOnLine)
AlignSequence();
} else if (Changes[i].Kind == tok::r_paren) {
- if (!FoundLeftParenOnLine)
+ if (LeftParensOnLine == 0)
AlignSequence();
- FoundLeftParenOnLine = false;
+ else
+LeftParensOnLine--;
} else if (Changes[i].Kind == tok::l_paren) {
- FoundLeftParenOnLine = true;
+ LeftParensOnLine++;
if (!FoundAssignmentOnLine)
AlignSequence();
-} else if (!FoundAssignmentOnLine && !FoundLeftBraceOnLine &&
- !FoundLeftParenOnLine && Changes[i].Kind == tok::equal) {
+} else if (Changes[i].Kind == tok::comma) {
+ if (!FoundAssignmentOnLine)
+CommasOnLine++;
+} else if (!FoundAssignmentOnLine && LeftBracesOnLine == 0 &&
+ LeftParensOnLine == 0 && Changes[i].Kind == tok::equal) {
FoundAssignmentOnLine = true;
if (StartOfSequence == 0)
StartOfSequence = i;
@@ -237,7 +246,8 @@
LineLengthAfter +=
Re: [PATCH] D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329)
berenm added a comment. I've also added a fix for the other issue reported on the same bug. I could split the two reviews if necessary. http://reviews.llvm.org/D14325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329)
berenm updated this revision to Diff 39232.
berenm added a comment.
[clang-format] Alignment code factorization.
http://reviews.llvm.org/D14325
Files:
lib/Format/WhitespaceManager.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8703,6 +8703,19 @@
" loongParameterB);\n"
"int j = 2;",
Alignment);
+
+ verifyFormat("template \n"
+ "auto foo() {}\n",
+ Alignment);
+ verifyFormat("int a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n"
+ "float b[1][] = {{3.f}};\n",
+ Alignment);
}
TEST_F(FormatTest, AlignConsecutiveDeclarations) {
@@ -8903,6 +8916,29 @@
"int myvar = 1;",
Alignment);
Alignment.ColumnLimit = 80;
+ Alignment.AlignConsecutiveAssignments = false;
+
+ verifyFormat(
+ "template \n"
+ "auto foo() {}\n",
+ Alignment);
+ verifyFormat("float a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n"
+ "float b[1][] = {{3.f}};\n",
+ Alignment);
+ Alignment.AlignConsecutiveAssignments = true;
+ verifyFormat("float a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n"
+ "float b[1][] = {{3.f}};\n",
+ Alignment);
+ Alignment.AlignConsecutiveAssignments = false;
}
TEST_F(FormatTest, LinuxBraceBreaking) {
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -148,86 +148,108 @@
}
}
-// Walk through all of the changes and find sequences of "=" to align. To do
-// so, keep track of the lines and whether or not an "=" was found on align. If
-// a "=" is found on a line, extend the current sequence. If the current line
-// cannot be part of a sequence, e.g. because there is an empty line before it
-// or it contains non-assignments, finalize the previous sequence.
-//
-// FIXME: The code between assignment and declaration alignment is mostly
-// duplicated and would benefit from factorization.
-void WhitespaceManager::alignConsecutiveAssignments() {
- if (!Style.AlignConsecutiveAssignments)
-return;
-
+struct TokenSequenceAligner {
+ template
+ void align(const FormatStyle &Style, F &&Matches,
+ SmallVector &Changes);
+
+ template
+ void alignSequence(unsigned Start, unsigned End, unsigned Column, F &&Matches,
+ SmallVector &Changes);
+};
+
+// Walk through all of the changes and find sequences of matching tokens to
+// align. To do so, keep track of the lines and whether or not a matching token
+// was found on a line. If a matching token is found, extend the current
+// sequence. If the current line cannot be part of a sequence, e.g. because
+// there is an empty line before it or it contains only non-matching tokens,
+// finalize the previous sequence.
+template
+void TokenSequenceAligner::align(
+const FormatStyle &Style, F &&Matches,
+SmallVector &Changes) {
unsigned MinColumn = 0;
unsigned MaxColumn = UINT_MAX;
+
+ // Line number of the start and the end of the current token sequence.
unsigned StartOfSequence = 0;
unsigned EndOfSequence = 0;
- bool FoundAssignmentOnLine = false;
- bool FoundLeftBraceOnLine = false;
- bool FoundLeftParenOnLine = false;
- // Aligns a sequence of assignment tokens, on the MinColumn column.
+ // Keep track of the number of braces and parentheses on the current line. If
+ // they aren't balanced, we will end the sequence.
+ unsigned LeftBracesOnLine = 0;
+ unsigned LeftParensOnLine = 0;
+
+ // Keep track of the number of commas on the line, we will only align a
+ // sequence of matching tokens if they are preceded by the same number of
+ // commas.
+ unsigned CommasOnPrevLine = 0;
+ unsigned CommasOnLine = 0;
+
+ // Whether a matching token has been found on the current line.
+ bool FoundOnLine = false;
+
+ // Aligns a sequence of matching tokens, on the MinColumn column.
//
- // Sequences start from the first assignment token to align, and end at the
+ // Sequences start from the first matching token to align, and end at the
// first token of the first line that doesn't need to be aligned.
//
// We need to adjust the StartOfTokenColumn of each Change that is on a line
- // containing any assignment to be aligned and located after such assignment
- auto AlignSequence = [&] {
+ // containing any matching tok
Re: [PATCH] D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329)
berenm updated this revision to Diff 39233.
berenm added a comment.
[clang-format] Remove deleted methods declaration.
http://reviews.llvm.org/D14325
Files:
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8703,6 +8703,19 @@
" loongParameterB);\n"
"int j = 2;",
Alignment);
+
+ verifyFormat("template \n"
+ "auto foo() {}\n",
+ Alignment);
+ verifyFormat("int a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n"
+ "float b[1][] = {{3.f}};\n",
+ Alignment);
}
TEST_F(FormatTest, AlignConsecutiveDeclarations) {
@@ -8903,6 +8916,29 @@
"int myvar = 1;",
Alignment);
Alignment.ColumnLimit = 80;
+ Alignment.AlignConsecutiveAssignments = false;
+
+ verifyFormat(
+ "template \n"
+ "auto foo() {}\n",
+ Alignment);
+ verifyFormat("float a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n"
+ "float b[1][] = {{3.f}};\n",
+ Alignment);
+ Alignment.AlignConsecutiveAssignments = true;
+ verifyFormat("float a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n"
+ "float b[1][] = {{3.f}};\n",
+ Alignment);
+ Alignment.AlignConsecutiveAssignments = false;
}
TEST_F(FormatTest, LinuxBraceBreaking) {
Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -168,20 +168,9 @@
/// \brief Align consecutive assignments over all \c Changes.
void alignConsecutiveAssignments();
- /// \brief Align consecutive assignments from change \p Start to change \p End
- /// at
- /// the specified \p Column.
- void alignConsecutiveAssignments(unsigned Start, unsigned End,
- unsigned Column);
-
/// \brief Align consecutive declarations over all \c Changes.
void alignConsecutiveDeclarations();
- /// \brief Align consecutive declarations from change \p Start to change \p
- /// End at the specified \p Column.
- void alignConsecutiveDeclarations(unsigned Start, unsigned End,
-unsigned Column);
-
/// \brief Align trailing comments over all \c Changes.
void alignTrailingComments();
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -148,86 +148,108 @@
}
}
-// Walk through all of the changes and find sequences of "=" to align. To do
-// so, keep track of the lines and whether or not an "=" was found on align. If
-// a "=" is found on a line, extend the current sequence. If the current line
-// cannot be part of a sequence, e.g. because there is an empty line before it
-// or it contains non-assignments, finalize the previous sequence.
-//
-// FIXME: The code between assignment and declaration alignment is mostly
-// duplicated and would benefit from factorization.
-void WhitespaceManager::alignConsecutiveAssignments() {
- if (!Style.AlignConsecutiveAssignments)
-return;
-
+struct TokenSequenceAligner {
+ template
+ void align(const FormatStyle &Style, F &&Matches,
+ SmallVector &Changes);
+
+ template
+ void alignSequence(unsigned Start, unsigned End, unsigned Column, F &&Matches,
+ SmallVector &Changes);
+};
+
+// Walk through all of the changes and find sequences of matching tokens to
+// align. To do so, keep track of the lines and whether or not a matching token
+// was found on a line. If a matching token is found, extend the current
+// sequence. If the current line cannot be part of a sequence, e.g. because
+// there is an empty line before it or it contains only non-matching tokens,
+// finalize the previous sequence.
+template
+void TokenSequenceAligner::align(
+const FormatStyle &Style, F &&Matches,
+SmallVector &Changes) {
unsigned MinColumn = 0;
unsigned MaxColumn = UINT_MAX;
+
+ // Line number of the start and the end of the current token sequence.
unsigned StartOfSequence = 0;
unsigned EndOfSequence = 0;
- bool FoundAssignmentOnLine = false;
- bool FoundLeftBraceOnLine = false;
- bool FoundLeftParenOnLine = false;
- // Aligns a sequence of assignment tokens, on the MinColumn column.
+ // Keep track of the number o
Re: [PATCH] D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329)
berenm marked an inline comment as done.
Comment at: lib/Format/WhitespaceManager.cpp:150-151
@@ -149,109 +149,4 @@
}
-// Walk through all of the changes and find sequences of "=" to align. To do
-// so, keep track of the lines and whether or not an "=" was found on align. If
-// a "=" is found on a line, extend the current sequence. If the current line
-// cannot be part of a sequence, e.g. because there is an empty line before it
-// or it contains non-assignments, finalize the previous sequence.
-//
-// FIXME: The code between assignment and declaration alignment is mostly
-// duplicated and would benefit from factorization.
-void WhitespaceManager::alignConsecutiveAssignments() {
- if (!Style.AlignConsecutiveAssignments)
-return;
-
- unsigned MinColumn = 0;
- unsigned MaxColumn = UINT_MAX;
- unsigned StartOfSequence = 0;
- unsigned EndOfSequence = 0;
- bool FoundAssignmentOnLine = false;
- bool FoundLeftBraceOnLine = false;
- bool FoundLeftParenOnLine = false;
-
- // Aligns a sequence of assignment tokens, on the MinColumn column.
- //
- // Sequences start from the first assignment token to align, and end at the
- // first token of the first line that doesn't need to be aligned.
- //
- // We need to adjust the StartOfTokenColumn of each Change that is on a line
- // containing any assignment to be aligned and located after such assignment
- auto AlignSequence = [&] {
-if (StartOfSequence > 0 && StartOfSequence < EndOfSequence)
- alignConsecutiveAssignments(StartOfSequence, EndOfSequence, MinColumn);
-MinColumn = 0;
-MaxColumn = UINT_MAX;
-StartOfSequence = 0;
-EndOfSequence = 0;
- };
-
- for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
-if (Changes[i].NewlinesBefore != 0) {
- EndOfSequence = i;
- // If there is a blank line, if the last line didn't contain any
- // assignment, or if we found an open brace or paren, the sequence ends
- // here.
- if (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine ||
- FoundLeftBraceOnLine || FoundLeftParenOnLine) {
-// NB: In the latter case, the sequence should end at the beggining of
-// the previous line, but it doesn't really matter as there is no
-// assignment on it
-AlignSequence();
- }
-
- FoundAssignmentOnLine = false;
- FoundLeftBraceOnLine = false;
- FoundLeftParenOnLine = false;
-}
-
-// If there is more than one "=" per line, or if the "=" appears first on
-// the line of if it appears last, end the sequence
-if (Changes[i].Kind == tok::equal &&
-(FoundAssignmentOnLine || Changes[i].NewlinesBefore > 0 ||
- Changes[i + 1].NewlinesBefore > 0)) {
- AlignSequence();
-} else if (Changes[i].Kind == tok::r_brace) {
- if (!FoundLeftBraceOnLine)
-AlignSequence();
- FoundLeftBraceOnLine = false;
-} else if (Changes[i].Kind == tok::l_brace) {
- FoundLeftBraceOnLine = true;
- if (!FoundAssignmentOnLine)
-AlignSequence();
-} else if (Changes[i].Kind == tok::r_paren) {
- if (!FoundLeftParenOnLine)
-AlignSequence();
- FoundLeftParenOnLine = false;
-} else if (Changes[i].Kind == tok::l_paren) {
- FoundLeftParenOnLine = true;
- if (!FoundAssignmentOnLine)
-AlignSequence();
-} else if (!FoundAssignmentOnLine && !FoundLeftBraceOnLine &&
- !FoundLeftParenOnLine && Changes[i].Kind == tok::equal) {
- FoundAssignmentOnLine = true;
- if (StartOfSequence == 0)
-StartOfSequence = i;
-
- unsigned ChangeMinColumn = Changes[i].StartOfTokenColumn;
- int LineLengthAfter = -Changes[i].Spaces;
- for (unsigned j = i; j != e && Changes[j].NewlinesBefore == 0; ++j)
-LineLengthAfter += Changes[j].Spaces + Changes[j].TokenLength;
- unsigned ChangeMaxColumn = Style.ColumnLimit - LineLengthAfter;
-
- if (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) {
-AlignSequence();
-StartOfSequence = i;
- }
-
- MinColumn = std::max(MinColumn, ChangeMinColumn);
- MaxColumn = std::min(MaxColumn, ChangeMaxColumn);
-}
- }
-
- EndOfSequence = Changes.size();
- AlignSequence();
-}
-
-void WhitespaceManager::alignConsecutiveAssignments(unsigned Start,
-unsigned End,
-unsigned Column) {
- bool FoundAssignmentOnLine = false;
+// Align a single sequence of tokens, see AlignTokens below.
+template
Haha, good question. I initially moved the variable there, but then it didn't
looked really better, so I moved the variables back into the function...
http://reviews.llvm.org/D14325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk
berenm added inline comments. Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:86 @@ -69,1 +85,3 @@ +IComponentModel componentModel = GetService(typeof(SComponentModel)) as IComponentModel; +editorAdaptersFactoryService = componentModel.GetService(); I did more tests on my side, and apparently this line does not work on VS2012, componentModel is null. I don't know at all why and how to fix it, and it works fine starting with VS2013. http://reviews.llvm.org/D12407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10933: Add new IdentifierNaming check
berenm updated this revision to Diff 31447.
berenm added a comment.
Here is an updated version with some style fixes, and function splits.
The styles are still selected the same way as before (no split between finding
the best type style and falling back to available style), but there is only the
StyleKind returned and the warning message is deduced from the StyleNames
(lower case conversion and underscores replaced with spaces).
I can still implement the mechanism suggested in comment #218645 if you find it
is cleaner.
http://reviews.llvm.org/D10933
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tidy/readability/IdentifierNamingCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -0,0 +1,261 @@
+// RUN: $(dirname %s)/check_clang_tidy.sh %s readability-identifier-naming %t \
+// RUN: -config='{CheckOptions: [ \
+// RUN: {key: readability-identifier-naming.AbstractClassCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.AbstractClassPrefix, value: 'A'}, \
+// RUN: {key: readability-identifier-naming.ClassCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ClassPrefix, value: 'C'}, \
+// RUN: {key: readability-identifier-naming.ClassConstantCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ClassConstantPrefix, value: 'k'}, \
+// RUN: {key: readability-identifier-naming.ClassMemberCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ClassMethodCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.ConstantCase, value: UPPER_CASE}, \
+// RUN: {key: readability-identifier-naming.ConstantSuffix, value: '_CST'}, \
+// RUN: {key: readability-identifier-naming.ConstexprFunctionCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.ConstexprMethodCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.ConstexprVariableCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.EnumCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.EnumPrefix, value: 'E'}, \
+// RUN: {key: readability-identifier-naming.EnumConstantCase, value: UPPER_CASE}, \
+// RUN: {key: readability-identifier-naming.FunctionCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.GlobalConstantCase, value: UPPER_CASE}, \
+// RUN: {key: readability-identifier-naming.GlobalFunctionCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.GlobalVariableCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.GlobalVariablePrefix, value: 'g_'}, \
+// RUN: {key: readability-identifier-naming.InlineNamespaceCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.LocalConstantCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.LocalConstantPrefix, value: 'k'}, \
+// RUN: {key: readability-identifier-naming.LocalVariableCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.MemberCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.MemberPrefix, value: 'm_'}, \
+// RUN: {key: readability-identifier-naming.ConstantMemberCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.PrivateMemberPrefix, value: '__'}, \
+// RUN: {key: readability-identifier-naming.ProtectedMemberPrefix, value: '_'}, \
+// RUN: {key: readability-identifier-naming.PublicMemberCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.MethodCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.PrivateMethodPrefix, value: '__'}, \
+// RUN: {key: readability-identifier-naming.ProtectedMethodPrefix, value: '_'}, \
+// RUN: {key: readability-identifier-naming.NamespaceCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.ParameterCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.ParameterPrefix, value: 'a_'}, \
+// RUN: {key: readability-identifier-naming.ConstantParameterCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.ConstantParameterPrefix, value: 'i_'}, \
+// RUN: {key: readability-identifier-naming.ParameterPackCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.PureFunctionCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.PureMethodCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.StaticConstantCase, value: UPPER_CASE}, \
+// RUN: {key: readability-identifier-naming.StaticVariableCase, value: camelBack}, \
+// RUN: {key: readabili
Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes
berenm added a comment. I think it's better to update the diff with full context (git diff -U99) rather than attach the file, in order to have it integrated into phabricator diff view. Or use the arcanist command-line tool, which should do it for you. Except from that, from a quick reading it looks good to me and I'm happy that someone fixed theses cases, I see them from time to time actually but never had time to work on a proper fix. Repository: rL LLVM http://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes
berenm added a comment. There's still cases where the nesting level is still not correctly handled: when using unbraced conditionals / loops. For example (sorry, silly example): for (auto index = 0, e = 1000; index < e; ++index) int v = 0; long double value = 1; is aligned to for (auto index = 0, e = 1000; index < e; ++index) int v = 0; long double value = 1; I'm not sure how to detect these unbraced scopes, maybe by also looking for different IndentLevel when computing the ScopeLevel? http://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes
berenm added a comment.
It looks like it doesn't like the `operator[]` either:
struct test {
long long int foo();
int operator[](int a);
double bar();
long long int foo();
int operator()(int a);
double bar();
};
becomes:
struct test {
long long int foo();
int operator[](int a);
double bar();
long long int foo();
int operator()(int a);
doublebar();
};
http://reviews.llvm.org/D21279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes
berenm added a comment.
Thanks! The operators are now correctly handled.
Another thing I've found is that constructors aren't aligned either with other
declarations or either together. Do you think it would be possible to treat
them as functions as well?
Friend functions also aren't aligned with the other functions, but I'm not sure
why or even if they should be. I believe most of the time friend functions are
declared in a separate declaration "group" anyway.
struct A {
explicit A(int);
A();
unsigned operator=(int a);
long bar(int a);
friend void foo();
friend unsigned baz();
};
Repository:
rL LLVM
http://reviews.llvm.org/D21279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes
berenm added a comment. In http://reviews.llvm.org/D21279#462578, @bmharper wrote: > 2. friend functions: I don't really understand why the current behavior is > what it is, but I think it's reasonable to argue that it actually improves > readability by drawing attention to the fact these are friend functions, > which ought to be quite rare in most code Actually it looks like it works now... I'm not sure what I did when I had this misalignment. It would have been fine by me anyway. Regarding constructors, your comment seems reasonable to me. The patch already improves the current state, so I think it's good like it is and further improvements could be added later on. Ping @djasper for his review and eventual merge. Comment at: lib/Format/WhitespaceManager.cpp:95 @@ -97,2 +94,3 @@ std::sort(Changes.begin(), Changes.end(), Change::IsBeforeInFile(SourceMgr)); + calculateScopeLevel(); calculateLineBreakInformation(); Maybe we could spare the computation if we aren't going to align anything? Is it better for clarity to always compute additional information? @djasper what's the Clang way to do? Repository: rL LLVM http://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329)
berenm marked an inline comment as done. berenm added a comment. Ping? http://reviews.llvm.org/D14325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329)
berenm updated this revision to Diff 41271.
berenm added a comment.
[clang-format] alignConsecutiveXXX: replace the buggy paren / braces counter
with a better scope tracker.
http://reviews.llvm.org/D14325
Files:
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8659,7 +8659,7 @@
Alignment);
verifyFormat("class C {\n"
"public:\n"
- " int i = 1;\n"
+ " int i= 1;\n"
" virtual void f() = 0;\n"
"};",
Alignment);
@@ -8708,6 +8708,19 @@
" loongParameterB);\n"
"int j = 2;",
Alignment);
+
+ verifyFormat("template \n"
+ "auto foo() {}\n",
+ Alignment);
+ verifyFormat("int a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n"
+ "float b[1][] = {{3.f}};\n",
+ Alignment);
}
TEST_F(FormatTest, AlignConsecutiveDeclarations) {
@@ -8908,6 +8921,47 @@
"int myvar = 1;",
Alignment);
Alignment.ColumnLimit = 80;
+ Alignment.AlignConsecutiveAssignments = false;
+
+ verifyFormat(
+ "template \n"
+ "auto foo() {}\n",
+ Alignment);
+ verifyFormat("float a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n"
+ "float b[1][] = {{3.f}};\n",
+ Alignment);
+ Alignment.AlignConsecutiveAssignments = true;
+ verifyFormat("float a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n"
+ "float b[1][] = {{3.f}};\n",
+ Alignment);
+ Alignment.AlignConsecutiveAssignments = false;
+
+ Alignment.ColumnLimit = 30;
+ Alignment.BinPackParameters = false;
+ verifyFormat("void foo(float a,\n"
+ " float b,\n"
+ " int c,\n"
+ " uint32_t *d) {\n"
+ " int * e = 0;\n"
+ " float f = 0;\n"
+ " double g = 0;\n"
+ "}\n"
+ "void bar(ino_t a,\n"
+ " int b,\n"
+ " uint32_t *c,\n"
+ " bool d) {}\n",
+ Alignment);
+ Alignment.BinPackParameters = true;
+ Alignment.ColumnLimit = 80;
}
TEST_F(FormatTest, LinuxBraceBreaking) {
Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -168,20 +168,9 @@
/// \brief Align consecutive assignments over all \c Changes.
void alignConsecutiveAssignments();
- /// \brief Align consecutive assignments from change \p Start to change \p End
- /// at
- /// the specified \p Column.
- void alignConsecutiveAssignments(unsigned Start, unsigned End,
- unsigned Column);
-
/// \brief Align consecutive declarations over all \c Changes.
void alignConsecutiveDeclarations();
- /// \brief Align consecutive declarations from change \p Start to change \p
- /// End at the specified \p Column.
- void alignConsecutiveDeclarations(unsigned Start, unsigned End,
-unsigned Column);
-
/// \brief Align trailing comments over all \c Changes.
void alignTrailingComments();
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -148,125 +148,24 @@
}
}
-// Walk through all of the changes and find sequences of "=" to align. To do
-// so, keep track of the lines and whether or not an "=" was found on align. If
-// a "=" is found on a line, extend the current sequence. If the current line
-// cannot be part of a sequence, e.g. because there is an empty line before it
-// or it contains non-assignments, finalize the previous sequence.
-//
-// FIXME: The code between assignment and declaration alignment is mostly
-// duplicated and would benefit from factorization.
-void WhitespaceManager::alignConsecutiveAssignments() {
- if (!Style.AlignConsecutiveAssignments)
-return;
-
- unsigned MinColumn = 0;
- unsigned MaxColumn = UINT_MAX;
- unsigned StartOfSequence = 0;
- unsigned EndOfSequence = 0;
- bool FoundAssignmentOnLine = false;
- bool FoundLeftBraceOnLine = false;
- bool FoundLeftParenOnLine = false;
-
- // Aligns a sequence o
Re: [PATCH] D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329)
berenm added a comment. Updated the diff with a fix for the issue reported in http://lists.llvm.org/pipermail/cfe-dev/2015-November/046057.html http://reviews.llvm.org/D14325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329)
berenm updated this revision to Diff 41296.
berenm added a comment.
[clang-format] Code cleanup and variable naming in
WhitespaceManager::AlignTokens
http://reviews.llvm.org/D14325
Files:
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8659,7 +8659,7 @@
Alignment);
verifyFormat("class C {\n"
"public:\n"
- " int i = 1;\n"
+ " int i= 1;\n"
" virtual void f() = 0;\n"
"};",
Alignment);
@@ -8708,6 +8708,19 @@
" loongParameterB);\n"
"int j = 2;",
Alignment);
+
+ verifyFormat("template \n"
+ "auto foo() {}\n",
+ Alignment);
+ verifyFormat("int a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n"
+ "float b[1][] = {{3.f}};\n",
+ Alignment);
}
TEST_F(FormatTest, AlignConsecutiveDeclarations) {
@@ -8908,6 +8921,47 @@
"int myvar = 1;",
Alignment);
Alignment.ColumnLimit = 80;
+ Alignment.AlignConsecutiveAssignments = false;
+
+ verifyFormat(
+ "template \n"
+ "auto foo() {}\n",
+ Alignment);
+ verifyFormat("float a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n"
+ "float b[1][] = {{3.f}};\n",
+ Alignment);
+ Alignment.AlignConsecutiveAssignments = true;
+ verifyFormat("float a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n"
+ "float b[1][] = {{3.f}};\n",
+ Alignment);
+ Alignment.AlignConsecutiveAssignments = false;
+
+ Alignment.ColumnLimit = 30;
+ Alignment.BinPackParameters = false;
+ verifyFormat("void foo(float a,\n"
+ " float b,\n"
+ " int c,\n"
+ " uint32_t *d) {\n"
+ " int * e = 0;\n"
+ " float f = 0;\n"
+ " double g = 0;\n"
+ "}\n"
+ "void bar(ino_t a,\n"
+ " int b,\n"
+ " uint32_t *c,\n"
+ " bool d) {}\n",
+ Alignment);
+ Alignment.BinPackParameters = true;
+ Alignment.ColumnLimit = 80;
}
TEST_F(FormatTest, LinuxBraceBreaking) {
Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -168,20 +168,9 @@
/// \brief Align consecutive assignments over all \c Changes.
void alignConsecutiveAssignments();
- /// \brief Align consecutive assignments from change \p Start to change \p End
- /// at
- /// the specified \p Column.
- void alignConsecutiveAssignments(unsigned Start, unsigned End,
- unsigned Column);
-
/// \brief Align consecutive declarations over all \c Changes.
void alignConsecutiveDeclarations();
- /// \brief Align consecutive declarations from change \p Start to change \p
- /// End at the specified \p Column.
- void alignConsecutiveDeclarations(unsigned Start, unsigned End,
-unsigned Column);
-
/// \brief Align trailing comments over all \c Changes.
void alignTrailingComments();
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -148,125 +148,24 @@
}
}
-// Walk through all of the changes and find sequences of "=" to align. To do
-// so, keep track of the lines and whether or not an "=" was found on align. If
-// a "=" is found on a line, extend the current sequence. If the current line
-// cannot be part of a sequence, e.g. because there is an empty line before it
-// or it contains non-assignments, finalize the previous sequence.
-//
-// FIXME: The code between assignment and declaration alignment is mostly
-// duplicated and would benefit from factorization.
-void WhitespaceManager::alignConsecutiveAssignments() {
- if (!Style.AlignConsecutiveAssignments)
-return;
-
- unsigned MinColumn = 0;
- unsigned MaxColumn = UINT_MAX;
- unsigned StartOfSequence = 0;
- unsigned EndOfSequence = 0;
- bool FoundAssignmentOnLine = false;
- bool FoundLeftBraceOnLine = false;
- bool FoundLeftParenOnLine = false;
-
- // Aligns a sequence of assignment tokens, on
Re: [PATCH] D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329)
berenm marked 2 inline comments as done.
Comment at: lib/Format/WhitespaceManager.cpp:197
@@ +196,3 @@
+ // Keep track of the nesting level of matching tokens, i.e. the number of
+ // surrounding (), [], or {}. We will only align a sequence of matching
+ // token that share the same scope depth.
I have added some details in the comments, but couldn't manage to use the
FormatToken::NestingLevel member. I haven't figured out why exactly, but it
doesn't increase the nesting level counter in the braced scope of examples like:
```
int l = []() {
int i = 0;
int h = 0;
}
```
i.e.: tokens `i` and `h` have `NestingLevel == 0` as it is the case for token
`l`.
I will try to figure out why exactly but it will require a bit more time.
Comment at: lib/Format/WhitespaceManager.cpp:263-274
@@ -321,21 +262,14 @@
-if (Changes[i].Kind == tok::r_brace) {
- if (!FoundLeftBraceOnLine)
-AlignSequence();
- FoundLeftBraceOnLine = false;
-} else if (Changes[i].Kind == tok::l_brace) {
- FoundLeftBraceOnLine = true;
- if (!FoundDeclarationOnLine)
-AlignSequence();
-} else if (Changes[i].Kind == tok::r_paren) {
- if (!FoundLeftParenOnLine)
-AlignSequence();
- FoundLeftParenOnLine = false;
-} else if (Changes[i].Kind == tok::l_paren) {
- FoundLeftParenOnLine = true;
- if (!FoundDeclarationOnLine)
-AlignSequence();
-} else if (!FoundDeclarationOnLine && !FoundLeftBraceOnLine &&
- !FoundLeftParenOnLine && Changes[i].IsStartOfDeclName) {
- FoundDeclarationOnLine = true;
+if (Matches(Changes[i])) {
+ // If there is more than one matching token per line, or if the number of
+ // preceding commas, or the scope depth, do not match anymore, end the
+ // sequence.
+ if (FoundMatchOnLine || CommasBeforeMatch != CommasBeforeLastMatch ||
+ NestingLevel != NestingLevelOfLastMatch)
+AlignCurrentSequence();
+
+ CommasBeforeLastMatch = CommasBeforeMatch;
+ NestingLevelOfLastMatch = NestingLevel;
+ FoundMatchOnLine = true;
+
if (StartOfSequence == 0)
I did that.
It makes me think that the code actually forbids the generic `AlignToken`
function to be used with a matcher for aligning commas or braces, paren and
brackets...
http://reviews.llvm.org/D14325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329)
berenm updated this revision to Diff 41299.
berenm marked an inline comment as done.
berenm added a comment.
Fix continue statements.
http://reviews.llvm.org/D14325
Files:
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8659,7 +8659,7 @@
Alignment);
verifyFormat("class C {\n"
"public:\n"
- " int i = 1;\n"
+ " int i= 1;\n"
" virtual void f() = 0;\n"
"};",
Alignment);
@@ -8708,6 +8708,19 @@
" loongParameterB);\n"
"int j = 2;",
Alignment);
+
+ verifyFormat("template \n"
+ "auto foo() {}\n",
+ Alignment);
+ verifyFormat("int a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n"
+ "float b[1][] = {{3.f}};\n",
+ Alignment);
}
TEST_F(FormatTest, AlignConsecutiveDeclarations) {
@@ -8908,6 +8921,47 @@
"int myvar = 1;",
Alignment);
Alignment.ColumnLimit = 80;
+ Alignment.AlignConsecutiveAssignments = false;
+
+ verifyFormat(
+ "template \n"
+ "auto foo() {}\n",
+ Alignment);
+ verifyFormat("float a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n"
+ "float b[1][] = {{3.f}};\n",
+ Alignment);
+ Alignment.AlignConsecutiveAssignments = true;
+ verifyFormat("float a, b = 1;\n"
+ "int c = 2;\n"
+ "int dd = 3;\n",
+ Alignment);
+ verifyFormat("int aa = ((1 > 2) ? 3 : 4);\n"
+ "float b[1][] = {{3.f}};\n",
+ Alignment);
+ Alignment.AlignConsecutiveAssignments = false;
+
+ Alignment.ColumnLimit = 30;
+ Alignment.BinPackParameters = false;
+ verifyFormat("void foo(float a,\n"
+ " float b,\n"
+ " int c,\n"
+ " uint32_t *d) {\n"
+ " int * e = 0;\n"
+ " float f = 0;\n"
+ " double g = 0;\n"
+ "}\n"
+ "void bar(ino_t a,\n"
+ " int b,\n"
+ " uint32_t *c,\n"
+ " bool d) {}\n",
+ Alignment);
+ Alignment.BinPackParameters = true;
+ Alignment.ColumnLimit = 80;
}
TEST_F(FormatTest, LinuxBraceBreaking) {
Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -168,20 +168,9 @@
/// \brief Align consecutive assignments over all \c Changes.
void alignConsecutiveAssignments();
- /// \brief Align consecutive assignments from change \p Start to change \p End
- /// at
- /// the specified \p Column.
- void alignConsecutiveAssignments(unsigned Start, unsigned End,
- unsigned Column);
-
/// \brief Align consecutive declarations over all \c Changes.
void alignConsecutiveDeclarations();
- /// \brief Align consecutive declarations from change \p Start to change \p
- /// End at the specified \p Column.
- void alignConsecutiveDeclarations(unsigned Start, unsigned End,
-unsigned Column);
-
/// \brief Align trailing comments over all \c Changes.
void alignTrailingComments();
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -148,125 +148,24 @@
}
}
-// Walk through all of the changes and find sequences of "=" to align. To do
-// so, keep track of the lines and whether or not an "=" was found on align. If
-// a "=" is found on a line, extend the current sequence. If the current line
-// cannot be part of a sequence, e.g. because there is an empty line before it
-// or it contains non-assignments, finalize the previous sequence.
-//
-// FIXME: The code between assignment and declaration alignment is mostly
-// duplicated and would benefit from factorization.
-void WhitespaceManager::alignConsecutiveAssignments() {
- if (!Style.AlignConsecutiveAssignments)
-return;
-
- unsigned MinColumn = 0;
- unsigned MaxColumn = UINT_MAX;
- unsigned StartOfSequence = 0;
- unsigned EndOfSequence = 0;
- bool FoundAssignmentOnLine = false;
- bool FoundLeftBraceOnLine = false;
- bool FoundLeftParenOnLine = false;
-
- // Aligns a sequence of assignment tokens, on the MinColumn col
Re: [PATCH] D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329)
berenm added inline comments.
Comment at: lib/Format/WhitespaceManager.cpp:242-243
@@ -320,34 +241,4 @@
}
-if (Changes[i].Kind == tok::r_brace) {
- if (!FoundLeftBraceOnLine)
-AlignSequence();
- FoundLeftBraceOnLine = false;
-} else if (Changes[i].Kind == tok::l_brace) {
- FoundLeftBraceOnLine = true;
- if (!FoundDeclarationOnLine)
-AlignSequence();
-} else if (Changes[i].Kind == tok::r_paren) {
- if (!FoundLeftParenOnLine)
-AlignSequence();
- FoundLeftParenOnLine = false;
-} else if (Changes[i].Kind == tok::l_paren) {
- FoundLeftParenOnLine = true;
- if (!FoundDeclarationOnLine)
-AlignSequence();
-} else if (!FoundDeclarationOnLine && !FoundLeftBraceOnLine &&
- !FoundLeftParenOnLine && Changes[i].IsStartOfDeclName) {
- FoundDeclarationOnLine = true;
- if (StartOfSequence == 0)
-StartOfSequence = i;
-
- unsigned ChangeMinColumn = Changes[i].StartOfTokenColumn;
- int LineLengthAfter = -Changes[i].Spaces;
- for (unsigned j = i; j != e && Changes[j].NewlinesBefore == 0; ++j)
-LineLengthAfter += Changes[j].Spaces + Changes[j].TokenLength;
- unsigned ChangeMaxColumn = Style.ColumnLimit - LineLengthAfter;
-
- if (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) {
-AlignSequence();
-StartOfSequence = i;
- }
+if (Changes[i].Kind == tok::comma) {
+ ++CommasBeforeMatch;
The code was only checking for a match if none of the previous conditions were
true. I changed that and with your suggestion so I guess it's OK now.
http://reviews.llvm.org/D14325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329)
berenm added a comment. I don't have commit access, if you don't mind landing it for me :) http://reviews.llvm.org/D14325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329)
berenm added a comment.
I don't have any compilation error with GCC 5.2.1. I tried again with Clang 3.8
and indeed there is this error:
tools/clang/lib/Format/WhitespaceManager.cpp:155:51: error: 'Change' is a
private member of 'clang::format::WhitespaceManager'
SmallVector &Changes) {
^
tools/clang/lib/Format/WhitespaceManager.h:87:10: note: declared private here
struct Change {
^
This can easily be reproduced: http://goo.gl/CUk665 vs http://goo.gl/6KcBFT
I don't know precisely which one is correct here, it looks like to be related
to the access permission in the context of a template function instantiation.
http://reviews.llvm.org/D14325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk
berenm updated this revision to Diff 42718.
berenm added a comment.
- Patch rebased on trunk
- Small cleanups in the RunningDocumentTable usage
- Tested and appears work on VS >= 2012
http://reviews.llvm.org/D12407
Files:
tools/clang-format-vs/ClangFormat/ClangFormat.csproj
tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
Index: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
===
--- tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
+++ tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
@@ -12,12 +12,14 @@
//
//===--===//
+using Microsoft.VisualStudio;
using Microsoft.VisualStudio.Editor;
using Microsoft.VisualStudio.Shell;
using Microsoft.VisualStudio.Shell.Interop;
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.TextManager.Interop;
+using Microsoft.VisualStudio.ComponentModelHost;
using System;
using System.Collections;
using System.ComponentModel;
@@ -163,28 +165,51 @@
get { return sortIncludes; }
set { sortIncludes = value; }
}
+
+[Category("LLVM/Clang")]
+[DisplayName("Reformat On Save")]
+[Description("Reformat code when the source file is saved on disk")]
+public bool ReformatOnSave
+{
+get;
+set;
+}
}
[PackageRegistration(UseManagedResourcesOnly = true)]
[InstalledProductRegistration("#110", "#112", "1.0", IconResourceID = 400)]
[ProvideMenuResource("Menus.ctmenu", 1)]
[Guid(GuidList.guidClangFormatPkgString)]
[ProvideOptionPage(typeof(OptionPageGrid), "LLVM/Clang", "ClangFormat", 0, 0, true)]
-public sealed class ClangFormatPackage : Package
+[ProvideAutoLoad(UIContextGuids80.NoSolution)]
+public sealed class ClangFormatPackage : Package, IVsRunningDocTableEvents3
{
+private RunningDocumentTable runningDocumentTable;
+private uint adviseCookie;
+
#region Package Members
protected override void Initialize()
{
base.Initialize();
+runningDocumentTable = new RunningDocumentTable(this);
+adviseCookie = runningDocumentTable.Advise(this);
+
var commandService = GetService(typeof(IMenuCommandService)) as OleMenuCommandService;
if (commandService != null)
{
var menuCommandID = new CommandID(GuidList.guidClangFormatCmdSet, (int)PkgCmdIDList.cmdidClangFormat);
var menuItem = new MenuCommand(MenuItemCallback, menuCommandID);
commandService.AddCommand(menuItem);
}
}
+
+protected override void Dispose(bool disposing)
+{
+runningDocumentTable.Unadvise(adviseCookie);
+
+base.Dispose(disposing);
+}
#endregion
private void MenuItemCallback(object sender, EventArgs args)
@@ -202,10 +227,16 @@
if (start >= text.Length && text.Length > 0)
start = text.Length - 1;
string path = GetDocumentParent(view);
+FormatTextBuffer(view.TextBuffer, start, length, path);
+}
+
+private void FormatTextBuffer(ITextBuffer textBuffer, int offset, int length, string path)
+{
try
{
-var root = XElement.Parse(RunClangFormat(text, start, length, path));
-var edit = view.TextBuffer.CreateEdit();
+string text = textBuffer.CurrentSnapshot.GetText();
+var root = XElement.Parse(RunClangFormat(text, offset, length, path));
+var edit = textBuffer.CreateEdit();
foreach (XElement replacement in root.Descendants("replacement"))
{
var span = new Span(
@@ -316,7 +347,7 @@
var userData = (IVsUserData)textView;
if (userData == null)
return null;
-Guid guidWpfViewHost = DefGuidList.guidIWpfTextViewHost;
+Guid guidWpfViewHost = Microsoft.VisualStudio.Editor.DefGuidList.guidIWpfTextViewHost;
object host;
userData.GetData(ref guidWpfViewHost, out host);
return ((IWpfTextViewHost)host).TextView;
@@ -346,6 +377,12 @@
return page.SortIncludes;
}
+private bool ShouldReformatOnSave()
+{
+var page = (OptionPageGrid)GetDialogPage(typeof(OptionPageGrid));
+return page.ReformatOnSave;
+}
+
private string GetDocumentParent(IWpfTextView view)
{
ITextDocument document;
@@ -355,5 +392,78 @@
}
return null;
}
+
+public int OnAfterFirstDocumentLock(uint docCookie, uint dwRDTLockType,
+uint dwReadLocksRemaining, uint dwEditLocksRemainin
Re: [PATCH] D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk
berenm added inline comments. Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:195 @@ -179,1 +194,3 @@ +runningDocumentTable = new RunningDocumentTable(this); +adviseCookie = runningDocumentTable.Advise(this); In this blog post, they go through another interface to modify the file contents. I wanted to re-use the FormatTextBuffer method, that was operating on an ITextBuffer, and getting this ITextBuffer from a IDontKnowWhatDocument requires to go through this ComponentModel thing. I did some small changes and some new quick testing and I think everything now works on VS2012 and later. I haven't tested the plugin on VS2010 though. Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:441 @@ +440,3 @@ +var documentInfo = runningDocumentTable.GetDocumentInfo(docCookie); +if (documentInfo.DocData == null) +return VSConstants.S_OK; Sorry I dropped this part :'( http://reviews.llvm.org/D12407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk
berenm marked 8 inline comments as done. berenm added a comment. For information, I just tested the current diff under Visual Studio 2010 SP1 and everything looks fine as well. http://reviews.llvm.org/D12407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk
berenm added a comment. Actually, the main issue that I see is the difficulty to build the plugin itself. The project does not build out-of-the-box on VS2013, and I had to change the assembly references by hand. I have a patch to get the required assemblies in a more portable way using nuget packages, but I didn't want to include that in this revision. I'll open a separate one if that's useful. http://reviews.llvm.org/D12407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
