Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[Alexander Kornienko via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL272993: [clang-tidy] readability-identifier-naming - Support 
for Macros (authored by alexfh).

Changed prior to commit:
  http://reviews.llvm.org/D21020?vs=61071=61074#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D21020

Files:
  clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp

Index: clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -12,11 +12,53 @@
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Format.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/DenseMapInfo.h"
 
 #define DEBUG_TYPE "clang-tidy"
 
 using namespace clang::ast_matchers;
 
+namespace llvm {
+/// Specialisation of DenseMapInfo to allow NamingCheckId objects in DenseMaps
+template <>
+struct DenseMapInfo<
+clang::tidy::readability::IdentifierNamingCheck::NamingCheckId> {
+  using NamingCheckId =
+  clang::tidy::readability::IdentifierNamingCheck::NamingCheckId;
+
+  static inline NamingCheckId getEmptyKey() {
+return NamingCheckId(
+clang::SourceLocation::getFromRawEncoding(static_cast(-1)),
+"EMPTY");
+  }
+
+  static inline NamingCheckId getTombstoneKey() {
+return NamingCheckId(
+clang::SourceLocation::getFromRawEncoding(static_cast(-2)),
+"TOMBSTONE");
+  }
+
+  static unsigned getHashValue(NamingCheckId Val) {
+assert(Val != getEmptyKey() && "Cannot hash the empty key!");
+assert(Val != getTombstoneKey() && "Cannot hash the tombstone key!");
+
+std::hash SecondHash;
+return Val.first.getRawEncoding() + SecondHash(Val.second);
+  }
+
+  static bool isEqual(NamingCheckId LHS, NamingCheckId RHS) {
+if (RHS == getEmptyKey())
+  return LHS == getEmptyKey();
+if (RHS == getTombstoneKey())
+  return LHS == getTombstoneKey();
+return LHS == RHS;
+  }
+};
+} // namespace llvm
+
 namespace clang {
 namespace tidy {
 namespace readability {
@@ -66,6 +108,7 @@
 m(TemplateTemplateParameter) \
 m(TemplateParameter) \
 m(TypeAlias) \
+m(MacroDefinition) \
 
 enum StyleKind {
 #define ENUMERATE(v) SK_ ## v,
@@ -84,6 +127,33 @@
 #undef NAMING_KEYS
 // clang-format on
 
+namespace {
+/// Callback supplies macros to IdentifierNamingCheck::checkMacro
+class IdentifierNamingCheckPPCallbacks : public PPCallbacks {
+public:
+  IdentifierNamingCheckPPCallbacks(Preprocessor *PP,
+   IdentifierNamingCheck *Check)
+  : PP(PP), Check(Check) {}
+
+  /// MacroDefined calls checkMacro for macros in the main file
+  void MacroDefined(const Token ,
+const MacroDirective *MD) override {
+Check->checkMacro(PP->getSourceManager(), MacroNameTok, MD->getMacroInfo());
+  }
+
+  /// MacroExpands calls expandMacro for macros in the main file
+  void MacroExpands(const Token , const MacroDefinition ,
+SourceRange /*Range*/,
+const MacroArgs * /*Args*/) override {
+Check->expandMacro(MacroNameTok, MD.getMacroInfo());
+  }
+
+private:
+  Preprocessor *PP;
+  IdentifierNamingCheck *Check;
+};
+} // namespace
+
 IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context) {
@@ -146,6 +216,12 @@
   Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"), this);
 }
 
+void IdentifierNamingCheck::registerPPCallbacks(CompilerInstance ) {
+  Compiler.getPreprocessor().addPPCallbacks(
+  llvm::make_unique(
+  (), this));
+}
+
 static bool matchesStyle(StringRef Name,
  IdentifierNamingCheck::NamingStyle Style) {
   static llvm::Regex Matchers[] = {
@@ -506,8 +582,8 @@
 }
 
 static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap ,
- const NamedDecl *Decl, SourceRange Range,
- const SourceManager *SM) {
+ const IdentifierNamingCheck::NamingCheckId ,
+ SourceRange Range) {
   // Do nothing if the provided range is invalid.
   if (Range.getBegin().isInvalid() || Range.getEnd().isInvalid())
 return;
@@ -522,14 +598,22 @@
   !Range.getEnd().isMacroID();
 }
 
+/// Convenience method when the usage to be added is a NamedDecl
+static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap ,
+ 

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

In http://reviews.llvm.org/D21020#460734, @JamesReynolds wrote:

> Thanks! All fixed now. Could you land this for me please?


Sure, will do. Thank you for working on this!


http://reviews.llvm.org/D21020



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[James Reynolds via cfe-commits]

JamesReynolds marked an inline comment as done.
JamesReynolds added a comment.

Thanks! All fixed now. Could you land this for me please?


http://reviews.llvm.org/D21020



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[James Reynolds via cfe-commits]

JamesReynolds marked 2 inline comments as done.


Comment at: clang-tidy/readability/IdentifierNamingCheck.h:98
@@ +97,3 @@
+
+  /// Add a usage of a macro if it already has a violation.
+  void expandMacro(const Token , const MacroInfo *MI);

Fixed this one too.


http://reviews.llvm.org/D21020



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[James Reynolds via cfe-commits]

JamesReynolds updated this revision to Diff 61071.
JamesReynolds added a comment.

Fixed nits.


http://reviews.llvm.org/D21020

Files:
  clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tidy/readability/IdentifierNamingCheck.h
  docs/ReleaseNotes.rst
  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
@@ -61,6 +61,7 @@
 // RUN: {key: readability-identifier-naming.VariableCase, value: lower_case}, \
 // RUN: {key: readability-identifier-naming.VirtualMethodCase, value: UPPER_CASE}, \
 // RUN: {key: readability-identifier-naming.VirtualMethodPrefix, value: 'v_'}, \
+// RUN: {key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE}, \
 // RUN: {key: readability-identifier-naming.TypeAliasCase, value: lower_case}, \
 // RUN: {key: readability-identifier-naming.TypeAliasSuffix, value: '_t'}, \
 // RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0} \
@@ -307,6 +308,12 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for typedef 'struct_type'
 // CHECK-FIXES: {{^}}typedef this_structure struct_type_t;{{$}}
 
+struct_type GlobalTypedefTestFunction(struct_type a_argument1) {
+// CHECK-FIXES: {{^}}struct_type_t GlobalTypedefTestFunction(struct_type_t a_argument1) {
+struct_type typedef_test_1;
+// CHECK-FIXES: {{^}}struct_type_t typedef_test_1;
+}
+
 using my_struct_type = THIS___Structure;
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for type alias 'my_struct_type'
 // CHECK-FIXES: {{^}}using my_struct_type_t = this_structure;{{$}}
@@ -329,5 +336,11 @@
 // CHECK-FIXES: {{^}}  using ::foo_ns::inline_namespace::ce_function;{{$}}
 }
 
+#define MY_TEST_Macro(X) X()
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for macro definition 'MY_TEST_Macro'
+// CHECK-FIXES: {{^}}#define MY_TEST_MACRO(X) X()
+
+void MY_TEST_Macro(function) {}
+// CHECK-FIXES: {{^}}void MY_TEST_MACRO(function) {}
 }
 }
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -249,6 +249,11 @@
   Warns about defaulted constructors and assignment operators that are actually
   deleted.
 
+- Updated `readability-identifier-naming-check
+  `_
+
+  Added support for enforcing the case of macro statements.
+
 - New `readability-redundant-control-flow
   `_ check
 
Index: clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tidy/readability/IdentifierNamingCheck.h
@@ -13,6 +13,9 @@
 #include "../ClangTidy.h"
 
 namespace clang {
+
+class MacroInfo;
+
 namespace tidy {
 namespace readability {
 
@@ -36,6 +39,7 @@
   void storeOptions(ClangTidyOptions::OptionMap ) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void registerPPCallbacks(CompilerInstance ) override;
   void onEndOfTranslationUnit() override;
 
   enum CaseType {
@@ -64,7 +68,7 @@
 
   /// \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.
+  /// identifier usages.
   struct NamingCheckFailure {
 std::string KindName;
 std::string Fixup;
@@ -81,9 +85,19 @@
 
 NamingCheckFailure() : ShouldFix(true) {}
   };
-  typedef llvm::DenseMap
+
+  typedef std::pair NamingCheckId;
+
+  typedef llvm::DenseMap
   NamingCheckFailureMap;
 
+  /// Check Macros for style violations.
+  void checkMacro(SourceManager , const Token ,
+  const MacroInfo *MI);
+
+  /// Add a usage of a macro if it already has a violation.
+  void expandMacro(const Token , const MacroInfo *MI);
+
 private:
   std::vector NamingStyles;
   bool IgnoreFailedSplit;
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -12,11 +12,53 @@
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Format.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/DenseMapInfo.h"
 
 #define DEBUG_TYPE "clang-tidy"
 
 using namespace clang::ast_matchers;
 
+namespace llvm {
+/// 

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[Alexander Kornienko via cfe-commits]

alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

Looks good with a couple of nits.



Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:808
@@ +807,3 @@
+  addUsage(NamingCheckFailures, ID, Range);
+  return;
+}

`return`?


Comment at: clang-tidy/readability/IdentifierNamingCheck.h:94
@@ -86,1 +93,3 @@
 
+  /// Check Macros for style violations
+  void checkMacro(SourceManager , const Token ,

Please add trailing periods to comments. See also 
http://llvm.org/docs/CodingStandards.html#commenting


http://reviews.llvm.org/D21020



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[James Reynolds via cfe-commits]

JamesReynolds marked an inline comment as done.
JamesReynolds added a comment.

http://reviews.llvm.org/D21020



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[James Reynolds via cfe-commits]

JamesReynolds updated this revision to Diff 60058.
JamesReynolds marked an inline comment as not done.
JamesReynolds added a comment.

Missed one modification in last update.


http://reviews.llvm.org/D21020

Files:
  clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tidy/readability/IdentifierNamingCheck.h
  docs/ReleaseNotes.rst
  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
@@ -61,6 +61,7 @@
 // RUN: {key: readability-identifier-naming.VariableCase, value: lower_case}, \
 // RUN: {key: readability-identifier-naming.VirtualMethodCase, value: UPPER_CASE}, \
 // RUN: {key: readability-identifier-naming.VirtualMethodPrefix, value: 'v_'}, \
+// RUN: {key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE}, \
 // RUN: {key: readability-identifier-naming.TypeAliasCase, value: lower_case}, \
 // RUN: {key: readability-identifier-naming.TypeAliasSuffix, value: '_t'}, \
 // RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0} \
@@ -307,6 +308,12 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for typedef 'struct_type'
 // CHECK-FIXES: {{^}}typedef this_structure struct_type_t;{{$}}
 
+struct_type GlobalTypedefTestFunction(struct_type a_argument1) {
+// CHECK-FIXES: {{^}}struct_type_t GlobalTypedefTestFunction(struct_type_t a_argument1) {
+struct_type typedef_test_1;
+// CHECK-FIXES: {{^}}struct_type_t typedef_test_1;
+}
+
 using my_struct_type = THIS___Structure;
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for type alias 'my_struct_type'
 // CHECK-FIXES: {{^}}using my_struct_type_t = this_structure;{{$}}
@@ -329,5 +336,11 @@
 // CHECK-FIXES: {{^}}  using ::foo_ns::inline_namespace::ce_function;{{$}}
 }
 
+#define MY_TEST_Macro(X) X()
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for macro definition 'MY_TEST_Macro'
+// CHECK-FIXES: {{^}}#define MY_TEST_MACRO(X) X()
+
+void MY_TEST_Macro(function) {}
+// CHECK-FIXES: {{^}}void MY_TEST_MACRO(function) {}
 }
 }
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -249,6 +249,11 @@
   Warns about defaulted constructors and assignment operators that are actually
   deleted.
 
+- Updated `readability-identifier-naming-check
+  `_
+
+  Added support for enforcing the case of macro statements.
+
 - New `readability-redundant-control-flow
   `_ check
 
Index: clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tidy/readability/IdentifierNamingCheck.h
@@ -13,6 +13,9 @@
 #include "../ClangTidy.h"
 
 namespace clang {
+
+class MacroInfo;
+
 namespace tidy {
 namespace readability {
 
@@ -36,6 +39,7 @@
   void storeOptions(ClangTidyOptions::OptionMap ) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void registerPPCallbacks(CompilerInstance ) override;
   void onEndOfTranslationUnit() override;
 
   enum CaseType {
@@ -64,7 +68,7 @@
 
   /// \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.
+  /// identifier usages.
   struct NamingCheckFailure {
 std::string KindName;
 std::string Fixup;
@@ -81,9 +85,19 @@
 
 NamingCheckFailure() : ShouldFix(true) {}
   };
-  typedef llvm::DenseMap
+
+  typedef std::pair NamingCheckId;
+
+  typedef llvm::DenseMap
   NamingCheckFailureMap;
 
+  /// Check Macros for style violations
+  void checkMacro(SourceManager , const Token ,
+  const MacroInfo *MI);
+
+  /// Add a usage of a macro if it already has a violation
+  void expandMacro(const Token , const MacroInfo *MI);
+
 private:
   std::vector NamingStyles;
   bool IgnoreFailedSplit;
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -12,11 +12,53 @@
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Format.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/DenseMapInfo.h"
 
 #define DEBUG_TYPE 

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[James Reynolds via cfe-commits]

JamesReynolds marked an inline comment as done.
JamesReynolds added a comment.

http://reviews.llvm.org/D21020



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[James Reynolds via cfe-commits]

JamesReynolds added inline comments.


Comment at: clang-tidy/readability/IdentifierNamingCheck.h:89
@@ +88,3 @@
+
+  typedef std::pair NamingCheckId;
+

I think I thought that this wouldn't get picked up by the specialization 
machinery - but you're right, a typedef works out fine! Maybe I was thinking of 
overloads...


http://reviews.llvm.org/D21020



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[James Reynolds via cfe-commits]

JamesReynolds updated this revision to Diff 60056.
JamesReynolds marked 5 inline comments as done.
JamesReynolds added a comment.

Applied suggested code changes.


http://reviews.llvm.org/D21020

Files:
  clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tidy/readability/IdentifierNamingCheck.h
  docs/ReleaseNotes.rst
  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
@@ -61,6 +61,7 @@
 // RUN: {key: readability-identifier-naming.VariableCase, value: lower_case}, \
 // RUN: {key: readability-identifier-naming.VirtualMethodCase, value: UPPER_CASE}, \
 // RUN: {key: readability-identifier-naming.VirtualMethodPrefix, value: 'v_'}, \
+// RUN: {key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE}, \
 // RUN: {key: readability-identifier-naming.TypeAliasCase, value: lower_case}, \
 // RUN: {key: readability-identifier-naming.TypeAliasSuffix, value: '_t'}, \
 // RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0} \
@@ -307,6 +308,12 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for typedef 'struct_type'
 // CHECK-FIXES: {{^}}typedef this_structure struct_type_t;{{$}}
 
+struct_type GlobalTypedefTestFunction(struct_type a_argument1) {
+// CHECK-FIXES: {{^}}struct_type_t GlobalTypedefTestFunction(struct_type_t a_argument1) {
+struct_type typedef_test_1;
+// CHECK-FIXES: {{^}}struct_type_t typedef_test_1;
+}
+
 using my_struct_type = THIS___Structure;
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for type alias 'my_struct_type'
 // CHECK-FIXES: {{^}}using my_struct_type_t = this_structure;{{$}}
@@ -329,5 +336,11 @@
 // CHECK-FIXES: {{^}}  using ::foo_ns::inline_namespace::ce_function;{{$}}
 }
 
+#define MY_TEST_Macro(X) X()
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for macro definition 'MY_TEST_Macro'
+// CHECK-FIXES: {{^}}#define MY_TEST_MACRO(X) X()
+
+void MY_TEST_Macro(function) {}
+// CHECK-FIXES: {{^}}void MY_TEST_MACRO(function) {}
 }
 }
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -249,6 +249,11 @@
   Warns about defaulted constructors and assignment operators that are actually
   deleted.
 
+- Updated `readability-identifier-naming-check
+  `_
+
+  Added support for enforcing the case of macro statements.
+
 - New `readability-redundant-control-flow
   `_ check
 
Index: clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tidy/readability/IdentifierNamingCheck.h
@@ -13,6 +13,9 @@
 #include "../ClangTidy.h"
 
 namespace clang {
+
+class MacroInfo;
+
 namespace tidy {
 namespace readability {
 
@@ -36,6 +39,7 @@
   void storeOptions(ClangTidyOptions::OptionMap ) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void registerPPCallbacks(CompilerInstance ) override;
   void onEndOfTranslationUnit() override;
 
   enum CaseType {
@@ -64,7 +68,7 @@
 
   /// \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.
+  /// identifier usages.
   struct NamingCheckFailure {
 std::string KindName;
 std::string Fixup;
@@ -81,9 +85,19 @@
 
 NamingCheckFailure() : ShouldFix(true) {}
   };
-  typedef llvm::DenseMap
+
+  typedef std::pair NamingCheckId;
+
+  typedef llvm::DenseMap
   NamingCheckFailureMap;
 
+  /// Check Macros for style violations
+  void checkMacro(SourceManager , const Token ,
+  const MacroInfo *MI);
+
+  /// Add a usage of a macro if it already has a violation
+  void expandMacro(const Token , const MacroInfo *MI);
+
 private:
   std::vector NamingStyles;
   bool IgnoreFailedSplit;
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -12,11 +12,53 @@
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Format.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/DenseMapInfo.h"
 
 #define DEBUG_TYPE "clang-tidy"
 
 

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:142
@@ +141,3 @@
+SourceManager (PP->getSourceManager());
+if (!SM.isInMainFile(MacroNameTok.getLocation()))
+  return;

I'm not sure the check currently refuses to fix stuff defined in headers. It 
probably relies on -header-filter to limit its scope. It's intrinsically 
dangerous (since we should see all translation units using the entity to 
correctly rename it), but we should let users do this, in case they know what 
to do.


Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:152
@@ +151,3 @@
+SourceRange Range, const MacroArgs *Args) override {
+(void)Range;
+(void)Args;

No need to mute -Wunused _this_ way. Just comment out parameter names in the 
declaration.


Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:814
@@ +813,3 @@
+  SourceRange Range =
+  SourceRange(MacroNameTok.getLocation(), MacroNameTok.getEndLoc());
+  addUsage(NamingCheckFailures, ID, Range);

Remove ` = SourceRange`. Just `SourceRange Range(...);` is enough.


Comment at: clang-tidy/readability/IdentifierNamingCheck.h:86
@@ +85,3 @@
+
+  struct NamingCheckId : std::pair {
+typedef std::pair Parent;

Maybe just typedef?


Comment at: clang-tidy/readability/IdentifierNamingCheck.h:88
@@ +87,3 @@
+typedef std::pair Parent;
+using Parent::Parent;
+  };

Delegating constructors don't work in VS2013, which LLVM should still support.


http://reviews.llvm.org/D21020



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[James Reynolds via cfe-commits]

JamesReynolds updated this revision to Diff 60010.
JamesReynolds added a comment.

Changed the clang-tidy release notes addition to be in alphabetical order.


http://reviews.llvm.org/D21020

Files:
  clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tidy/readability/IdentifierNamingCheck.h
  docs/ReleaseNotes.rst
  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
@@ -61,6 +61,7 @@
 // RUN: {key: readability-identifier-naming.VariableCase, value: lower_case}, \
 // RUN: {key: readability-identifier-naming.VirtualMethodCase, value: UPPER_CASE}, \
 // RUN: {key: readability-identifier-naming.VirtualMethodPrefix, value: 'v_'}, \
+// RUN: {key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE}, \
 // RUN: {key: readability-identifier-naming.TypeAliasCase, value: lower_case}, \
 // RUN: {key: readability-identifier-naming.TypeAliasSuffix, value: '_t'}, \
 // RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0} \
@@ -307,6 +308,12 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for typedef 'struct_type'
 // CHECK-FIXES: {{^}}typedef this_structure struct_type_t;{{$}}
 
+struct_type GlobalTypedefTestFunction(struct_type a_argument1) {
+// CHECK-FIXES: {{^}}struct_type_t GlobalTypedefTestFunction(struct_type_t a_argument1) {
+struct_type typedef_test_1;
+// CHECK-FIXES: {{^}}struct_type_t typedef_test_1;
+}
+
 using my_struct_type = THIS___Structure;
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for type alias 'my_struct_type'
 // CHECK-FIXES: {{^}}using my_struct_type_t = this_structure;{{$}}
@@ -329,5 +336,11 @@
 // CHECK-FIXES: {{^}}  using ::foo_ns::inline_namespace::ce_function;{{$}}
 }
 
+#define MY_TEST_Macro(X) X()
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for macro definition 'MY_TEST_Macro'
+// CHECK-FIXES: {{^}}#define MY_TEST_MACRO(X) X()
+
+void MY_TEST_Macro(function) {}
+// CHECK-FIXES: {{^}}void MY_TEST_MACRO(function) {}
 }
 }
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -249,6 +249,11 @@
   Warns about defaulted constructors and assignment operators that are actually
   deleted.
 
+- Updated `readability-identifier-naming-check
+  `_
+
+  Added support for enforcing the case of macro statements.
+
 - New `readability-redundant-control-flow
   `_ check
 
Index: clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tidy/readability/IdentifierNamingCheck.h
@@ -13,6 +13,9 @@
 #include "../ClangTidy.h"
 
 namespace clang {
+
+class MacroInfo;
+
 namespace tidy {
 namespace readability {
 
@@ -36,6 +39,7 @@
   void storeOptions(ClangTidyOptions::OptionMap ) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void registerPPCallbacks(CompilerInstance ) override;
   void onEndOfTranslationUnit() override;
 
   enum CaseType {
@@ -64,7 +68,7 @@
 
   /// \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.
+  /// identifier usages.
   struct NamingCheckFailure {
 std::string KindName;
 std::string Fixup;
@@ -81,9 +85,22 @@
 
 NamingCheckFailure() : ShouldFix(true) {}
   };
-  typedef llvm::DenseMap
+
+  struct NamingCheckId : std::pair {
+typedef std::pair Parent;
+using Parent::Parent;
+  };
+
+  typedef llvm::DenseMap
   NamingCheckFailureMap;
 
+  /// Check Macros for style violations
+  void checkMacro(SourceManager , const Token ,
+  const MacroInfo *MI);
+
+  /// Add a usage of a macro if it already has a violation
+  void expandMacro(const Token , const MacroInfo *MI);
+
 private:
   std::vector NamingStyles;
   bool IgnoreFailedSplit;
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -12,11 +12,53 @@
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Format.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/PPCallbacks.h"
+#include 

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[James Reynolds via cfe-commits]

JamesReynolds marked an inline comment as done.
JamesReynolds added a comment.

http://reviews.llvm.org/D21020



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added inline comments.


Comment at: docs/ReleaseNotes.rst:262
@@ -261,1 +261,3 @@
 
+- Updated `readability-identifier-naming-check
+  
`_

Please put this not in alphabetical order. I just fixed misleading order of 
misc-unconventional-assign-operator.


http://reviews.llvm.org/D21020



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[James Reynolds via cfe-commits]

JamesReynolds updated this revision to Diff 59857.
JamesReynolds added a comment.

Thanks Eugene, I've added a comment into docs/ReleaseNotes.rst to say this is 
in. I'll create a BugZilla account to update PR.

I've also added a new test and code to create FixIts for uses of the Macros as 
well as the definitions themselves.


http://reviews.llvm.org/D21020

Files:
  clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tidy/readability/IdentifierNamingCheck.h
  docs/ReleaseNotes.rst
  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
@@ -61,6 +61,7 @@
 // RUN: {key: readability-identifier-naming.VariableCase, value: lower_case}, \
 // RUN: {key: readability-identifier-naming.VirtualMethodCase, value: UPPER_CASE}, \
 // RUN: {key: readability-identifier-naming.VirtualMethodPrefix, value: 'v_'}, \
+// RUN: {key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE}, \
 // RUN: {key: readability-identifier-naming.TypeAliasCase, value: lower_case}, \
 // RUN: {key: readability-identifier-naming.TypeAliasSuffix, value: '_t'}, \
 // RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0} \
@@ -307,6 +308,12 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for typedef 'struct_type'
 // CHECK-FIXES: {{^}}typedef this_structure struct_type_t;{{$}}
 
+struct_type GlobalTypedefTestFunction(struct_type a_argument1) {
+// CHECK-FIXES: {{^}}struct_type_t GlobalTypedefTestFunction(struct_type_t a_argument1) {
+struct_type typedef_test_1;
+// CHECK-FIXES: {{^}}struct_type_t typedef_test_1;
+}
+
 using my_struct_type = THIS___Structure;
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for type alias 'my_struct_type'
 // CHECK-FIXES: {{^}}using my_struct_type_t = this_structure;{{$}}
@@ -329,5 +336,11 @@
 // CHECK-FIXES: {{^}}  using ::foo_ns::inline_namespace::ce_function;{{$}}
 }
 
+#define MY_TEST_Macro(X) X()
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for macro definition 'MY_TEST_Macro'
+// CHECK-FIXES: {{^}}#define MY_TEST_MACRO(X) X()
+
+void MY_TEST_Macro(function) {}
+// CHECK-FIXES: {{^}}void MY_TEST_MACRO(function) {}
 }
 }
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -259,6 +259,11 @@
   Does not only checks for correct signature but also for correct ``return``
   statements (returning ``*this``)
 
+- Updated `readability-identifier-naming-check
+  `_
+
+  Added support for enforcing the case of macro statements.
+
 Fixed bugs:
 
 - Crash when running on compile database with relative source files paths.
Index: clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tidy/readability/IdentifierNamingCheck.h
@@ -13,6 +13,9 @@
 #include "../ClangTidy.h"
 
 namespace clang {
+
+class MacroInfo;
+
 namespace tidy {
 namespace readability {
 
@@ -36,6 +39,7 @@
   void storeOptions(ClangTidyOptions::OptionMap ) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void registerPPCallbacks(CompilerInstance ) override;
   void onEndOfTranslationUnit() override;
 
   enum CaseType {
@@ -64,7 +68,7 @@
 
   /// \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.
+  /// identifier usages.
   struct NamingCheckFailure {
 std::string KindName;
 std::string Fixup;
@@ -81,9 +85,22 @@
 
 NamingCheckFailure() : ShouldFix(true) {}
   };
-  typedef llvm::DenseMap
+
+  struct NamingCheckId : std::pair {
+typedef std::pair Parent;
+using Parent::Parent;
+  };
+
+  typedef llvm::DenseMap
   NamingCheckFailureMap;
 
+  /// Check Macros for style violations
+  void checkMacro(SourceManager , const Token ,
+  const MacroInfo *MI);
+
+  /// Add a usage of a macro if it already has a violation
+  void expandMacro(const Token , const MacroInfo *MI);
+
 private:
   std::vector NamingStyles;
   bool IgnoreFailedSplit;
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -12,11 +12,53 @@
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Format.h"
 #include 

Re: [PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko added a comment.

It'll be good idea to mention improvement in  docs/ReleaseNotes.rst.

Please close PR23198 after commit.


http://reviews.llvm.org/D21020



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[James Reynolds via cfe-commits]

JamesReynolds created this revision.
JamesReynolds added a reviewer: alexfh.
JamesReynolds added a subscriber: cfe-commits.

Added support for macro definitions.
--

1. Added a pre-processor callback to catch macro definitions
2. Changed the type of the failure map so that macros and declarations can 
share the same map
3. Added extra tests to ensure fix-ups work using the new map
4. Added fix-ups for type aliases in variable and function declarations as part 
of adding the new tests

http://reviews.llvm.org/D21020

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
@@ -61,6 +61,7 @@
 // RUN: {key: readability-identifier-naming.VariableCase, value: lower_case}, \
 // RUN: {key: readability-identifier-naming.VirtualMethodCase, value: UPPER_CASE}, \
 // RUN: {key: readability-identifier-naming.VirtualMethodPrefix, value: 'v_'}, \
+// RUN: {key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE}, \
 // RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0} \
 // RUN:   ]}' -- -std=c++11 -fno-delayed-template-parsing \
 // RUN:   -I%S/Inputs/readability-identifier-naming \
@@ -305,6 +306,12 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for typedef 'struct_type'
 // CHECK-FIXES: {{^}}typedef this_structure struct_type_t;{{$}}
 
+struct_type GlobalTypedefTestFunction(struct_type a_argument1) {
+// CHECK-FIXES: {{^}}struct_type_t GlobalTypedefTestFunction(struct_type_t a_argument1) {
+struct_type typedef_test_1;
+// CHECK-FIXES: {{^}}struct_type_t typedef_test_1;
+}
+
 static void static_Function() {
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for function 'static_Function'
 // CHECK-FIXES: {{^}}static void staticFunction() {{{$}}
@@ -318,5 +325,9 @@
 // CHECK-FIXES: {{^}}  using ::foo_ns::inline_namespace::ce_function;{{$}}
 }
 
+#define MY_TEST_Macro(X) X()
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for macro definition 'MY_TEST_Macro'
+// CHECK-FIXES: {{^}}#define MY_TEST_MACRO(X) X()
+
 }
 }
Index: clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tidy/readability/IdentifierNamingCheck.h
@@ -36,6 +36,7 @@
   void storeOptions(ClangTidyOptions::OptionMap ) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void registerPPCallbacks(CompilerInstance ) override;
   void onEndOfTranslationUnit() override;
 
   enum CaseType {
@@ -64,7 +65,7 @@
 
   /// \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.
+  /// identifier usages.
   struct NamingCheckFailure {
 std::string KindName;
 std::string Fixup;
@@ -81,9 +82,17 @@
 
 NamingCheckFailure() : ShouldFix(true) {}
   };
-  typedef llvm::DenseMap
+
+  struct NamingCheckId : std::pair {
+typedef std::pair Parent;
+using Parent::Parent;
+  };
+
+  typedef llvm::DenseMap
   NamingCheckFailureMap;
 
+  void checkMacro(SourceManager , const Token );
+
 private:
   std::vector NamingStyles;
   bool IgnoreFailedSplit;
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -12,11 +12,53 @@
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Format.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/DenseMapInfo.h"
 
 #define DEBUG_TYPE "clang-tidy"
 
 using namespace clang::ast_matchers;
 
+namespace llvm {
+/// Specialisation of DenseMapInfo to allow NamingCheckId objects in DenseMaps
+template <>
+struct DenseMapInfo<
+clang::tidy::readability::IdentifierNamingCheck::NamingCheckId> {
+  using NamingCheckId =
+  clang::tidy::readability::IdentifierNamingCheck::NamingCheckId;
+
+  static inline NamingCheckId getEmptyKey() {
+return NamingCheckId(
+clang::SourceLocation::getFromRawEncoding(static_cast(-1)),
+"EMPTY");
+  }
+
+  static inline NamingCheckId getTombstoneKey() {
+return NamingCheckId(
+clang::SourceLocation::getFromRawEncoding(static_cast(-2)),
+"TOMBSTONE");
+  }
+
+  static