[PATCH] D41716: clang-tidy: add IgnoreMacros option to readability-inconsistent-declaration-parameter-name

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos created this revision.
vmiklos added reviewers: alexfh, piotrdz.

  And also enable it by default to be consistent with e.g. modernize-use-using.
  
  This helps e.g. when running this check on client code where the macro is
  provided by the system, so there is no easy way to modify it.


https://reviews.llvm.org/D41716

Files:
  clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
  clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
  docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
  test/clang-tidy/readability-inconsistent-declaration-parameter-name-macros.cpp
  test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp

Index: test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
===
--- test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
+++ test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
@@ -178,11 +178,14 @@
 
 //
 
-#define DECLARE_FUNCTION_WITH_PARAM_NAME(function_name, param_name) \
-  void function_name(int param_name)
-
-// CHECK-MESSAGES: :[[@LINE+1]]:34: warning: function 'macroFunction' has 1 other declaration with different parameter names [readability-inconsistent-declaration-parameter-name]
-DECLARE_FUNCTION_WITH_PARAM_NAME(macroFunction, a);
-// CHECK-MESSAGES: :[[@LINE+2]]:34: note: the 1st inconsistent declaration seen here
-// CHECK-MESSAGES: :[[@LINE+1]]:34: note: differing parameters are named here: ('b'), in the other declaration: ('a')
-DECLARE_FUNCTION_WITH_PARAM_NAME(macroFunction, b);
+// This resulted in a warning by default.
+#define MACRO() \
+  void f(int x);
+
+struct S {
+  MACRO();
+};
+
+void S::f(int y)
+{
+}
Index: test/clang-tidy/readability-inconsistent-declaration-parameter-name-macros.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-inconsistent-declaration-parameter-name-macros.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s readability-inconsistent-declaration-parameter-name %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-inconsistent-declaration-parameter-name.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -std=c++11
+
+#define MACRO() \
+  void f(int x);
+
+struct S {
+  MACRO();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'S::f' has a definition with different parameter names
+};
+
+void S::f(int y) {
+}
+
+//
+
+#define DECLARE_FUNCTION_WITH_PARAM_NAME(function_name, param_name) \
+  void function_name(int param_name)
+
+// CHECK-MESSAGES: :[[@LINE+1]]:34: warning: function 'macroFunction' has 1 other declaration with different parameter names [readability-inconsistent-declaration-parameter-name]
+DECLARE_FUNCTION_WITH_PARAM_NAME(macroFunction, a);
+// CHECK-MESSAGES: :[[@LINE+2]]:34: note: the 1st inconsistent declaration seen here
+// CHECK-MESSAGES: :[[@LINE+1]]:34: note: differing parameters are named here: ('b'), in the other declaration: ('a')
+DECLARE_FUNCTION_WITH_PARAM_NAME(macroFunction, b);
Index: docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
===
--- docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
+++ docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
@@ -42,3 +42,8 @@
 In the case of multiple redeclarations or function template specializations,
 a warning is issued for every redeclaration or specialization inconsistent with
 the definition or the first declaration seen in a translation unit.
+
+.. option:: IgnoreMacros
+
+   If this option is set to non-zero (default is `1`), the check will not warn
+   about names declared inside macros.
Index: clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
===
--- clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
+++ clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
@@ -27,15 +27,18 @@
 public:
   InconsistentDeclarationParameterNameCheck(StringRef Name,
 ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
 
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
   void markRedeclarationsAsVisited(const FunctionDecl *FunctionDeclaration);
 
   llvm::DenseSet VisitedDeclarations;
+  const bool IgnoreMacros;
 };
 
 } // namespace readability
Index: clang-tidy/readability/InconsistentDeclarationParameterN

[PATCH] D41716: clang-tidy: add IgnoreMacros option to readability-inconsistent-declaration-parameter-name

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

In https://reviews.llvm.org/D41716#967237, @lebedev.ri wrote:

> That changes the defaults though. I thought clang-tidy *tried* to produce the 
> same results
>  on different clang-tidy versions with the same `.clang-tidy` config? Or is 
> there no such guarantees?


Hmm, I see this as a trade-off between backwards compatibility and consistency. 
Here I went for consistency, so that all of 
readability-inconsistent-declaration-parameter-name, 
modernize-use-default-member-init and modernize-use-using default to 
IgnoreMacros=1. But I don't have a too strong opinion on that, can change the 
default to 0 if really wanted,


https://reviews.llvm.org/D41716



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

[PATCH] D41716: clang-tidy: add IgnoreMacros option to readability-inconsistent-declaration-parameter-name

[Miklos Vajna via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL321913: clang-tidy: add IgnoreMacros option to 
readability-inconsistent-declaration… (authored by vmiklos, committed by ).
Herald added a subscriber: klimek.

Changed prior to commit:
  https://reviews.llvm.org/D41716?vs=128562&id=128818#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D41716

Files:
  
clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
  
clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
  
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
  
clang-tools-extra/trunk/test/clang-tidy/readability-inconsistent-declaration-parameter-name-macros.cpp
  
clang-tools-extra/trunk/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp

Index: clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
+++ clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
@@ -27,15 +27,18 @@
 public:
   InconsistentDeclarationParameterNameCheck(StringRef Name,
 ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
 
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
   void markRedeclarationsAsVisited(const FunctionDecl *FunctionDeclaration);
 
   llvm::DenseSet VisitedDeclarations;
+  const bool IgnoreMacros;
 };
 
 } // namespace readability
Index: clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
@@ -281,6 +281,11 @@
 
 } // anonymous namespace
 
+void InconsistentDeclarationParameterNameCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+}
+
 void InconsistentDeclarationParameterNameCheck::registerMatchers(
 MatchFinder *Finder) {
   Finder->addMatcher(functionDecl(unless(isImplicit()), hasOtherDeclarations())
@@ -309,6 +314,12 @@
 return;
   }
 
+  SourceLocation StartLoc = OriginalDeclaration->getLocStart();
+  if (StartLoc.isMacroID() && IgnoreMacros) {
+markRedeclarationsAsVisited(OriginalDeclaration);
+return;
+  }
+
   if (OriginalDeclaration->getTemplatedKind() ==
   FunctionDecl::TK_FunctionTemplateSpecialization) {
 formatDiagnostics(this, ParameterSourceDeclaration, OriginalDeclaration,
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
@@ -42,3 +42,8 @@
 In the case of multiple redeclarations or function template specializations,
 a warning is issued for every redeclaration or specialization inconsistent with
 the definition or the first declaration seen in a translation unit.
+
+.. option:: IgnoreMacros
+
+   If this option is set to non-zero (default is `1`), the check will not warn
+   about names declared inside macros.
Index: clang-tools-extra/trunk/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
@@ -178,11 +178,14 @@
 
 //
 
-#define DECLARE_FUNCTION_WITH_PARAM_NAME(function_name, param_name) \
-  void function_name(int param_name)
+// This resulted in a warning by default.
+#define MACRO() \
+  void f(int x);
 
-// CHECK-MESSAGES: :[[@LINE+1]]:34: warning: function 'macroFunction' has 1 other declaration with different parameter names [readability-inconsistent-declaration-parameter-name]
-DECLARE_FUNCTION_WITH_PARAM_NAME(macroFunction, a);
-// CHECK-MESSAGES: :[[@LINE+2]]:34: note: the 1st inconsistent declaration seen here
-// CHECK-MESSAGES: :[[@LINE+1]]:34: note: differing parameters are named here: ('b'), in the o

[PATCH] D44366: run-clang-tidy: forward clang-tidy exit status

[Miklos Vajna via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE327854: run-clang-tidy: forward clang-tidy exit status 
(authored by vmiklos, committed by ).
Herald added a subscriber: cfe-commits.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44366

Files:
  clang-tidy/tool/run-clang-tidy.py
  test/clang-tidy/run-clang-tidy.cpp
  test/lit.cfg


Index: clang-tidy/tool/run-clang-tidy.py
===
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -153,16 +153,18 @@
   subprocess.call(invocation)
 
 
-def run_tidy(args, tmpdir, build_path, queue):
+def run_tidy(args, tmpdir, build_path, queue, failed_files):
   """Takes filenames out of queue and runs clang-tidy on them."""
   while True:
 name = queue.get()
 invocation = get_tidy_invocation(name, args.clang_tidy_binary, args.checks,
  tmpdir, build_path, args.header_filter,
  args.extra_arg, args.extra_arg_before,
  args.quiet, args.config)
 sys.stdout.write(' '.join(invocation) + '\n')
-subprocess.call(invocation)
+return_code = subprocess.call(invocation)
+if return_code != 0:
+  failed_files.append(name)
 queue.task_done()
 
 
@@ -255,12 +257,15 @@
   # Build up a big regexy filter from all command line arguments.
   file_name_re = re.compile('|'.join(args.files))
 
+  return_code = 0
   try:
 # Spin up a bunch of tidy-launching threads.
 task_queue = queue.Queue(max_task)
+# List of files with a non-zero return code.
+failed_files = []
 for _ in range(max_task):
   t = threading.Thread(target=run_tidy,
-   args=(args, tmpdir, build_path, task_queue))
+   args=(args, tmpdir, build_path, task_queue, 
failed_files))
   t.daemon = True
   t.start()
 
@@ -271,6 +276,8 @@
 
 # Wait for all threads to be done.
 task_queue.join()
+if len(failed_files):
+  return_code = 1
 
   except KeyboardInterrupt:
 # This is a sad hack. Unfortunately subprocess goes
@@ -280,7 +287,6 @@
   shutil.rmtree(tmpdir)
 os.kill(0, 9)
 
-  return_code = 0
   if args.export_fixes:
 print('Writing fixes to ' + args.export_fixes + ' ...')
 try:
Index: test/clang-tidy/run-clang-tidy.cpp
===
--- test/clang-tidy/run-clang-tidy.cpp
+++ test/clang-tidy/run-clang-tidy.cpp
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "[{\"directory\":\".\",\"command\":\"clang++ -c 
%/t/test.cpp\",\"file\":\"%/t/test.cpp\"}]" | sed -e 's/\\//g' > 
%t/compile_commands.json
+// RUN: echo "Checks: '-*,modernize-use-auto'" > %t/.clang-tidy
+// RUN: echo "WarningsAsErrors: '*'" >> %t/.clang-tidy
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: cd "%t"
+// RUN: not %run_clang_tidy "%t/test.cpp"
+
+int main()
+{
+  int* x = new int();
+  delete x;
+}
Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -129,6 +129,11 @@
 config.substitutions.append(
 ('%clang_tidy_diff',
  '%s %s' % (config.python_executable, clang_tidy_diff)) )
+run_clang_tidy = os.path.join(
+config.test_source_root, "..", "clang-tidy", "tool", 
"run-clang-tidy.py")
+config.substitutions.append(
+('%run_clang_tidy',
+ '%s %s' % (config.python_executable, run_clang_tidy)) )
 else:
 # exclude the clang-tidy test directory
 config.excludes.append('clang-tidy')


Index: clang-tidy/tool/run-clang-tidy.py
===
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -153,16 +153,18 @@
   subprocess.call(invocation)
 
 
-def run_tidy(args, tmpdir, build_path, queue):
+def run_tidy(args, tmpdir, build_path, queue, failed_files):
   """Takes filenames out of queue and runs clang-tidy on them."""
   while True:
 name = queue.get()
 invocation = get_tidy_invocation(name, args.clang_tidy_binary, args.checks,
  tmpdir, build_path, args.header_filter,
  args.extra_arg, args.extra_arg_before,
  args.quiet, args.config)
 sys.stdout.write(' '.join(invocation) + '\n')
-subprocess.call(invocation)
+return_code = subprocess.call(invocation)
+if return_code != 0:
+  failed_files.append(name)
 queue.task_done()
 
 
@@ -255,12 +257,15 @@
   # Build up a big regexy filter from all command line arguments.
   file_name_re = re.compile('|'.join(args.files))
 
+  return_code = 0
   try:
 # Spin up a bunch of tidy-launching threads.
 task_queue = queue.Queue(max_task)
+# List of files with a non-zero return code.
+failed_files = []
 for _ in r

[PATCH] D53217: clang-tidy: add IgnoreMacros option to modernize-use-equals-delete

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos created this revision.
vmiklos added reviewers: alexfh, malcolm.parsons.
Herald added a subscriber: cfe-commits.

And also enable it by default to be consistent with e.g. modernize-use-using.
This improves consistency inside the check itself as well: both checks are now 
disabled in macros by default.

This helps e.g. when running this check on client code where the macro is
provided by the system, so there is no easy way to modify it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53217

Files:
  clang-tidy/modernize/UseEqualsDeleteCheck.cpp
  clang-tidy/modernize/UseEqualsDeleteCheck.h
  docs/clang-tidy/checks/modernize-use-equals-delete.rst
  test/clang-tidy/modernize-use-equals-delete-macros.cpp
  test/clang-tidy/modernize-use-equals-delete.cpp


Index: test/clang-tidy/modernize-use-equals-delete.cpp
===
--- test/clang-tidy/modernize-use-equals-delete.cpp
+++ test/clang-tidy/modernize-use-equals-delete.cpp
@@ -185,3 +185,9 @@
   DISALLOW_COPY_AND_ASSIGN(ProtectedDeletedMacro2);
 };
 
+// This resulted in a warning by default.
+#define MACRO(type) void operator=(type const &)
+class C {
+private:
+  MACRO(C);
+};
Index: test/clang-tidy/modernize-use-equals-delete-macros.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-equals-delete-macros.cpp
@@ -0,0 +1,10 @@
+// RUN: %check_clang_tidy %s modernize-use-equals-delete %t -- \
+// RUN:   -config="{CheckOptions: [{key: 
modernize-use-equals-delete.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -std=c++11
+
+#define MACRO(type) void operator=(type const &)
+class C {
+private:
+  MACRO(C);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit 
calling of a special member function
+};
Index: docs/clang-tidy/checks/modernize-use-equals-delete.rst
===
--- docs/clang-tidy/checks/modernize-use-equals-delete.rst
+++ docs/clang-tidy/checks/modernize-use-equals-delete.rst
@@ -23,3 +23,8 @@
 A& operator=(const A&) = delete;
   };
 
+
+.. option:: IgnoreMacros
+
+   If this option is set to non-zero (default is `1`), the check will not warn
+   about functions declared inside macros.
Index: clang-tidy/modernize/UseEqualsDeleteCheck.h
===
--- clang-tidy/modernize/UseEqualsDeleteCheck.h
+++ clang-tidy/modernize/UseEqualsDeleteCheck.h
@@ -38,9 +38,13 @@
 class UseEqualsDeleteCheck : public ClangTidyCheck {
 public:
   UseEqualsDeleteCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+private:
+  const bool IgnoreMacros;
 };
 
 } // namespace modernize
Index: clang-tidy/modernize/UseEqualsDeleteCheck.cpp
===
--- clang-tidy/modernize/UseEqualsDeleteCheck.cpp
+++ clang-tidy/modernize/UseEqualsDeleteCheck.cpp
@@ -21,6 +21,10 @@
 static const char SpecialFunction[] = "SpecialFunction";
 static const char DeletedNotPublic[] = "DeletedNotPublic";
 
+void UseEqualsDeleteCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+}
+
 void UseEqualsDeleteCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus)
 return;
@@ -57,6 +61,8 @@
 SourceLocation EndLoc = Lexer::getLocForEndOfToken(
 Func->getEndLoc(), 0, *Result.SourceManager, getLangOpts());
 
+if (Func->getLocation().isMacroID() && IgnoreMacros)
+  return;
 // FIXME: Improve FixItHint to make the method public.
 diag(Func->getLocation(),
  "use '= delete' to prohibit calling of a special member function")
@@ -66,7 +72,7 @@
 // Ignore this warning in macros, since it's extremely noisy in code using
 // DISALLOW_COPY_AND_ASSIGN-style macros and there's no easy way to
 // automatically fix the warning when macros are in play.
-if (Func->getLocation().isMacroID())
+if (Func->getLocation().isMacroID() && IgnoreMacros)
   return;
 // FIXME: Add FixItHint to make the method public.
 diag(Func->getLocation(), "deleted member function should be public");


Index: test/clang-tidy/modernize-use-equals-delete.cpp
===
--- test/clang-tidy/modernize-use-equals-delete.cpp
+++ test/clang-tidy/modernize-use-equals-delete.cpp
@@ -185,3 +185,9 @@
   DISALLOW_COPY_AND_ASSIGN(ProtectedDeletedMacro2);
 };
 
+// This resulted in a warning by default.
+#define MACRO(type) void operator=(type const &)
+class C 

[PATCH] D53217: [clang-tidy] add IgnoreMacros option to modernize-use-equals-delete

[Miklos Vajna via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL30: [clang-tidy] add IgnoreMacros option to 
modernize-use-equals-delete (authored by vmiklos, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53217?vs=169490&id=169551#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53217

Files:
  clang-tools-extra/trunk/clang-tidy/modernize/UseEqualsDeleteCheck.cpp
  clang-tools-extra/trunk/clang-tidy/modernize/UseEqualsDeleteCheck.h
  clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-equals-delete.rst
  clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete-macros.cpp
  clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete.cpp


Index: clang-tools-extra/trunk/clang-tidy/modernize/UseEqualsDeleteCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/modernize/UseEqualsDeleteCheck.h
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseEqualsDeleteCheck.h
@@ -38,9 +38,14 @@
 class UseEqualsDeleteCheck : public ClangTidyCheck {
 public:
   UseEqualsDeleteCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const bool IgnoreMacros;
 };
 
 } // namespace modernize
Index: clang-tools-extra/trunk/clang-tidy/modernize/UseEqualsDeleteCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/UseEqualsDeleteCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseEqualsDeleteCheck.cpp
@@ -21,6 +21,10 @@
 static const char SpecialFunction[] = "SpecialFunction";
 static const char DeletedNotPublic[] = "DeletedNotPublic";
 
+void UseEqualsDeleteCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+}
+
 void UseEqualsDeleteCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus)
 return;
@@ -57,6 +61,8 @@
 SourceLocation EndLoc = Lexer::getLocForEndOfToken(
 Func->getEndLoc(), 0, *Result.SourceManager, getLangOpts());
 
+if (Func->getLocation().isMacroID() && IgnoreMacros)
+  return;
 // FIXME: Improve FixItHint to make the method public.
 diag(Func->getLocation(),
  "use '= delete' to prohibit calling of a special member function")
@@ -66,7 +72,7 @@
 // Ignore this warning in macros, since it's extremely noisy in code using
 // DISALLOW_COPY_AND_ASSIGN-style macros and there's no easy way to
 // automatically fix the warning when macros are in play.
-if (Func->getLocation().isMacroID())
+if (Func->getLocation().isMacroID() && IgnoreMacros)
   return;
 // FIXME: Add FixItHint to make the method public.
 diag(Func->getLocation(), "deleted member function should be public");
Index: 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-equals-delete.rst
===
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-equals-delete.rst
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-equals-delete.rst
@@ -23,3 +23,8 @@
 A& operator=(const A&) = delete;
   };
 
+
+.. option:: IgnoreMacros
+
+   If this option is set to non-zero (default is `1`), the check will not warn
+   about functions declared inside macros.
Index: clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete.cpp
@@ -185,3 +185,9 @@
   DISALLOW_COPY_AND_ASSIGN(ProtectedDeletedMacro2);
 };
 
+// This resulted in a warning by default.
+#define MACRO(type) void operator=(type const &)
+class C {
+private:
+  MACRO(C);
+};
Index: 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete-macros.cpp
===
--- 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete-macros.cpp
+++ 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-equals-delete-macros.cpp
@@ -0,0 +1,10 @@
+// RUN: %check_clang_tidy %s modernize-use-equals-delete %t -- \
+// RUN:   -config="{CheckOptions: [{key: 
modernize-use-equals-delete.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -std=c++11
+
+#define MACRO(type) void operator=(type const &)
+class C {
+private:
+  MACRO(C);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit 
calling of a special member function
+};


Index:

[PATCH] D53454: [clang-tidy] add IgnoreMacros option to eadability-redundant-smartptr-get

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos created this revision.
vmiklos added reviewers: alexfh, sbenza.
Herald added subscribers: cfe-commits, xazax.hun.

And also enable it by default to be consistent with e.g. modernize-use-using.

  

This helps e.g. when running this check on client code where the macro is
provided by the system, so there is no easy way to modify it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53454

Files:
  clang-tidy/readability/RedundantSmartptrGetCheck.cpp
  clang-tidy/readability/RedundantSmartptrGetCheck.h
  docs/clang-tidy/checks/readability-redundant-smartptr-get.rst
  test/clang-tidy/readability-redundant-smartptr-get-macros.cpp
  test/clang-tidy/readability-redundant-smartptr-get.cpp

Index: test/clang-tidy/readability-redundant-smartptr-get.cpp
===
--- test/clang-tidy/readability-redundant-smartptr-get.cpp
+++ test/clang-tidy/readability-redundant-smartptr-get.cpp
@@ -168,6 +168,8 @@
   // CHECK-FIXES: if (NULL == x);
 }
 
+#define MACRO(p) p.get()
+
 void Negative() {
   struct NegPtr {
 int* get();
@@ -193,4 +195,7 @@
   bool bb = ip.get() == nullptr;
   bb = !ip.get();
   bb = ip.get() ? true : false;
+  std::unique_ptr x;
+  if (MACRO(x) == nullptr)
+;
 }
Index: test/clang-tidy/readability-redundant-smartptr-get-macros.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-smartptr-get-macros.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s readability-redundant-smartptr-get %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-redundant-smartptr-get.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -std=c++11
+
+namespace std {
+
+template 
+struct shared_ptr {
+  T &operator*() const;
+  T *operator->() const;
+  T *get() const;
+  explicit operator bool() const noexcept;
+};
+
+} // namespace std
+
+#define MACRO(p) p.get()
+
+void Positive() {
+  std::shared_ptr x;
+  if (MACRO(x) == nullptr)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:13: warning: redundant get() call on smart pointer
+};
Index: docs/clang-tidy/checks/readability-redundant-smartptr-get.rst
===
--- docs/clang-tidy/checks/readability-redundant-smartptr-get.rst
+++ docs/clang-tidy/checks/readability-redundant-smartptr-get.rst
@@ -14,3 +14,8 @@
   *ptr->get()  ==>  **ptr
   if (ptr.get() == nullptr) ... => if (ptr == nullptr) ...
 
+
+.. option:: IgnoreMacros
+
+   If this option is set to non-zero (default is `1`), the check will not warn
+   about calls inside macros.
Index: clang-tidy/readability/RedundantSmartptrGetCheck.h
===
--- clang-tidy/readability/RedundantSmartptrGetCheck.h
+++ clang-tidy/readability/RedundantSmartptrGetCheck.h
@@ -28,9 +28,14 @@
 class RedundantSmartptrGetCheck : public ClangTidyCheck {
 public:
   RedundantSmartptrGetCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const bool IgnoreMacros;
 };
 
 } // namespace readability
Index: clang-tidy/readability/RedundantSmartptrGetCheck.cpp
===
--- clang-tidy/readability/RedundantSmartptrGetCheck.cpp
+++ clang-tidy/readability/RedundantSmartptrGetCheck.cpp
@@ -91,6 +91,11 @@
 
 } // namespace
 
+void RedundantSmartptrGetCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+}
+
 void RedundantSmartptrGetCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matchers for C++; the functionality currently does not
   // provide any benefit to other languages, despite being benign.
@@ -126,6 +131,9 @@
   bool IsPtrToPtr = Result.Nodes.getNodeAs("ptr_to_ptr") != nullptr;
   bool IsMemberExpr = Result.Nodes.getNodeAs("memberExpr") != nullptr;
   const auto *GetCall = Result.Nodes.getNodeAs("redundant_get");
+  if (GetCall->getBeginLoc().isMacroID() && IgnoreMacros)
+return;
+
   const auto *Smartptr = Result.Nodes.getNodeAs("smart_pointer");
 
   if (IsPtrToPtr && IsMemberExpr) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53454: [clang-tidy] add IgnoreMacros option to eadability-redundant-smartptr-get

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

In practice cppunit's `CPPUNIT_TEST_SUITE_END` macro triggers this problem.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53454



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

[PATCH] D53454: [clang-tidy] add IgnoreMacros option to eadability-redundant-smartptr-get

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

In https://reviews.llvm.org/D53454#1270298, @JonasToth wrote:

> LGTM, but could you please add a short notice in the release notes?


Sure, I will do that.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53454



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

[PATCH] D53454: [clang-tidy] add IgnoreMacros option to eadability-redundant-smartptr-get

[Miklos Vajna via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL344871: [clang-tidy] add IgnoreMacros option to 
readability-redundant-smartptr-get (authored by vmiklos, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53454?vs=170256&id=170347#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53454

Files:
  clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
  clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-redundant-smartptr-get.rst
  
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-smartptr-get-macros.cpp
  clang-tools-extra/trunk/test/clang-tidy/readability-redundant-smartptr-get.cpp

Index: clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
@@ -91,6 +91,11 @@
 
 } // namespace
 
+void RedundantSmartptrGetCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+}
+
 void RedundantSmartptrGetCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matchers for C++; the functionality currently does not
   // provide any benefit to other languages, despite being benign.
@@ -126,6 +131,9 @@
   bool IsPtrToPtr = Result.Nodes.getNodeAs("ptr_to_ptr") != nullptr;
   bool IsMemberExpr = Result.Nodes.getNodeAs("memberExpr") != nullptr;
   const auto *GetCall = Result.Nodes.getNodeAs("redundant_get");
+  if (GetCall->getBeginLoc().isMacroID() && IgnoreMacros)
+return;
+
   const auto *Smartptr = Result.Nodes.getNodeAs("smart_pointer");
 
   if (IsPtrToPtr && IsMemberExpr) {
Index: clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.h
+++ clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.h
@@ -28,9 +28,14 @@
 class RedundantSmartptrGetCheck : public ClangTidyCheck {
 public:
   RedundantSmartptrGetCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const bool IgnoreMacros;
 };
 
 } // namespace readability
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-redundant-smartptr-get.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/readability-redundant-smartptr-get.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/readability-redundant-smartptr-get.rst
@@ -14,3 +14,8 @@
   *ptr->get()  ==>  **ptr
   if (ptr.get() == nullptr) ... => if (ptr == nullptr) ...
 
+
+.. option:: IgnoreMacros
+
+   If this option is set to non-zero (default is `1`), the check will not warn
+   about calls inside macros.
Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst
===
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst
@@ -67,6 +67,10 @@
 Improvements to clang-tidy
 --
 
+- The :doc:`readability-redundant-smartptr-get
+  ` check does not warn
+  about calls inside macros anymore by default.
+
 - New :doc:`abseil-duration-division
   ` check.
 
Index: clang-tools-extra/trunk/test/clang-tidy/readability-redundant-smartptr-get-macros.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/readability-redundant-smartptr-get-macros.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/readability-redundant-smartptr-get-macros.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s readability-redundant-smartptr-get %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-redundant-smartptr-get.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -std=c++11
+
+namespace std {
+
+template 
+struct shared_ptr {
+  T &operator*() const;
+  T *operator->() const;
+  T *get() const;
+  explicit operator bool() const noexcept;
+};
+
+} // namespace std
+
+#define MACRO(p) p.get()
+
+void Positive() {
+  std::shared_ptr x;
+  if (MACRO(x) == nullptr)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:13: warning: redundant get() call on smart pointer
+};
Index: clang-tools-extra/trunk/test/clang-tidy/readability-redundant-smartpt

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350922: [clang-tidy] new check 
'readability-redundant-preprocessor' (authored by vmiklos, committed 
by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54349?vs=174439&id=181214#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54349/new/

https://reviews.llvm.org/D54349

Files:
  clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/trunk/clang-tidy/readability/RedundantPreprocessorCheck.cpp
  clang-tools-extra/trunk/clang-tidy/readability/RedundantPreprocessorCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-redundant-preprocessor.rst
  
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
  clang-tools-extra/trunk/test/clang-tidy/readability-redundant-preprocessor.cpp
  clang-tools-extra/trunk/test/clang-tidy/readability-redundant-preprocessor.h

Index: clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
@@ -24,6 +24,7 @@
   RedundantDeclarationCheck.cpp
   RedundantFunctionPtrDereferenceCheck.cpp
   RedundantMemberInitCheck.cpp
+  RedundantPreprocessorCheck.cpp
   RedundantSmartptrGetCheck.cpp
   RedundantStringCStrCheck.cpp
   RedundantStringInitCheck.cpp
Index: clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -31,6 +31,7 @@
 #include "RedundantDeclarationCheck.h"
 #include "RedundantFunctionPtrDereferenceCheck.h"
 #include "RedundantMemberInitCheck.h"
+#include "RedundantPreprocessorCheck.h"
 #include "RedundantSmartptrGetCheck.h"
 #include "RedundantStringCStrCheck.h"
 #include "RedundantStringInitCheck.h"
@@ -83,6 +84,8 @@
 "readability-redundant-function-ptr-dereference");
 CheckFactories.registerCheck(
 "readability-redundant-member-init");
+CheckFactories.registerCheck(
+"readability-redundant-preprocessor");
 CheckFactories.registerCheck(
 "readability-simplify-subscript-expr");
 CheckFactories.registerCheck(
Index: clang-tools-extra/trunk/clang-tidy/readability/RedundantPreprocessorCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/RedundantPreprocessorCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/RedundantPreprocessorCheck.cpp
@@ -0,0 +1,109 @@
+//===--- RedundantPreprocessorCheck.cpp - clang-tidy --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "RedundantPreprocessorCheck.h"
+#include "clang/Frontend/CompilerInstance.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+namespace {
+/// Information about an opening preprocessor directive.
+struct PreprocessorEntry {
+  SourceLocation Loc;
+  /// Condition used after the preprocessor directive.
+  std::string Condition;
+};
+
+class RedundantPreprocessorCallbacks : public PPCallbacks {
+  enum DirectiveKind { DK_If = 0, DK_Ifdef = 1, DK_Ifndef = 2 };
+
+public:
+  explicit RedundantPreprocessorCallbacks(ClangTidyCheck &Check,
+  Preprocessor &PP)
+  : Check(Check), PP(PP),
+WarningDescription("nested redundant %select{#if|#ifdef|#ifndef}0; "
+   "consider removing it"),
+NoteDescription("previous %select{#if|#ifdef|#ifndef}0 was here") {}
+
+  void If(SourceLocation Loc, SourceRange ConditionRange,
+  ConditionValueKind ConditionValue) override {
+StringRef Condition =
+Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange),
+ PP.getSourceManager(), PP.getLangOpts());
+CheckMacroRedundancy(Loc, Condition, IfStack, DK_If, DK_If, true);
+  }
+
+  void Ifdef(SourceLocation Loc, const Token &MacroNameTok,
+ const MacroDefinition &MacroDefinition) override {
+std::string MacroName = PP.getSpelling(MacroNameTok);
+CheckMacroRedundancy(Loc, MacroName, IfdefStack, DK_Ifdef, DK_Ifdef, true);
+CheckMacroRedundancy(Loc, MacroName, IfndefStack, DK_Ifdef, DK_Ifndef,
+ false);

[PATCH] D56966: [clang-tidy] misc-non-private-member-variables-in-classes: ignore implicit methods

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos marked 8 inline comments as done.
vmiklos added a comment.

I also noticed I forgot to clang-format the testcase, done now.




Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:21
 
 AST_MATCHER(CXXRecordDecl, hasMethods) {
+  for (const auto &Method : Node.methods()) {

lebedev.ri wrote:
> let's keep this one as-is.
Done.



Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:31-34
 AST_MATCHER(CXXRecordDecl, hasNonStaticMethod) {
   return hasMethod(unless(isStaticStorageClass()))
   .matches(Node, Finder, Builder);
 }

lebedev.ri wrote:
> And change this to something like
> ```
> AST_MATCHER(CXXRecordDecl, hasNonStaticNonImplicitMethod) {
>   return hasMethod(unless(anyOf(isStaticStorageClass(), isImplicit(
>   .matches(Node, Finder, Builder);
> }
> ```
Done.



Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:74-76
   // We only want the records that not only contain the mutable data 
(non-static
   // member variables), but also have some logic (non-static member functions).
   // We may optionally ignore records where all the member variables are 
public.

lebedev.ri wrote:
> Comment needs updating?
Done.



Comment at: 
docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst:9
 
-Finds classes that contain non-static data members in addition to non-static
+Finds classes that contain non-static data members in addition to explicit 
non-static
 member functions and diagnose all data members declared with a non-``public``

lebedev.ri wrote:
> I don't like the wording. We are not looking for `explicit` methods,
> we are looking for user-provided methods, as opposed to compiler-provided 
> methods.
OK, used "user-provided" instead.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56966/new/

https://reviews.llvm.org/D56966



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

[PATCH] D56966: [clang-tidy] misc-non-private-member-variables-in-classes: ignore implicit methods

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos updated this revision to Diff 182702.
vmiklos marked 3 inline comments as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56966/new/

https://reviews.llvm.org/D56966

Files:
  clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
  docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst
  test/clang-tidy/misc-non-private-member-variables-in-classes.cpp


Index: test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
===
--- test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
+++ test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
@@ -35,6 +35,18 @@
   int S1_v3;
 };
 
+// Only data and implicit methods, do not warn
+
+class C {
+public:
+  C() {}
+  ~C() {}
+};
+
+struct S1Implicit {
+  C S1Implicit_v0;
+};
+
 
////
 
 // All functions are static, do not warn.
Index: docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst
===
--- docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst
+++ docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst
@@ -6,11 +6,11 @@
 `cppcoreguidelines-non-private-member-variables-in-classes` redirects here
 as an alias for this check.
 
-Finds classes that contain non-static data members in addition to non-static
-member functions and diagnose all data members declared with a non-``public``
-access specifier. The data members should be declared as ``private`` and
-accessed through member functions instead of exposed to derived classes or
-class consumers.
+Finds classes that contain non-static data members in addition to user-provided
+non-static member functions and diagnose all data members declared with a
+non-``public`` access specifier. The data members should be declared as
+``private`` and accessed through member functions instead of exposed to derived
+classes or class consumers.
 
 Options
 ---
Index: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
===
--- clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
+++ clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
@@ -23,7 +23,7 @@
 }
 
 AST_MATCHER(CXXRecordDecl, hasNonStaticMethod) {
-  return hasMethod(unless(isStaticStorageClass()))
+  return hasMethod(unless(anyOf(isStaticStorageClass(), isImplicit(
   .matches(Node, Finder, Builder);
 }
 
@@ -66,8 +66,9 @@
   IgnorePublicMemberVariables ? isProtected() : unless(isPrivate()));
 
   // We only want the records that not only contain the mutable data 
(non-static
-  // member variables), but also have some logic (non-static member functions).
-  // We may optionally ignore records where all the member variables are 
public.
+  // member variables), but also have some logic (non-static, non-implicit
+  // member functions).  We may optionally ignore records where all the member
+  // variables are public.
   Finder->addMatcher(cxxRecordDecl(anyOf(isStruct(), isClass()), hasMethods(),
hasNonStaticMethod(),
unless(ShouldIgnoreRecord),


Index: test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
===
--- test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
+++ test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
@@ -35,6 +35,18 @@
   int S1_v3;
 };
 
+// Only data and implicit methods, do not warn
+
+class C {
+public:
+  C() {}
+  ~C() {}
+};
+
+struct S1Implicit {
+  C S1Implicit_v0;
+};
+
 ////
 
 // All functions are static, do not warn.
Index: docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst
===
--- docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst
+++ docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst
@@ -6,11 +6,11 @@
 `cppcoreguidelines-non-private-member-variables-in-classes` redirects here
 as an alias for this check.
 
-Finds classes that contain non-static data members in addition to non-static
-member functions and diagnose all data members declared with a non-``public``
-access specifier. The data members should be declared as ``private`` and
-accessed through member functions instead of exposed to derived classes or
-class consumers.
+Finds classes that contain non-static data members in addition to user-provided
+non-static member functions and diagnose all data members declared with a
+non-``public`` access specifier. The data members should be declared as
+``private`` and accessed through member functions instead of exposed to derived
+classes or class c

[PATCH] D56966: [clang-tidy] misc-non-private-member-variables-in-classes: ignore implicit methods

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

Sorry, forgot to use the magic line at the end of the commit message to 
auto-close this review. Done in r351686, anyhow.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56966/new/

https://reviews.llvm.org/D56966



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-pp'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos created this revision.
vmiklos added reviewers: JonasToth, alexfh.
Herald added subscribers: cfe-commits, kbarton, xazax.hun, mgorny, nemanjai.

Finds potentially redundant preprocessor usage.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54349

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/RedundantPpCheck.cpp
  clang-tidy/readability/RedundantPpCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-redundant-pp.rst
  test/clang-tidy/readability-redundant-pp-ifdef.cpp
  test/clang-tidy/readability-redundant-pp.cpp

Index: test/clang-tidy/readability-redundant-pp.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-pp.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s readability-redundant-pp %t
+
+// Positive testing.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-pp]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here
+void f();
+#endif
+#endif
+
+// Negative testing.
+#ifndef BAR
+void g();
+#endif
+
+#ifndef FOO
+#ifndef BAR
+void h();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-pp-ifdef.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-pp-ifdef.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s readability-redundant-pp %t -- -- -DFOO
+
+// Positive testing.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-pp]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here
+void f();
+#endif
+#endif
+
+// Negative testing.
+#ifdef BAR
+void g();
+#endif
+
+#ifdef FOO
+#ifdef BAR
+void h();
+#endif
+#endif
Index: docs/clang-tidy/checks/readability-redundant-pp.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-pp.rst
@@ -0,0 +1,11 @@
+.. title:: clang-tidy - readability-redundant-pp
+
+readability-redundant-pp
+=
+
+Finds potentially redundant preprocessor usage. At the moment the following cases are detected:
+
+* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the
+  same condition.
+
+* Same for `#ifndef` .. `#endif` pairs.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -240,6 +240,7 @@
readability-misplaced-array-index
readability-named-parameter
readability-non-const-parameter
+   readability-redundant-pp
readability-redundant-control-flow
readability-redundant-declaration
readability-redundant-function-ptr-dereference
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,6 +67,12 @@
 Improvements to clang-tidy
 --
 
+- New :doc:`readability-redundant-pp
+  ` check.
+
+  Finds potentially redundant preprocessor usage.
+
+
 - New :doc:`abseil-duration-division
   ` check.
 
Index: clang-tidy/readability/RedundantPpCheck.h
===
--- /dev/null
+++ clang-tidy/readability/RedundantPpCheck.h
@@ -0,0 +1,34 @@
+//===--- RedundantPpCheck.h - clang-tidy ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPPCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPPCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// This check flags redundant preprocessor usage.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-pp.html
+class RedundantPpCheck : public ClangTidyCheck {
+public:
+  RedundantPpCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerPPCallbacks(CompilerInstance &Compiler) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPPCHECK_H
Index: clang-tidy/readability/RedundantPpCheck.cpp
===
--- /dev/null
+++ clang-tidy/readability/RedundantPpCheck.cpp
@@ -0,0 +1,94 @@
+//===--- RedundantPpCheck.cpp - clang-tidy ---===//
+//
+//   

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-pp'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

I've used this check originally on LibreOffice (core, online) code and it found 
a handful of not necessary ifdef / ifndef lines. It seemed it's simple and 
generic enough that it would make sense to upstream this.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54349



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-pp'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

In https://reviews.llvm.org/D54349#1293622, @lebedev.ri wrote:

> No one will know for sure what "pp" in "readability-redundant-pp" means.
>  I'd highly recommend to fully spell it out.


Will do.

> Also, i'd like to see some analysis of the false-positives.

Things I considered:

- header guards would easily generate "nested ifndef" false positives, so I 
limited the check to the main file only

- if there are nested `#ifdef FOO` .. `#endif` blocks, but FOO is not defined, 
we fail to detect the redundancy.

- I read that in general checks should be careful around templates and macros, 
but given this deals with the preprocessor, I don't expect issues there.

This implicitly means that I'm not aware of actual false positives of the check 
in its current form.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54349



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos updated this revision to Diff 173465.
vmiklos retitled this revision from "[clang-tidy] new check 
'readability-redundant-pp'" to "[clang-tidy] new check 
'readability-redundant-preprocessor'".

https://reviews.llvm.org/D54349

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-redundant-preprocessor.rst
  test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
  test/clang-tidy/readability-redundant-preprocessor.cpp
  test/clang-tidy/readability-redundant-preprocessor.h

Index: test/clang-tidy/readability-redundant-preprocessor.h
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.h
@@ -0,0 +1,5 @@
+#ifndef FOO
+#ifndef FOO // this would warn, but not in a header
+void f();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.cpp
@@ -0,0 +1,23 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -I %S
+
+// Positive testing.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here
+void f();
+#endif
+#endif
+
+// Negative testing.
+#include "readability-redundant-preprocessor.h"
+
+#ifndef BAR
+void g();
+#endif
+
+#ifndef FOO
+#ifndef BAR
+void h();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -DFOO
+
+// Positive testing.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here
+void f();
+#endif
+#endif
+
+// Negative testing.
+#ifdef BAR
+void g();
+#endif
+
+#ifdef FOO
+#ifdef BAR
+void h();
+#endif
+#endif
Index: docs/clang-tidy/checks/readability-redundant-preprocessor.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-preprocessor.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-redundant-preprocessor
+
+readability-redundant-preprocessor
+==
+
+Finds potentially redundant preprocessor usage. At the moment the following
+cases are detected:
+
+* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the
+  same condition. For example:
+
+.. code-block:: c++
+
+  #ifdef FOO
+  #ifdef FOO // inner ifdef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* Same for `#ifndef` .. `#endif` pairs. For example:
+
+.. code-block:: c++
+
+  #ifndef FOO
+  #ifndef FOO // inner ifndef is considered redundant
+  void f();
+  #endif
+  #endif
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -244,6 +244,7 @@
readability-redundant-declaration
readability-redundant-function-ptr-dereference
readability-redundant-member-init
+   readability-redundant-preprocessor
readability-redundant-smartptr-get
readability-redundant-string-cstr
readability-redundant-string-init
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -154,6 +154,11 @@
   Detects usage of magic numbers, numbers that are used as literals instead of
   introduced via constants or symbols.
 
+- New :doc:`readability-redundant-preprocessor
+  ` check.
+
+  Finds potentially redundant preprocessor usage.
+
 - New :doc:`readability-uppercase-literal-suffix
   ` check.
 
Index: clang-tidy/readability/RedundantPreprocessorCheck.h
===
--- /dev/null
+++ clang-tidy/readability/RedundantPreprocessorCheck.h
@@ -0,0 +1,35 @@
+//===--- RedundantPreprocessorCheck.h - clang-tidy ---*- C++
+//-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPREPROCESSORCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TID

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

> I think that name is not very descriptive for the user of clang-tidy. `pp`
>  should be `preprocessor` or some other constellation that makes it very clear
>  its about the preprocessor.

Done, renamed to readability-redundant-preprocessor.

> you are in namespace `clang`, you can ellide `clang::`

Done.

> Maybe `SmallVector`? Would be better for performance.

Done.

> I think it would be better to have these methods defined inline, as the
>  splitting does not achieve its original goal (declaration in header,
>  definition in impl).

Done.

> The two functions are copied, please remove this duplicatoin and refactor it
>  to a general parametrized function.

Done.

> Please order the checks alphabetically in the release notes, and one empty
>  line at the end is enough.

Done.

> This needs more explanation, please add `.. code-block:: c++` sections for
>  the cases that demonstrate the issue.

Done.

> Please add a test where the redundancy comes from including other headers. If
>  the headers are nested this case might still occur, but its not safe to
>  diagnose/remove these checks as other include-places might not have the same
>  constellation.
> 
> `ifdef` and `ifndef` are used for the include-guards and therefore
>  particularly necessary to test.

Done. I've added a test to make sure we don't warn in headers, the code for
this was already there, just had no coverage. (Exactly for the reason you
mention, the possibility of include-guards generating false positives.)


https://reviews.llvm.org/D54349



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos updated this revision to Diff 173468.
vmiklos edited the summary of this revision.

https://reviews.llvm.org/D54349

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-redundant-preprocessor.rst
  test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
  test/clang-tidy/readability-redundant-preprocessor.cpp
  test/clang-tidy/readability-redundant-preprocessor.h

Index: test/clang-tidy/readability-redundant-preprocessor.h
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.h
@@ -0,0 +1,5 @@
+#ifndef FOO
+#ifndef FOO // this would warn, but not in a header
+void f();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.cpp
@@ -0,0 +1,23 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -I %S
+
+// Positive testing.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here
+void f();
+#endif
+#endif
+
+// Negative testing.
+#include "readability-redundant-preprocessor.h"
+
+#ifndef BAR
+void g();
+#endif
+
+#ifndef FOO
+#ifndef BAR
+void h();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -DFOO
+
+// Positive testing.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here
+void f();
+#endif
+#endif
+
+// Negative testing.
+#ifdef BAR
+void g();
+#endif
+
+#ifdef FOO
+#ifdef BAR
+void h();
+#endif
+#endif
Index: docs/clang-tidy/checks/readability-redundant-preprocessor.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-preprocessor.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-redundant-preprocessor
+
+readability-redundant-preprocessor
+==
+
+Finds potentially redundant preprocessor directives. At the moment the
+following cases are detected:
+
+* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the
+  same condition. For example:
+
+.. code-block:: c++
+
+  #ifdef FOO
+  #ifdef FOO // inner ifdef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* Same for `#ifndef` .. `#endif` pairs. For example:
+
+.. code-block:: c++
+
+  #ifndef FOO
+  #ifndef FOO // inner ifndef is considered redundant
+  void f();
+  #endif
+  #endif
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -244,6 +244,7 @@
readability-redundant-declaration
readability-redundant-function-ptr-dereference
readability-redundant-member-init
+   readability-redundant-preprocessor
readability-redundant-smartptr-get
readability-redundant-string-cstr
readability-redundant-string-init
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -154,6 +154,11 @@
   Detects usage of magic numbers, numbers that are used as literals instead of
   introduced via constants or symbols.
 
+- New :doc:`readability-redundant-preprocessor
+  ` check.
+
+  Finds potentially redundant preprocessor directives.
+
 - New :doc:`readability-uppercase-literal-suffix
   ` check.
 
Index: clang-tidy/readability/RedundantPreprocessorCheck.h
===
--- /dev/null
+++ clang-tidy/readability/RedundantPreprocessorCheck.h
@@ -0,0 +1,35 @@
+//===--- RedundantPreprocessorCheck.h - clang-tidy ---*- C++
+//-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPREPROCESSORCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPREPROCESSORCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespa

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

> preprocessor directives? Same in documentation.

Done.


https://reviews.llvm.org/D54349



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos updated this revision to Diff 173524.

https://reviews.llvm.org/D54349

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-redundant-preprocessor.rst
  test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
  test/clang-tidy/readability-redundant-preprocessor.cpp
  test/clang-tidy/readability-redundant-preprocessor.h

Index: test/clang-tidy/readability-redundant-preprocessor.h
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.h
@@ -0,0 +1,5 @@
+#ifndef FOO
+#ifndef FOO // this would warn, but not in a header
+void f();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.cpp
@@ -0,0 +1,23 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -I %S
+
+// Positive testing.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here
+void f();
+#endif
+#endif
+
+// Negative testing.
+#include "readability-redundant-preprocessor.h"
+
+#ifndef BAR
+void g();
+#endif
+
+#ifndef FOO
+#ifndef BAR
+void h();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -DFOO
+
+// Positive testing.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here
+void f();
+#endif
+#endif
+
+// Negative testing.
+#ifdef BAR
+void g();
+#endif
+
+#ifdef FOO
+#ifdef BAR
+void h();
+#endif
+#endif
Index: docs/clang-tidy/checks/readability-redundant-preprocessor.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-preprocessor.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-redundant-preprocessor
+
+readability-redundant-preprocessor
+==
+
+Finds potentially redundant preprocessor directives. At the moment the
+following cases are detected:
+
+* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the
+  same condition. For example:
+
+.. code-block:: c++
+
+  #ifdef FOO
+  #ifdef FOO // inner ifdef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* Same for `#ifndef` .. `#endif` pairs. For example:
+
+.. code-block:: c++
+
+  #ifndef FOO
+  #ifndef FOO // inner ifndef is considered redundant
+  void f();
+  #endif
+  #endif
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -244,6 +244,7 @@
readability-redundant-declaration
readability-redundant-function-ptr-dereference
readability-redundant-member-init
+   readability-redundant-preprocessor
readability-redundant-smartptr-get
readability-redundant-string-cstr
readability-redundant-string-init
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -154,6 +154,11 @@
   Detects usage of magic numbers, numbers that are used as literals instead of
   introduced via constants or symbols.
 
+- New :doc:`readability-redundant-preprocessor
+  ` check.
+
+  Finds potentially redundant preprocessor directives.
+
 - New :doc:`readability-uppercase-literal-suffix
   ` check.
 
Index: clang-tidy/readability/RedundantPreprocessorCheck.h
===
--- /dev/null
+++ clang-tidy/readability/RedundantPreprocessorCheck.h
@@ -0,0 +1,35 @@
+//===--- RedundantPreprocessorCheck.h - clang-tidy --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPREPROCESSORCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPREPROCESSORCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// This check flag

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

> What do you think about code like:
> 
>   #if FOO == 4
>   #if FOO == 4
>   #endif
>   #endif
>   
>   #if defined(FOO)
>   #if defined(FOO)
>   #endif
>   #endif
>   
>   #if !defined(FOO)
>   #if !defined(FOO)
>   #endif
>   #endif

I looked at supporting these, but it's not clear to me how to get e.g. the 'FOO

4' string (the condition) from the `If()` callback. So far I only see how to


get the start location of the condition and the whole range till the end of
endif. Do you have a code pointer how to do this, please? (Or OK if I leave
this for a follow-up patch?)

>   #if defined(FOO)
>   #if !defined(FOO)
>   #endif
>   #endif
>   
>   #if !defined(FOO)
>   #if defined(FOO)
>   #endif
>   #endif

I expect handling these correctly is more complex -- I would prefer focusing on
matching conditons in this patch, and get back to inverted conditions in a
follow-up patch. Is that OK? If we handle inverted conditions, then handling `a
&& b` and `!a || !b` would make sense as well for example.

> Please keep this list sorted alphabetically.

Done.

> This name is not particularly descriptive. This seems to be more like
>  `CheckMacroRedundancy` or something like that?

Makes sense, done.

> This comment should be re-flowed to fit the column width.

Done.

> What constitutes "redundancy"? A bit more exposition here would be useful.

Hopefully "nested directives with the same condition" makes it easier to
understand the intention and current scope.


https://reviews.llvm.org/D54349



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos marked an inline comment as done.
vmiklos added inline comments.



Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:19-22
+struct Entry {
+  SourceLocation Loc;
+  std::string MacroName;
+};

Szelethus wrote:
> This is a way too general name in my opinion. Either include comments that 
> describe it, or rename it (preferably both).
Renamed to `PreprocessorCondition`, hope it helps. :-)


https://reviews.llvm.org/D54349



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos updated this revision to Diff 173526.
vmiklos marked an inline comment as done.

https://reviews.llvm.org/D54349

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-redundant-preprocessor.rst
  test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
  test/clang-tidy/readability-redundant-preprocessor.cpp
  test/clang-tidy/readability-redundant-preprocessor.h

Index: test/clang-tidy/readability-redundant-preprocessor.h
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.h
@@ -0,0 +1,5 @@
+#ifndef FOO
+#ifndef FOO // this would warn, but not in a header
+void f();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.cpp
@@ -0,0 +1,23 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -I %S
+
+// Positive testing.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here
+void f();
+#endif
+#endif
+
+// Negative testing.
+#include "readability-redundant-preprocessor.h"
+
+#ifndef BAR
+void g();
+#endif
+
+#ifndef FOO
+#ifndef BAR
+void h();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -DFOO
+
+// Positive testing.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here
+void f();
+#endif
+#endif
+
+// Negative testing.
+#ifdef BAR
+void g();
+#endif
+
+#ifdef FOO
+#ifdef BAR
+void h();
+#endif
+#endif
Index: docs/clang-tidy/checks/readability-redundant-preprocessor.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-preprocessor.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-redundant-preprocessor
+
+readability-redundant-preprocessor
+==
+
+Finds potentially redundant preprocessor directives. At the moment the
+following cases are detected:
+
+* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the
+  same condition. For example:
+
+.. code-block:: c++
+
+  #ifdef FOO
+  #ifdef FOO // inner ifdef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* Same for `#ifndef` .. `#endif` pairs. For example:
+
+.. code-block:: c++
+
+  #ifndef FOO
+  #ifndef FOO // inner ifndef is considered redundant
+  void f();
+  #endif
+  #endif
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -244,6 +244,7 @@
readability-redundant-declaration
readability-redundant-function-ptr-dereference
readability-redundant-member-init
+   readability-redundant-preprocessor
readability-redundant-smartptr-get
readability-redundant-string-cstr
readability-redundant-string-init
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -154,6 +154,11 @@
   Detects usage of magic numbers, numbers that are used as literals instead of
   introduced via constants or symbols.
 
+- New :doc:`readability-redundant-preprocessor
+  ` check.
+
+  Finds potentially redundant preprocessor directives.
+
 - New :doc:`readability-uppercase-literal-suffix
   ` check.
 
Index: clang-tidy/readability/RedundantPreprocessorCheck.h
===
--- /dev/null
+++ clang-tidy/readability/RedundantPreprocessorCheck.h
@@ -0,0 +1,35 @@
+//===--- RedundantPreprocessorCheck.h - clang-tidy --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPREPROCESSORCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPREPROCESSORCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+name

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos updated this revision to Diff 173554.

https://reviews.llvm.org/D54349

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-redundant-preprocessor.rst
  test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
  test/clang-tidy/readability-redundant-preprocessor.cpp
  test/clang-tidy/readability-redundant-preprocessor.h

Index: test/clang-tidy/readability-redundant-preprocessor.h
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.h
@@ -0,0 +1,5 @@
+#ifndef FOO
+#ifndef FOO // this would warn, but not in a header
+void f();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.cpp
@@ -0,0 +1,38 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -I %S
+
+// Positive testing.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here
+void f();
+#endif
+#endif
+
+// Positive testing of inverted condition.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here
+void f2();
+#endif
+#endif
+
+// Negative testing.
+#include "readability-redundant-preprocessor.h"
+
+#ifndef BAR
+void g();
+#endif
+
+#ifndef FOO
+#ifndef BAR
+void h();
+#endif
+#endif
+
+#ifndef FOO
+#ifdef BAR
+void i();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
@@ -0,0 +1,36 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -DFOO
+
+// Positive testing.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here
+void f();
+#endif
+#endif
+
+// Positive testing of inverted condition.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here
+void f2();
+#endif
+#endif
+
+// Negative testing.
+#ifdef BAR
+void g();
+#endif
+
+#ifdef FOO
+#ifdef BAR
+void h();
+#endif
+#endif
+
+#ifdef FOO
+#ifndef BAR
+void i();
+#endif
+#endif
Index: docs/clang-tidy/checks/readability-redundant-preprocessor.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-preprocessor.rst
@@ -0,0 +1,49 @@
+.. title:: clang-tidy - readability-redundant-preprocessor
+
+readability-redundant-preprocessor
+==
+
+Finds potentially redundant preprocessor directives. At the moment the
+following cases are detected:
+
+* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the
+  same condition. For example:
+
+.. code-block:: c++
+
+  #ifdef FOO
+  #ifdef FOO // inner ifdef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* Same for `#ifndef` .. `#endif` pairs. For example:
+
+.. code-block:: c++
+
+  #ifndef FOO
+  #ifndef FOO // inner ifndef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* `#ifndef` inside an `#ifdef` with the same condition:
+
+.. code-block:: c++
+
+ #ifdef FOO
+ #ifndef FOO // inner ifndef is considered redundant
+ void f();
+ #endif
+ #endif
+
+* `#ifdef` inside an `#ifndef` with the same condition:
+
+.. code-block:: c++
+
+ #ifndef FOO
+ #ifdef FOO
+ void f();
+ #endif
+ #endif
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -244,6 +244,7 @@
readability-redundant-declaration
readability-redundant-function-ptr-dereference
readability-redundant-member-init
+   readability-redundant-preprocessor
readability-redundant-smartptr-get
readability-redundant-string-cstr
readability-redundant-string-init
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -154,6 +154,11 @@
   Detects usage of magic numbers, numbers that are used as literals instead of
   introduced via constants or symbols.
 
+- New :doc:`readability-redunda

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

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

> As it stands, I feel like this check is a bit too simplistic to have much 
> value, but if you covered some of these other cases, it would alleviate that 
> worry for me.

I've now added support for detecting inverted conditions when it comes to 
`#ifdef` and `#ifndef` (see extended documentation and testcases). I'll also 
check what can I do for `#if`. I think it would not be too hard to detect just 
repeated conditions. if I have the full range, then I can look for end of the 
first line that does not end with `\`, and that would be a poor man's way to 
get the `#if` condition. Let's see if that works in practice.




Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:19-22
+struct Entry {
+  SourceLocation Loc;
+  std::string MacroName;
+};

Szelethus wrote:
> vmiklos wrote:
> > Szelethus wrote:
> > > This is a way too general name in my opinion. Either include comments 
> > > that describe it, or rename it (preferably both).
> > Renamed to `PreprocessorCondition`, hope it helps. :-)
> I actually meant it for `Entry`, if you hover your mouse over an inline 
> comment, you can see which lines it applies to. Sorry for the confusing 
> communication :D
Done now. Nah, it's more about I'm not so fluent with phabricator. :-)


https://reviews.llvm.org/D54349



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos updated this revision to Diff 173556.
vmiklos marked an inline comment as done.

https://reviews.llvm.org/D54349

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-redundant-preprocessor.rst
  test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
  test/clang-tidy/readability-redundant-preprocessor.cpp
  test/clang-tidy/readability-redundant-preprocessor.h

Index: test/clang-tidy/readability-redundant-preprocessor.h
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.h
@@ -0,0 +1,5 @@
+#ifndef FOO
+#ifndef FOO // this would warn, but not in a header
+void f();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.cpp
@@ -0,0 +1,76 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -I %S
+
+// Positive testing.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here
+void f();
+#endif
+#endif
+
+// Positive testing of inverted condition.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here
+void f2();
+#endif
+#endif
+
+// Negative testing.
+#include "readability-redundant-preprocessor.h"
+
+#ifndef BAR
+void g();
+#endif
+
+#ifndef FOO
+#ifndef BAR
+void h();
+#endif
+#endif
+
+#ifndef FOO
+#ifdef BAR
+void i();
+#endif
+#endif
+
+// Positive #if testing.
+#define FOO 4
+
+#if FOO == 4
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant if; consider removing it [readability-redundant-preprocessor]
+#if FOO == 4
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous if was here
+void j();
+#endif
+#endif
+
+#if FOO == \
+4
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant if; consider removing it [readability-redundant-preprocessor]
+#if FOO == \
+4
+// CHECK-NOTES: [[@LINE-5]]:2: note: previous if was here
+void j();
+#endif
+#endif
+
+// Negative #if testing.
+#define BAR 4
+
+#if FOO == 4
+#if BAR == 4
+void k();
+#endif
+#endif
+
+#if FOO == \
+4
+#if BAR == \
+5
+void k();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
@@ -0,0 +1,36 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -DFOO
+
+// Positive testing.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here
+void f();
+#endif
+#endif
+
+// Positive testing of inverted condition.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here
+void f2();
+#endif
+#endif
+
+// Negative testing.
+#ifdef BAR
+void g();
+#endif
+
+#ifdef FOO
+#ifdef BAR
+void h();
+#endif
+#endif
+
+#ifdef FOO
+#ifndef BAR
+void i();
+#endif
+#endif
Index: docs/clang-tidy/checks/readability-redundant-preprocessor.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-preprocessor.rst
@@ -0,0 +1,61 @@
+.. title:: clang-tidy - readability-redundant-preprocessor
+
+readability-redundant-preprocessor
+==
+
+Finds potentially redundant preprocessor directives. At the moment the
+following cases are detected:
+
+* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the
+  same condition. For example:
+
+.. code-block:: c++
+
+  #ifdef FOO
+  #ifdef FOO // inner ifdef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* Same for `#ifndef` .. `#endif` pairs. For example:
+
+.. code-block:: c++
+
+  #ifndef FOO
+  #ifndef FOO // inner ifndef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* `#ifndef` inside an `#ifdef` with the same condition:
+
+.. code-block:: c++
+
+ #ifdef FOO
+ #ifndef FOO // inner ifndef is considered redundant
+ void f();
+ #endif
+ #endif
+
+* `#ifdef` inside an `#ifndef` with the same condition:
+
+.. code-block:: c++
+
+ #ifndef FOO
+ #ifdef FOO // inner ifdef is considered redundant
+ void f();
+ #endif
+ #endif
+
+* `#if` .. `#endif` pa

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

In https://reviews.llvm.org/D54349#1294600, @vmiklos wrote:

> Let's see if that works in practice.


I've implemented this now, please take a look. (Extended test + docs as well, 
as usual.) Thanks!


https://reviews.llvm.org/D54349



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos updated this revision to Diff 173572.

https://reviews.llvm.org/D54349

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-redundant-preprocessor.rst
  test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
  test/clang-tidy/readability-redundant-preprocessor.cpp
  test/clang-tidy/readability-redundant-preprocessor.h

Index: test/clang-tidy/readability-redundant-preprocessor.h
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.h
@@ -0,0 +1,5 @@
+#ifndef FOO
+#ifndef FOO // this would warn, but not in a header
+void f();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.cpp
@@ -0,0 +1,76 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -I %S
+
+// Positive testing.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here
+void f();
+#endif
+#endif
+
+// Positive testing of inverted condition.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here
+void f2();
+#endif
+#endif
+
+// Negative testing.
+#include "readability-redundant-preprocessor.h"
+
+#ifndef BAR
+void g();
+#endif
+
+#ifndef FOO
+#ifndef BAR
+void h();
+#endif
+#endif
+
+#ifndef FOO
+#ifdef BAR
+void i();
+#endif
+#endif
+
+// Positive #if testing.
+#define FOO 4
+
+#if FOO == 4
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant if; consider removing it [readability-redundant-preprocessor]
+#if FOO == 4
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous if was here
+void j();
+#endif
+#endif
+
+#if FOO == \
+4
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant if; consider removing it [readability-redundant-preprocessor]
+#if FOO == \
+4
+// CHECK-NOTES: [[@LINE-5]]:2: note: previous if was here
+void j();
+#endif
+#endif
+
+// Negative #if testing.
+#define BAR 4
+
+#if FOO == 4
+#if BAR == 4
+void k();
+#endif
+#endif
+
+#if FOO == \
+4
+#if BAR == \
+5
+void k();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
@@ -0,0 +1,36 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -DFOO
+
+// Positive testing.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here
+void f();
+#endif
+#endif
+
+// Positive testing of inverted condition.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here
+void f2();
+#endif
+#endif
+
+// Negative testing.
+#ifdef BAR
+void g();
+#endif
+
+#ifdef FOO
+#ifdef BAR
+void h();
+#endif
+#endif
+
+#ifdef FOO
+#ifndef BAR
+void i();
+#endif
+#endif
Index: docs/clang-tidy/checks/readability-redundant-preprocessor.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-preprocessor.rst
@@ -0,0 +1,61 @@
+.. title:: clang-tidy - readability-redundant-preprocessor
+
+readability-redundant-preprocessor
+==
+
+Finds potentially redundant preprocessor directives. At the moment the
+following cases are detected:
+
+* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the
+  same condition. For example:
+
+.. code-block:: c++
+
+  #ifdef FOO
+  #ifdef FOO // inner ifdef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* Same for `#ifndef` .. `#endif` pairs. For example:
+
+.. code-block:: c++
+
+  #ifndef FOO
+  #ifndef FOO // inner ifndef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* `#ifndef` inside an `#ifdef` with the same condition:
+
+.. code-block:: c++
+
+  #ifdef FOO
+  #ifndef FOO // inner ifndef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* `#ifdef` inside an `#ifndef` with the same condition:
+
+.. code-block:: c++
+
+  #ifndef FOO
+  #ifdef FOO // inner ifdef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* `#if` .. `#endif` pairs which are nested inside an o

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos marked an inline comment as done.
vmiklos added inline comments.



Comment at: docs/clang-tidy/checks/readability-redundant-preprocessor.rst:34
+
+ #ifdef FOO
+ #ifndef FOO // inner ifndef is considered redundant

JonasToth wrote:
> The indendation for these examples (following as well) is not enough. Please 
> use 2 spaces.
Fixed.


https://reviews.llvm.org/D54349



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos marked 2 inline comments as done.
vmiklos added inline comments.



Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:56-59
+StringRef SourceText =
+Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange),
+ PP.getSourceManager(), PP.getLangOpts());
+std::string Condition = getCondition(SourceText);

Szelethus wrote:
> I'm a little confused. To me, it seems like you acquired the condition 
> already -- doesn't `ConditionRange` actually cover the, well, condition 
> range? This is how I imagined it:
> 
> ```
> #ifdef CUTE_PANDA_CUBS
>^~~
>ConditionRange
> ```
> Why is there a need for `getCondition`? Is there any? If there is (maybe the 
> acquired text contains other things), can you document it? I haven't played 
> with `PPCallbacks` much, so I'm fine with being in the wrong.
ConditionRange covers more than what you expect:

```
#if FOO == 4
   ^
void f();
~
#endif
~~
```

to find out if the condition of the `#if` is the same as a previous one, I want 
to extract just `FOO == 4` from that, then deal with that part similar to 
`#ifdef` and `#ifndef`, which are easier as you have a single Token for the 
condition. But you're right, I should add a comment explaining this.


https://reviews.llvm.org/D54349



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos updated this revision to Diff 173575.
vmiklos marked an inline comment as done.

https://reviews.llvm.org/D54349

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-redundant-preprocessor.rst
  test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
  test/clang-tidy/readability-redundant-preprocessor.cpp
  test/clang-tidy/readability-redundant-preprocessor.h

Index: test/clang-tidy/readability-redundant-preprocessor.h
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.h
@@ -0,0 +1,5 @@
+#ifndef FOO
+#ifndef FOO // this would warn, but not in a header
+void f();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.cpp
@@ -0,0 +1,76 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -I %S
+
+// Positive testing.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here
+void f();
+#endif
+#endif
+
+// Positive testing of inverted condition.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here
+void f2();
+#endif
+#endif
+
+// Negative testing.
+#include "readability-redundant-preprocessor.h"
+
+#ifndef BAR
+void g();
+#endif
+
+#ifndef FOO
+#ifndef BAR
+void h();
+#endif
+#endif
+
+#ifndef FOO
+#ifdef BAR
+void i();
+#endif
+#endif
+
+// Positive #if testing.
+#define FOO 4
+
+#if FOO == 4
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant if; consider removing it [readability-redundant-preprocessor]
+#if FOO == 4
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous if was here
+void j();
+#endif
+#endif
+
+#if FOO == \
+4
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant if; consider removing it [readability-redundant-preprocessor]
+#if FOO == \
+4
+// CHECK-NOTES: [[@LINE-5]]:2: note: previous if was here
+void j();
+#endif
+#endif
+
+// Negative #if testing.
+#define BAR 4
+
+#if FOO == 4
+#if BAR == 4
+void k();
+#endif
+#endif
+
+#if FOO == \
+4
+#if BAR == \
+5
+void k();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
@@ -0,0 +1,36 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -DFOO
+
+// Positive testing.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here
+void f();
+#endif
+#endif
+
+// Positive testing of inverted condition.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here
+void f2();
+#endif
+#endif
+
+// Negative testing.
+#ifdef BAR
+void g();
+#endif
+
+#ifdef FOO
+#ifdef BAR
+void h();
+#endif
+#endif
+
+#ifdef FOO
+#ifndef BAR
+void i();
+#endif
+#endif
Index: docs/clang-tidy/checks/readability-redundant-preprocessor.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-preprocessor.rst
@@ -0,0 +1,61 @@
+.. title:: clang-tidy - readability-redundant-preprocessor
+
+readability-redundant-preprocessor
+==
+
+Finds potentially redundant preprocessor directives. At the moment the
+following cases are detected:
+
+* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the
+  same condition. For example:
+
+.. code-block:: c++
+
+  #ifdef FOO
+  #ifdef FOO // inner ifdef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* Same for `#ifndef` .. `#endif` pairs. For example:
+
+.. code-block:: c++
+
+  #ifndef FOO
+  #ifndef FOO // inner ifndef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* `#ifndef` inside an `#ifdef` with the same condition:
+
+.. code-block:: c++
+
+  #ifdef FOO
+  #ifndef FOO // inner ifndef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* `#ifdef` inside an `#ifndef` with the same condition:
+
+.. code-block:: c++
+
+  #ifndef FOO
+  #ifdef FOO // inner ifdef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* `#if` .. `

[PATCH] D54450: Get the correct range of tokens for preprocessor conditions

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

As far as I see GCC warns on these 3 things.




Comment at: lib/Lex/PPDirectives.cpp:553
 } else {
   const SourceLocation CondBegin = CurPPLexer->getSourceLocation();
   // Restore the value of LexingRawMode so that identifiers are

Is `CondBegin` still needed after your changes?



Comment at: lib/Lex/PPDirectives.cpp:563
   if (Callbacks) {
 const SourceLocation CondEnd = CurPPLexer->getSourceLocation();
+Callbacks->Elif(

Is `CondEnd` still needed after your changes?




Comment at: lib/Lex/PPExpressions.cpp:852
 DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;
 return {false, DT.IncludedUndefinedIds};
   }

The new `ExprRange` member is not initialized here, it seems.



https://reviews.llvm.org/D54450



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

[PATCH] D54450: Get the correct range of tokens for preprocessor conditions

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

Oh, and running the `check-clang-tools` target, this currently results in:

  Failing Tests (3):
  Clang Tools :: modularize/ProblemsInconsistent.modularize
  Clang Tools :: pp-trace/pp-trace-conditional.cpp
  Clang Tools :: pp-trace/pp-trace-macro.cpp

Perhaps the pp-trace one just has to be updated for the new behavior. :-)


https://reviews.llvm.org/D54450



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos marked 2 inline comments as done.
vmiklos added inline comments.



Comment at: clang-tidy/readability/ReadabilityTidyModule.cpp:84
+CheckFactories.registerCheck(
+"readability-redundant-preprocessor");
 CheckFactories.registerCheck(

aaron.ballman wrote:
> Please keep this list sorted alphabetically.
Done.



Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:56-59
+StringRef SourceText =
+Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange),
+ PP.getSourceManager(), PP.getLangOpts());
+std::string Condition = getCondition(SourceText);

aaron.ballman wrote:
> Szelethus wrote:
> > vmiklos wrote:
> > > Szelethus wrote:
> > > > I'm a little confused. To me, it seems like you acquired the condition 
> > > > already -- doesn't `ConditionRange` actually cover the, well, condition 
> > > > range? This is how I imagined it:
> > > > 
> > > > ```
> > > > #ifdef CUTE_PANDA_CUBS
> > > >^~~
> > > >ConditionRange
> > > > ```
> > > > Why is there a need for `getCondition`? Is there any? If there is 
> > > > (maybe the acquired text contains other things), can you document it? I 
> > > > haven't played with `PPCallbacks` much, so I'm fine with being in the 
> > > > wrong.
> > > ConditionRange covers more than what you expect:
> > > 
> > > ```
> > > #if FOO == 4
> > >^
> > > void f();
> > > ~
> > > #endif
> > > ~~
> > > ```
> > > 
> > > to find out if the condition of the `#if` is the same as a previous one, 
> > > I want to extract just `FOO == 4` from that, then deal with that part 
> > > similar to `#ifdef` and `#ifndef`, which are easier as you have a single 
> > > Token for the condition. But you're right, I should add a comment 
> > > explaining this.
> > Oh my god. There is no tool or a convenient method for this??? I had the 
> > displeasure of working with the preprocessor in the last couple months, and 
> > I get shocked by things like this almost every day.
> > 
> > Yea, unfortunately you will have to write excessive amount of comments to 
> > counterweights the shortcomings of `Preprocessor` :/
> This is working around a bug, and I think it would be better to fix that bug 
> instead of jump through these hoops here.
> 
> `Preprocessor::EvaluateDirectiveExpression()` needs to squirrel away the 
> condition range in the `DirectiveEvalResult` it returns. I'll take a stab at 
> it and report back.
Thanks! I've now rebased this on top of D54450, to be able to drop the ugly 
`getCondition()` function.


https://reviews.llvm.org/D54349



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos updated this revision to Diff 174021.
vmiklos marked 2 inline comments as done.

https://reviews.llvm.org/D54349

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-redundant-preprocessor.rst
  test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
  test/clang-tidy/readability-redundant-preprocessor.cpp
  test/clang-tidy/readability-redundant-preprocessor.h

Index: test/clang-tidy/readability-redundant-preprocessor.h
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.h
@@ -0,0 +1,5 @@
+#ifndef FOO
+#ifndef FOO // this would warn, but not in a header
+void f();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.cpp
@@ -0,0 +1,76 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -I %S
+
+// Positive testing.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here
+void f();
+#endif
+#endif
+
+// Positive testing of inverted condition.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here
+void f2();
+#endif
+#endif
+
+// Negative testing.
+#include "readability-redundant-preprocessor.h"
+
+#ifndef BAR
+void g();
+#endif
+
+#ifndef FOO
+#ifndef BAR
+void h();
+#endif
+#endif
+
+#ifndef FOO
+#ifdef BAR
+void i();
+#endif
+#endif
+
+// Positive #if testing.
+#define FOO 4
+
+#if FOO == 4
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant if; consider removing it [readability-redundant-preprocessor]
+#if FOO == 4
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous if was here
+void j();
+#endif
+#endif
+
+#if FOO == \
+4
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant if; consider removing it [readability-redundant-preprocessor]
+#if FOO == \
+4
+// CHECK-NOTES: [[@LINE-5]]:2: note: previous if was here
+void j();
+#endif
+#endif
+
+// Negative #if testing.
+#define BAR 4
+
+#if FOO == 4
+#if BAR == 4
+void k();
+#endif
+#endif
+
+#if FOO == \
+4
+#if BAR == \
+5
+void k();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
@@ -0,0 +1,36 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -DFOO
+
+// Positive testing.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here
+void f();
+#endif
+#endif
+
+// Positive testing of inverted condition.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here
+void f2();
+#endif
+#endif
+
+// Negative testing.
+#ifdef BAR
+void g();
+#endif
+
+#ifdef FOO
+#ifdef BAR
+void h();
+#endif
+#endif
+
+#ifdef FOO
+#ifndef BAR
+void i();
+#endif
+#endif
Index: docs/clang-tidy/checks/readability-redundant-preprocessor.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-preprocessor.rst
@@ -0,0 +1,61 @@
+.. title:: clang-tidy - readability-redundant-preprocessor
+
+readability-redundant-preprocessor
+==
+
+Finds potentially redundant preprocessor directives. At the moment the
+following cases are detected:
+
+* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the
+  same condition. For example:
+
+.. code-block:: c++
+
+  #ifdef FOO
+  #ifdef FOO // inner ifdef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* Same for `#ifndef` .. `#endif` pairs. For example:
+
+.. code-block:: c++
+
+  #ifndef FOO
+  #ifndef FOO // inner ifndef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* `#ifndef` inside an `#ifdef` with the same condition:
+
+.. code-block:: c++
+
+  #ifdef FOO
+  #ifndef FOO // inner ifndef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* `#ifdef` inside an `#ifndef` with the same condition:
+
+.. code-block:: c++
+
+  #ifndef FOO
+  #ifdef FOO // inner ifdef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* `#if` .. `

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos updated this revision to Diff 174439.

https://reviews.llvm.org/D54349

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-redundant-preprocessor.rst
  test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
  test/clang-tidy/readability-redundant-preprocessor.cpp
  test/clang-tidy/readability-redundant-preprocessor.h

Index: test/clang-tidy/readability-redundant-preprocessor.h
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.h
@@ -0,0 +1,5 @@
+#ifndef FOO
+#ifndef FOO // this would warn, but not in a header
+void f();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.cpp
@@ -0,0 +1,84 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -I %S
+
+// Positive testing.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant #ifndef; consider removing it [readability-redundant-preprocessor]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous #ifndef was here
+void f();
+#endif
+#endif
+
+// Positive testing of inverted condition.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant #ifdef; consider removing it [readability-redundant-preprocessor]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous #ifndef was here
+void f2();
+#endif
+#endif
+
+// Negative testing.
+#include "readability-redundant-preprocessor.h"
+
+#ifndef BAR
+void g();
+#endif
+
+#ifndef FOO
+#ifndef BAR
+void h();
+#endif
+#endif
+
+#ifndef FOO
+#ifdef BAR
+void i();
+#endif
+#endif
+
+// Positive #if testing.
+#define FOO 4
+
+#if FOO == 4
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant #if; consider removing it [readability-redundant-preprocessor]
+#if FOO == 4
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous #if was here
+void j();
+#endif
+#endif
+
+#if FOO == 3 + 1
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant #if; consider removing it [readability-redundant-preprocessor]
+#if FOO == 3 + 1
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous #if was here
+void j();
+#endif
+#endif
+
+#if FOO == \
+4
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant #if; consider removing it [readability-redundant-preprocessor]
+#if FOO == \
+4
+// CHECK-NOTES: [[@LINE-5]]:2: note: previous #if was here
+void j();
+#endif
+#endif
+
+// Negative #if testing.
+#define BAR 4
+
+#if FOO == 4
+#if BAR == 4
+void k();
+#endif
+#endif
+
+#if FOO == \
+4
+#if BAR == \
+5
+void k();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
@@ -0,0 +1,36 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -DFOO
+
+// Positive testing.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant #ifdef; consider removing it [readability-redundant-preprocessor]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous #ifdef was here
+void f();
+#endif
+#endif
+
+// Positive testing of inverted condition.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant #ifndef; consider removing it [readability-redundant-preprocessor]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous #ifdef was here
+void f2();
+#endif
+#endif
+
+// Negative testing.
+#ifdef BAR
+void g();
+#endif
+
+#ifdef FOO
+#ifdef BAR
+void h();
+#endif
+#endif
+
+#ifdef FOO
+#ifndef BAR
+void i();
+#endif
+#endif
Index: docs/clang-tidy/checks/readability-redundant-preprocessor.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-preprocessor.rst
@@ -0,0 +1,61 @@
+.. title:: clang-tidy - readability-redundant-preprocessor
+
+readability-redundant-preprocessor
+==
+
+Finds potentially redundant preprocessor directives. At the moment the
+following cases are detected:
+
+* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the
+  same condition. For example:
+
+.. code-block:: c++
+
+  #ifdef FOO
+  #ifdef FOO // inner ifdef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* Same for `#ifndef` .. `#endif` pairs. For example:
+
+.. code-block:: c++
+
+  #ifndef FOO
+  #ifndef FOO // inner ifndef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* `#ifndef` inside an `#ifdef` with the same condition:
+
+.. code-block:: c++
+
+  #ifdef FOO
+  #ifndef FOO // inner ifndef is considered redundant
+  void f();
+  #

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos marked 2 inline comments as done.
vmiklos added inline comments.



Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:37
+CheckMacroRedundancy(Loc, Condition, IfStack,
+ "nested redundant if; consider removing it",
+ "previous if was here", true);

aaron.ballman wrote:
> I think these diagnostics should be hoisted as private constant members of 
> the class. Something like:
> `nested redundant %select{#if|#ifdef|#ifndef}0; consider removing it"` and 
> `previous %select{#if|#ifdef|#ifndef}0 here`
Done, I've also added an enum to specify if/ifdef/ifndef to avoid hard-to-read 
0/1/2 for these.


https://reviews.llvm.org/D54349



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos marked an inline comment as done.
vmiklos added inline comments.



Comment at: test/clang-tidy/readability-redundant-preprocessor.cpp:53
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant #if; consider 
removing it [readability-redundant-preprocessor]
+#if FOO == 3 + 1
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous #if was here

aaron.ballman wrote:
> I didn't describe my test scenario very well -- can you change this line to 
> FOO == 4 but leave the preceding conditional check as FOO == 3 + 1? The goal 
> is to test that this isn't a token-by-token check, but uses the symbolic 
> values instead.
That doesn't work at the moment, since `PreprocessorEntry::Condition` is just a 
string, so `3 + 1` won't equal to `4`. I think we discussed this above already, 
and you said:

> I think it requires manual work. There's a FIXME in `PPCallbacks::If()` to 
> pass a list/tree of tokens that would make implementing this reasonable. I'd 
> say it's fine as a follow-up patch.

But later you wrote:

> I agree that it's more complex, but that's why I was asking for it -- I don't 
> think the design you have currently will extend for these sort of cases, and 
> I was hoping to cover more utility with the check to hopefully shake out 
> those forward-looking design considerations. As it stands, I feel like this 
> check is a bit too simplistic to have much value, but if you covered some of 
> these other cases, it would alleviate that worry for me.

My hope is that the check with its current "feature set" already has some 
value, but let me know if I'm too optimistic. :-)

That being said, if I want to support simple cases like "realize that `3 + 1` 
and `4` is the same" -- do you imagine that would be manually handled in this 
check or there is already some reusable building block in the codebase to 
evaluate if two expressions have the same integer value? I guess doing this 
manually is a lot of work, e.g. the answer for `FOO + 4` would be "it depends", 
so that should not equal to `8`, even if FOO happens to be `4`, etc.


https://reviews.llvm.org/D54349



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

In https://reviews.llvm.org/D54349#1301663, @aaron.ballman wrote:

> I think this check is in good shape for the initial commit. Additional 
> functionality can be added incrementally.


OK, thanks. I'll lcommit this once https://reviews.llvm.org/D54450 is committed.


https://reviews.llvm.org/D54349



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

[PATCH] D54450: Get the correct range of tokens for preprocessor conditions

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

Could you please review this at some stage? As mentioned previously, this is a 
dependency for D54349 . Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54450/new/

https://reviews.llvm.org/D54450



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

[PATCH] D54450: Get the correct range of tokens for preprocessor conditions

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

Next ping -- could you please review this one? Thanks! :-)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54450/new/

https://reviews.llvm.org/D54450



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

[PATCH] D37634: clang-rename: let -force handle multiple renames

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos created this revision.

The use case is that renaming multiple symbols in a large enough codebase is
much faster if all of these can be done with a single invocation, but
there will be multiple translation units where one or more symbols are
not found.

  

Old behavior was to exit with an error (default) or exit without
reporting an error (-force). New behavior is that -force results in a
best-effort rename: rename symbols which are found and just ignore the
rest.

  

The existing help for -force sort of already implies this behavior.


https://reviews.llvm.org/D37634

Files:
  lib/Tooling/Refactoring/Rename/RenamingAction.cpp
  lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
  test/clang-rename/ForceMulti.cpp
  tools/clang-rename/ClangRename.cpp


Index: tools/clang-rename/ClangRename.cpp
===
--- tools/clang-rename/ClangRename.cpp
+++ tools/clang-rename/ClangRename.cpp
@@ -175,12 +175,6 @@
 return 1;
   }
 
-  if (Force && PrevNames.size() < NewNames.size()) {
-// No matching PrevName for all NewNames. Without Force this is an error
-// above already.
-return 0;
-  }
-
   // Perform the renaming.
   tooling::RenamingAction RenameAction(NewNames, PrevNames, USRList,
Tool.getReplacements(), PrintLocations);
Index: test/clang-rename/ForceMulti.cpp
===
--- /dev/null
+++ test/clang-rename/ForceMulti.cpp
@@ -0,0 +1,8 @@
+class B /* Test 1 */ { // CHECK: class B2 /* Test 1 */ {
+};
+
+class D : public B /* Test 1 */ { // CHECK: class D : public B2 /* Test 1 */ {
+};
+
+// Test 1.
+// RUN: clang-rename -force -qualified-name B -new-name B2 -qualified-name E 
-new-name E2 %s -- | sed 's,//.*,,' | FileCheck %s
Index: lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
===
--- lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
+++ lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
@@ -198,8 +198,11 @@
 return false;
   }
 
-  if (Force)
+  if (Force) {
+SpellingNames.push_back(std::string());
+USRList.push_back(std::vector());
 return true;
+  }
 
   unsigned CouldNotFindSymbolNamed = Engine.getCustomDiagID(
   DiagnosticsEngine::Error, "clang-rename could not find symbol %0");
Index: lib/Tooling/Refactoring/Rename/RenamingAction.cpp
===
--- lib/Tooling/Refactoring/Rename/RenamingAction.cpp
+++ lib/Tooling/Refactoring/Rename/RenamingAction.cpp
@@ -84,8 +84,13 @@
 FileToReplaces(FileToReplaces), PrintLocations(PrintLocations) {}
 
   void HandleTranslationUnit(ASTContext &Context) override {
-for (unsigned I = 0; I < NewNames.size(); ++I)
+for (unsigned I = 0; I < NewNames.size(); ++I) {
+  // If the previous name was not found, ignore this rename request.
+  if (PrevNames[I].empty())
+continue;
+
   HandleOneRename(Context, NewNames[I], PrevNames[I], USRList[I]);
+}
   }
 
   void HandleOneRename(ASTContext &Context, const std::string &NewName,


Index: tools/clang-rename/ClangRename.cpp
===
--- tools/clang-rename/ClangRename.cpp
+++ tools/clang-rename/ClangRename.cpp
@@ -175,12 +175,6 @@
 return 1;
   }
 
-  if (Force && PrevNames.size() < NewNames.size()) {
-// No matching PrevName for all NewNames. Without Force this is an error
-// above already.
-return 0;
-  }
-
   // Perform the renaming.
   tooling::RenamingAction RenameAction(NewNames, PrevNames, USRList,
Tool.getReplacements(), PrintLocations);
Index: test/clang-rename/ForceMulti.cpp
===
--- /dev/null
+++ test/clang-rename/ForceMulti.cpp
@@ -0,0 +1,8 @@
+class B /* Test 1 */ { // CHECK: class B2 /* Test 1 */ {
+};
+
+class D : public B /* Test 1 */ { // CHECK: class D : public B2 /* Test 1 */ {
+};
+
+// Test 1.
+// RUN: clang-rename -force -qualified-name B -new-name B2 -qualified-name E -new-name E2 %s -- | sed 's,//.*,,' | FileCheck %s
Index: lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
===
--- lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
+++ lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
@@ -198,8 +198,11 @@
 return false;
   }
 
-  if (Force)
+  if (Force) {
+SpellingNames.push_back(std::string());
+USRList.push_back(std::vector());
 return true;
+  }
 
   unsigned CouldNotFindSymbolNamed = Engine.getCustomDiagID(
   DiagnosticsEngine::Error, "clang-rename could not find symbol %0");
Index: lib/Tooling/Refactoring/Rename/RenamingAction.cpp

[PATCH] D37634: clang-rename: let -force handle multiple renames

[Miklos Vajna via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL312942: clang-rename: let -force handle multiple renames 
(authored by vmiklos).

Changed prior to commit:
  https://reviews.llvm.org/D37634?vs=114402&id=114663#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37634

Files:
  cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
  cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
  cfe/trunk/test/clang-rename/ForceMulti.cpp
  cfe/trunk/tools/clang-rename/ClangRename.cpp


Index: cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
@@ -198,8 +198,11 @@
 return false;
   }
 
-  if (Force)
+  if (Force) {
+SpellingNames.push_back(std::string());
+USRList.push_back(std::vector());
 return true;
+  }
 
   unsigned CouldNotFindSymbolNamed = Engine.getCustomDiagID(
   DiagnosticsEngine::Error, "clang-rename could not find symbol %0");
Index: cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
@@ -84,8 +84,13 @@
 FileToReplaces(FileToReplaces), PrintLocations(PrintLocations) {}
 
   void HandleTranslationUnit(ASTContext &Context) override {
-for (unsigned I = 0; I < NewNames.size(); ++I)
+for (unsigned I = 0; I < NewNames.size(); ++I) {
+  // If the previous name was not found, ignore this rename request.
+  if (PrevNames[I].empty())
+continue;
+
   HandleOneRename(Context, NewNames[I], PrevNames[I], USRList[I]);
+}
   }
 
   void HandleOneRename(ASTContext &Context, const std::string &NewName,
Index: cfe/trunk/tools/clang-rename/ClangRename.cpp
===
--- cfe/trunk/tools/clang-rename/ClangRename.cpp
+++ cfe/trunk/tools/clang-rename/ClangRename.cpp
@@ -175,12 +175,6 @@
 return 1;
   }
 
-  if (Force && PrevNames.size() < NewNames.size()) {
-// No matching PrevName for all NewNames. Without Force this is an error
-// above already.
-return 0;
-  }
-
   // Perform the renaming.
   tooling::RenamingAction RenameAction(NewNames, PrevNames, USRList,
Tool.getReplacements(), PrintLocations);
Index: cfe/trunk/test/clang-rename/ForceMulti.cpp
===
--- cfe/trunk/test/clang-rename/ForceMulti.cpp
+++ cfe/trunk/test/clang-rename/ForceMulti.cpp
@@ -0,0 +1,8 @@
+class B /* Test 1 */ { // CHECK: class B2 /* Test 1 */ {
+};
+
+class D : public B /* Test 1 */ { // CHECK: class D : public B2 /* Test 1 */ {
+};
+
+// Test 1.
+// RUN: clang-rename -force -qualified-name B -new-name B2 -qualified-name E 
-new-name E2 %s -- | sed 's,//.*,,' | FileCheck %s


Index: cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
@@ -198,8 +198,11 @@
 return false;
   }
 
-  if (Force)
+  if (Force) {
+SpellingNames.push_back(std::string());
+USRList.push_back(std::vector());
 return true;
+  }
 
   unsigned CouldNotFindSymbolNamed = Engine.getCustomDiagID(
   DiagnosticsEngine::Error, "clang-rename could not find symbol %0");
Index: cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
@@ -84,8 +84,13 @@
 FileToReplaces(FileToReplaces), PrintLocations(PrintLocations) {}
 
   void HandleTranslationUnit(ASTContext &Context) override {
-for (unsigned I = 0; I < NewNames.size(); ++I)
+for (unsigned I = 0; I < NewNames.size(); ++I) {
+  // If the previous name was not found, ignore this rename request.
+  if (PrevNames[I].empty())
+continue;
+
   HandleOneRename(Context, NewNames[I], PrevNames[I], USRList[I]);
+}
   }
 
   void HandleOneRename(ASTContext &Context, const std::string &NewName,
Index: cfe/trunk/tools/clang-rename/ClangRename.cpp
===
--- cfe/trunk/tools/clang-rename/ClangRename.cpp
+++ cfe/trunk/tools/clang-rename/ClangRename.cpp
@@ -175,12 +175,6 @@
 return 1;
   }
 
-  if (Force && PrevNames.size() < NewNames.size()) {
-// No matching PrevName for all NewNames. Without Force this is an error
-// above already.
-retur

[PATCH] D63621: [git-clang-format] recognize hxx as a C++ file

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos created this revision.
vmiklos added reviewers: jbcoe, djasper.
Herald added subscribers: cfe-commits, kadircet, ilya-biryukov.
Herald added a project: clang.

clangd, clang-tidy, etc does that already, no reason why git-clang-format 
should skip hxx files.


Repository:
  rC Clang

https://reviews.llvm.org/D63621

Files:
  clang/tools/clang-format/git-clang-format


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -77,7 +77,7 @@
   'c', 'h',  # C
   'm',  # ObjC
   'mm',  # ObjC++
-  'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp',  # C++
+  'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp', 'hxx',  # C++
   'cu',  # CUDA
   # Other languages that clang-format supports
   'proto', 'protodevel',  # Protocol Buffers


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -77,7 +77,7 @@
   'c', 'h',  # C
   'm',  # ObjC
   'mm',  # ObjC++
-  'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp',  # C++
+  'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp', 'hxx',  # C++
   'cu',  # CUDA
   # Other languages that clang-format supports
   'proto', 'protodevel',  # Protocol Buffers
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63621: [git-clang-format] recognize hxx as a C++ file

[Miklos Vajna via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364014: [git-clang-format] recognize hxx as a C++ file 
(authored by vmiklos, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63621?vs=205884&id=205953#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63621/new/

https://reviews.llvm.org/D63621

Files:
  cfe/trunk/tools/clang-format/git-clang-format


Index: cfe/trunk/tools/clang-format/git-clang-format
===
--- cfe/trunk/tools/clang-format/git-clang-format
+++ cfe/trunk/tools/clang-format/git-clang-format
@@ -77,7 +77,7 @@
   'c', 'h',  # C
   'm',  # ObjC
   'mm',  # ObjC++
-  'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp',  # C++
+  'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp', 'hxx',  # C++
   'cu',  # CUDA
   # Other languages that clang-format supports
   'proto', 'protodevel',  # Protocol Buffers


Index: cfe/trunk/tools/clang-format/git-clang-format
===
--- cfe/trunk/tools/clang-format/git-clang-format
+++ cfe/trunk/tools/clang-format/git-clang-format
@@ -77,7 +77,7 @@
   'c', 'h',  # C
   'm',  # ObjC
   'mm',  # ObjC++
-  'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp',  # C++
+  'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp', 'hxx',  # C++
   'cu',  # CUDA
   # Other languages that clang-format supports
   'proto', 'protodevel',  # Protocol Buffers
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63621: [git-clang-format] recognize hxx as a C++ file

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

Thanks!


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63621/new/

https://reviews.llvm.org/D63621



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

[PATCH] D54450: Get the correct range of tokens for preprocessor conditions

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

Ping, could this be reviewed, please? :-) Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54450/new/

https://reviews.llvm.org/D54450



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

[PATCH] D56025: [clang-tidy] add IgnoreMacros option to readability-uppercase-literal-suffix

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos created this revision.
vmiklos added reviewers: JonasToth, alexfh, lebedev.ri.
Herald added subscribers: cfe-commits, xazax.hun.

And also enable it by default to be consistent with e.g. modernize-use-using.

This helps e.g. when running this check on client code where the macro is 
provided by the system, so there is no easy way to modify it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56025

Files:
  clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
  clang-tidy/readability/UppercaseLiteralSuffixCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
  test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp


Index: test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
===
--- test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
+++ test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I 
%S
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- \
+// RUN:   -config="{CheckOptions: [{key: 
readability-uppercase-literal-suffix.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -I %S
 
 void macros() {
 #define INMACRO(X) 1.f
Index: docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
===
--- docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
+++ docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
@@ -49,3 +49,8 @@
 * ``uL`` will be kept as is.
 * ``ull`` will be kept as is, since it is not in the list
 * and so on.
+
+.. option:: IgnoreMacros
+
+   If this option is set to non-zero (default is `1`), the check will not warn
+   about literal suffixes inside macros.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -236,6 +236,10 @@
   ` check does not warn
   about calls inside macros anymore by default.
 
+- The :doc:`readability-uppercase-literal-suffix
+  ` check does not warn
+  about literal suffixes inside macros anymore by default.
+
 - The :doc:`cppcoreguidelines-narrowing-conversions
   ` check now
   detects more narrowing conversions:
Index: clang-tidy/readability/UppercaseLiteralSuffixCheck.h
===
--- clang-tidy/readability/UppercaseLiteralSuffixCheck.h
+++ clang-tidy/readability/UppercaseLiteralSuffixCheck.h
@@ -35,6 +35,7 @@
   bool checkBoundMatch(const ast_matchers::MatchFinder::MatchResult &Result);
 
   const std::vector NewSuffixes;
+  const bool IgnoreMacros;
 };
 
 } // namespace readability
Index: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
===
--- clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
+++ clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
@@ -183,12 +183,14 @@
 StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   NewSuffixes(
-  utils::options::parseStringList(Options.get("NewSuffixes", ""))) {}
+  utils::options::parseStringList(Options.get("NewSuffixes", ""))),
+  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
 
 void UppercaseLiteralSuffixCheck::storeOptions(
 ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "NewSuffixes",
 utils::options::serializeStringList(NewSuffixes));
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
 }
 
 void UppercaseLiteralSuffixCheck::registerMatchers(MatchFinder *Finder) {
@@ -216,6 +218,8 @@
   // We might have a suffix that is already uppercase.
   if (auto Details = shouldReplaceLiteralSuffix(
   *Literal, NewSuffixes, *Result.SourceManager, getLangOpts())) {
+if (Details->LiteralLocation.getBegin().isMacroID() && IgnoreMacros)
+  return true;
 auto Complaint = diag(Details->LiteralLocation.getBegin(),
   "%0 literal has suffix '%1', which is not uppercase")
  << LiteralType::Name << Details->OldSuffix;


Index: test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
===
--- test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
+++ test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I %S
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-uppercase-literal-suffix.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -I %S
 
 void macros() {
 #define INMACRO(X) 1.f
Index: docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
==

[PATCH] D56025: [clang-tidy] add IgnoreMacros option to readability-uppercase-literal-suffix

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos updated this revision to Diff 179371.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56025/new/

https://reviews.llvm.org/D56025

Files:
  clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
  clang-tidy/readability/UppercaseLiteralSuffixCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
  test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
  test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp


Index: test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
===
--- test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
+++ test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
@@ -235,6 +235,10 @@
   // CHECK-FIXES: static constexpr auto m0 = PASSTHROUGH(1U);
   static_assert(is_same::value, "");
   static_assert(m0 == 1, "");
+
+  // This location is inside a macro, no warning on that by default.
+#define MACRO 1u
+  int foo = MACRO;
 }
 
 // Check that user-defined literals do not cause any diags.
Index: test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
===
--- test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
+++ test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I 
%S
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- \
+// RUN:   -config="{CheckOptions: [{key: 
readability-uppercase-literal-suffix.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -I %S
 
 void macros() {
 #define INMACRO(X) 1.f
Index: docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
===
--- docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
+++ docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
@@ -49,3 +49,8 @@
 * ``uL`` will be kept as is.
 * ``ull`` will be kept as is, since it is not in the list
 * and so on.
+
+.. option:: IgnoreMacros
+
+   If this option is set to non-zero (default is `1`), the check will not warn
+   about literal suffixes inside macros.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -236,6 +236,10 @@
   ` check does not warn
   about calls inside macros anymore by default.
 
+- The :doc:`readability-uppercase-literal-suffix
+  ` check does not warn
+  about literal suffixes inside macros anymore by default.
+
 - The :doc:`cppcoreguidelines-narrowing-conversions
   ` check now
   detects more narrowing conversions:
Index: clang-tidy/readability/UppercaseLiteralSuffixCheck.h
===
--- clang-tidy/readability/UppercaseLiteralSuffixCheck.h
+++ clang-tidy/readability/UppercaseLiteralSuffixCheck.h
@@ -35,6 +35,7 @@
   bool checkBoundMatch(const ast_matchers::MatchFinder::MatchResult &Result);
 
   const std::vector NewSuffixes;
+  const bool IgnoreMacros;
 };
 
 } // namespace readability
Index: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
===
--- clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
+++ clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
@@ -183,12 +183,14 @@
 StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   NewSuffixes(
-  utils::options::parseStringList(Options.get("NewSuffixes", ""))) {}
+  utils::options::parseStringList(Options.get("NewSuffixes", ""))),
+  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
 
 void UppercaseLiteralSuffixCheck::storeOptions(
 ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "NewSuffixes",
 utils::options::serializeStringList(NewSuffixes));
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
 }
 
 void UppercaseLiteralSuffixCheck::registerMatchers(MatchFinder *Finder) {
@@ -216,6 +218,8 @@
   // We might have a suffix that is already uppercase.
   if (auto Details = shouldReplaceLiteralSuffix(
   *Literal, NewSuffixes, *Result.SourceManager, getLangOpts())) {
+if (Details->LiteralLocation.getBegin().isMacroID() && IgnoreMacros)
+  return true;
 auto Complaint = diag(Details->LiteralLocation.getBegin(),
   "%0 literal has suffix '%1', which is not uppercase")
  << LiteralType::Name << Details->OldSuffix;


Index: test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
===
--- test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
+++ test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
@@ -235,6 +235,10 @@
   // CHECK-FIXES: static constex

[PATCH] D56025: [clang-tidy] add IgnoreMacros option to readability-uppercase-literal-suffix

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos marked 2 inline comments as done.
vmiklos added inline comments.



Comment at: 
test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp:2-3
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- \
+// RUN:   -config="{CheckOptions: [{key: 
readability-uppercase-literal-suffix.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -I %S
 

lebedev.ri wrote:
> And the positive test?
Added a positive test now. :-)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56025/new/

https://reviews.llvm.org/D56025



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

[PATCH] D56025: [clang-tidy] add IgnoreMacros option to readability-uppercase-literal-suffix

[Miklos Vajna via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
vmiklos marked an inline comment as done.
Closed by commit rL350056: [clang-tidy] add IgnoreMacros option to 
readability-uppercase-literal-suffix (authored by vmiklos, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D56025?vs=179371&id=179476#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56025/new/

https://reviews.llvm.org/D56025

Files:
  clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
  clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
  
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
  
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp


Index: 
clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
===
--- 
clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
+++ 
clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
@@ -183,12 +183,14 @@
 StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   NewSuffixes(
-  utils::options::parseStringList(Options.get("NewSuffixes", ""))) {}
+  utils::options::parseStringList(Options.get("NewSuffixes", ""))),
+  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
 
 void UppercaseLiteralSuffixCheck::storeOptions(
 ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "NewSuffixes",
 utils::options::serializeStringList(NewSuffixes));
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
 }
 
 void UppercaseLiteralSuffixCheck::registerMatchers(MatchFinder *Finder) {
@@ -216,6 +218,8 @@
   // We might have a suffix that is already uppercase.
   if (auto Details = shouldReplaceLiteralSuffix(
   *Literal, NewSuffixes, *Result.SourceManager, getLangOpts())) {
+if (Details->LiteralLocation.getBegin().isMacroID() && IgnoreMacros)
+  return true;
 auto Complaint = diag(Details->LiteralLocation.getBegin(),
   "%0 literal has suffix '%1', which is not uppercase")
  << LiteralType::Name << Details->OldSuffix;
Index: 
clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.h
+++ clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.h
@@ -35,6 +35,7 @@
   bool checkBoundMatch(const ast_matchers::MatchFinder::MatchResult &Result);
 
   const std::vector NewSuffixes;
+  const bool IgnoreMacros;
 };
 
 } // namespace readability
Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst
===
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst
@@ -236,6 +236,10 @@
   ` check does not warn
   about calls inside macros anymore by default.
 
+- The :doc:`readability-uppercase-literal-suffix
+  ` check does not warn
+  about literal suffixes inside macros anymore by default.
+
 - The :doc:`cppcoreguidelines-narrowing-conversions
   ` check now
   detects more narrowing conversions:
Index: 
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
===
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
@@ -49,3 +49,8 @@
 * ``uL`` will be kept as is.
 * ``ull`` will be kept as is, since it is not in the list
 * and so on.
+
+.. option:: IgnoreMacros
+
+   If this option is set to non-zero (default is `1`), the check will not warn
+   about literal suffixes inside macros.
Index: 
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
===
--- 
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
+++ 
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I 
%S
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- \
+// RUN:   -config="{CheckOptions: [{key: 
readability-uppercase-literal-suffix.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -I %S
 
 void macros() {
 #define INMACRO(X) 1.f
Index: 
clang-tools-extra/trunk/test/clang-tidy/readability-upp

[PATCH] D56025: [clang-tidy] add IgnoreMacros option to readability-uppercase-literal-suffix

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

In D56025#1340618 , @JonasToth wrote:

> LGTM, do you have commit rights?


Yes, thanks. Committed now.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56025/new/

https://reviews.llvm.org/D56025



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

[PATCH] D54450: Get the correct range of tokens for preprocessor conditions

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

Could you please review this one? It would be especially helpful, due to the 
depending other review. Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54450/new/

https://reviews.llvm.org/D54450



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

[PATCH] D54450: Get the correct range of tokens for preprocessor conditions

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

@aaron.ballman do you wait for someone else to commit this? Thanks.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54450/new/

https://reviews.llvm.org/D54450



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

[PATCH] D87931: [clang-format] Recognize "hxx" as a C++ header in clang-format-diff.py

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
vmiklos requested review of this revision.

And shift "proto" to the next line to avoid a too long line.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87931

Files:
  clang/tools/clang-format/clang-format-diff.py


Index: clang/tools/clang-format/clang-format-diff.py
===
--- clang/tools/clang-format/clang-format-diff.py
+++ clang/tools/clang-format/clang-format-diff.py
@@ -47,8 +47,8 @@
   help='custom pattern selecting file paths to reformat '
   '(case sensitive, overrides -iregex)')
   parser.add_argument('-iregex', metavar='PATTERN', default=
-  
r'.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hh|hpp|m|mm|inc|js|ts|proto'
-  r'|protodevel|java|cs)',
+  r'.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hh|hpp|hxx|m|mm|inc|js|ts'
+  r'|proto|protodevel|java|cs)',
   help='custom pattern selecting file paths to reformat '
   '(case insensitive, overridden by -regex)')
   parser.add_argument('-sort-includes', action='store_true', default=False,


Index: clang/tools/clang-format/clang-format-diff.py
===
--- clang/tools/clang-format/clang-format-diff.py
+++ clang/tools/clang-format/clang-format-diff.py
@@ -47,8 +47,8 @@
   help='custom pattern selecting file paths to reformat '
   '(case sensitive, overrides -iregex)')
   parser.add_argument('-iregex', metavar='PATTERN', default=
-  r'.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hh|hpp|m|mm|inc|js|ts|proto'
-  r'|protodevel|java|cs)',
+  r'.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hh|hpp|hxx|m|mm|inc|js|ts'
+  r'|proto|protodevel|java|cs)',
   help='custom pattern selecting file paths to reformat '
   '(case insensitive, overridden by -regex)')
   parser.add_argument('-sort-includes', action='store_true', default=False,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87931: [clang-format] Recognize "hxx" as a C++ header in clang-format-diff.py

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

Mydeveloperday, Sam: could you please take a look? Hopefully a trivial one. 
Thanks :-)

There are some larger codebases like LibreOffice that use .hxx as their header 
file extension, it's a bit inconsistent that .cxx is recognized but .hxx is 
not. This is meant to fix the issue.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87931/new/

https://reviews.llvm.org/D87931

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

[PATCH] D87931: [clang-format] Recognize "hxx" as a C++ header in clang-format-diff.py

[Miklos Vajna via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb168bbfae42e: [clang-format] Recognize "hxx" as a 
C++ header in clang-format-diff.py (authored by vmiklos).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87931/new/

https://reviews.llvm.org/D87931

Files:
  clang/tools/clang-format/clang-format-diff.py


Index: clang/tools/clang-format/clang-format-diff.py
===
--- clang/tools/clang-format/clang-format-diff.py
+++ clang/tools/clang-format/clang-format-diff.py
@@ -47,8 +47,8 @@
   help='custom pattern selecting file paths to reformat '
   '(case sensitive, overrides -iregex)')
   parser.add_argument('-iregex', metavar='PATTERN', default=
-  
r'.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hh|hpp|m|mm|inc|js|ts|proto'
-  r'|protodevel|java|cs)',
+  r'.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hh|hpp|hxx|m|mm|inc|js|ts'
+  r'|proto|protodevel|java|cs)',
   help='custom pattern selecting file paths to reformat '
   '(case insensitive, overridden by -regex)')
   parser.add_argument('-sort-includes', action='store_true', default=False,


Index: clang/tools/clang-format/clang-format-diff.py
===
--- clang/tools/clang-format/clang-format-diff.py
+++ clang/tools/clang-format/clang-format-diff.py
@@ -47,8 +47,8 @@
   help='custom pattern selecting file paths to reformat '
   '(case sensitive, overrides -iregex)')
   parser.add_argument('-iregex', metavar='PATTERN', default=
-  r'.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hh|hpp|m|mm|inc|js|ts|proto'
-  r'|protodevel|java|cs)',
+  r'.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hh|hpp|hxx|m|mm|inc|js|ts'
+  r'|proto|protodevel|java|cs)',
   help='custom pattern selecting file paths to reformat '
   '(case insensitive, overridden by -regex)')
   parser.add_argument('-sort-includes', action='store_true', default=False,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87931: [clang-format] Recognize "hxx" as a C++ header in clang-format-diff.py

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

Thanks for the quick feedback!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87931/new/

https://reviews.llvm.org/D87931

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

[PATCH] D148439: [clang-rename] Exit gracefully when no input provided

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added subscribers: arphaman, klimek.
vmiklos added a comment.

Looks good to me, but probably you want an approval from @arphaman, @klimek or 
@kbobyrev before committing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148439/new/

https://reviews.llvm.org/D148439

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

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added inline comments.



Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.h:20
+///\brief Warns about using throw in function declared as noexcept.
+/// It complains about every throw, even if it is caught later.
+class ThrowWithNoexceptCheck : public ClangTidyCheck {

Prazek wrote:
> aaron.ballman wrote:
> > What is the false positive rate with this check over a large codebase that 
> > uses exceptions?
> Do you know any good project to check it?
LibreOffice might be a candidate, see 
 for details on how 
to generate a compile database for it (since it does not use cmake), then you 
can start testing.


https://reviews.llvm.org/D19201



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

[PATCH] D32945: clang-tidy: add IgnoreMacros option to modernize-use-default-member-init

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

In https://reviews.llvm.org/D32945#748135, @malcolm.parsons wrote:

> Should this option be shared?


If the alternative is to make this a general clang-tidy option where all but 
these two checks ignore it, I think that's more confusing to the users. Or do 
you have an example option in mind that's shared only between a few checks?


https://reviews.llvm.org/D32945



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

[PATCH] D32945: clang-tidy: add IgnoreMacros option to modernize-use-default-member-init

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos updated this revision to Diff 98108.

https://reviews.llvm.org/D32945

Files:
  clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
  clang-tidy/modernize/UseDefaultMemberInitCheck.h
  docs/clang-tidy/checks/modernize-use-default-member-init.rst
  test/clang-tidy/modernize-use-default-member-init-macros.cpp
  test/clang-tidy/modernize-use-default-member-init.cpp


Index: test/clang-tidy/modernize-use-default-member-init.cpp
===
--- test/clang-tidy/modernize-use-default-member-init.cpp
+++ test/clang-tidy/modernize-use-default-member-init.cpp
@@ -380,3 +380,12 @@
 
 NegativeTemplateExisting ntei(0);
 NegativeTemplateExisting nted(0);
+
+// This resulted in a warning by default.
+#define MACRO() \
+  struct MacroS { \
+void *P; \
+MacroS() : P(nullptr) {} \
+  };
+
+MACRO();
Index: test/clang-tidy/modernize-use-default-member-init-macros.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-default-member-init-macros.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s modernize-use-default-member-init %t -- \
+// RUN:   -config="{CheckOptions: [{key: 
modernize-use-default-member-init.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -std=c++11
+
+#define MACRO() \
+  struct S { \
+void *P; \
+S() : P(nullptr) {} \
+  };
+
+MACRO();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use default member initializer for 
'P'
+
+struct S2 {
+  void *P;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use default member initializer 
for 'P'
+  S2() : P(nullptr) {}
+};
Index: docs/clang-tidy/checks/modernize-use-default-member-init.rst
===
--- docs/clang-tidy/checks/modernize-use-default-member-init.rst
+++ docs/clang-tidy/checks/modernize-use-default-member-init.rst
@@ -47,3 +47,8 @@
 int i = 5;
 double j = 10.0;
   };
+
+.. option:: IgnoreMacros
+
+   If this option is set to non-zero (default is `1`), the check will not warn
+   about members declared inside macros.
Index: clang-tidy/modernize/UseDefaultMemberInitCheck.h
===
--- clang-tidy/modernize/UseDefaultMemberInitCheck.h
+++ clang-tidy/modernize/UseDefaultMemberInitCheck.h
@@ -36,6 +36,7 @@
  const CXXCtorInitializer *Init);
 
   const bool UseAssignment;
+  const bool IgnoreMacros;
 };
 
 } // namespace modernize
Index: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -139,11 +139,13 @@
 UseDefaultMemberInitCheck::UseDefaultMemberInitCheck(StringRef Name,
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  UseAssignment(Options.get("UseAssignment", 0) != 0) {}
+  UseAssignment(Options.get("UseAssignment", 0) != 0),
+  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
 
 void UseDefaultMemberInitCheck::storeOptions(
 ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "UseAssignment", UseAssignment);
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
 }
 
 void UseDefaultMemberInitCheck::registerMatchers(MatchFinder *Finder) {
@@ -197,6 +199,10 @@
 const MatchFinder::MatchResult &Result, const CXXCtorInitializer *Init) {
   const FieldDecl *Field = Init->getMember();
 
+  SourceLocation StartLoc = Field->getLocStart();
+  if (StartLoc.isMacroID() && IgnoreMacros)
+return;
+
   SourceLocation FieldEnd =
   Lexer::getLocForEndOfToken(Field->getSourceRange().getEnd(), 0,
  *Result.SourceManager, getLangOpts());


Index: test/clang-tidy/modernize-use-default-member-init.cpp
===
--- test/clang-tidy/modernize-use-default-member-init.cpp
+++ test/clang-tidy/modernize-use-default-member-init.cpp
@@ -380,3 +380,12 @@
 
 NegativeTemplateExisting ntei(0);
 NegativeTemplateExisting nted(0);
+
+// This resulted in a warning by default.
+#define MACRO() \
+  struct MacroS { \
+void *P; \
+MacroS() : P(nullptr) {} \
+  };
+
+MACRO();
Index: test/clang-tidy/modernize-use-default-member-init-macros.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-default-member-init-macros.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s modernize-use-default-member-init %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-default-member-init.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -std=c++11
+
+#define MACRO() \
+  struct S { \
+void *P; \
+S() : P(nullptr) {} \
+  };
+
+MACRO();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use default member initializer for 'P'
+
+struct S2 {
+  void *P;
+  // CHEC

[PATCH] D32945: clang-tidy: add IgnoreMacros option to modernize-use-default-member-init

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

In https://reviews.llvm.org/D32945#748225, @malcolm.parsons wrote:

> See uses of `OptionsView::getLocalOrGlobal()`.


Ah, I see. Then yes, I think it makes sense; e.g. for the above cppunit case 
ideally I want to avoid any kind of warning from macros, since none of them can 
be easily fixed in system headers. (And the other way around, if system headers 
are not a problem, then all kind of warnings inside macros are probably 
interesting.) The updated patch uses `getLocalOrGlobal()` for the new option.


https://reviews.llvm.org/D32945



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

[PATCH] D32945: clang-tidy: add IgnoreMacros option to modernize-use-default-member-init

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos updated this revision to Diff 98163.

https://reviews.llvm.org/D32945

Files:
  clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
  clang-tidy/modernize/UseDefaultMemberInitCheck.h
  clang-tidy/modernize/UseUsingCheck.cpp
  docs/clang-tidy/checks/modernize-use-default-member-init.rst
  test/clang-tidy/modernize-use-default-member-init-macros.cpp
  test/clang-tidy/modernize-use-default-member-init.cpp

Index: test/clang-tidy/modernize-use-default-member-init.cpp
===
--- test/clang-tidy/modernize-use-default-member-init.cpp
+++ test/clang-tidy/modernize-use-default-member-init.cpp
@@ -380,3 +380,12 @@
 
 NegativeTemplateExisting ntei(0);
 NegativeTemplateExisting nted(0);
+
+// This resulted in a warning by default.
+#define MACRO() \
+  struct MacroS { \
+void *P; \
+MacroS() : P(nullptr) {} \
+  };
+
+MACRO();
Index: test/clang-tidy/modernize-use-default-member-init-macros.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-default-member-init-macros.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s modernize-use-default-member-init %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-default-member-init.IgnoreMacros, value: 0}]}" \
+// RUN:   -- -std=c++11
+
+#define MACRO() \
+  struct S { \
+void *P; \
+S() : P(nullptr) {} \
+  };
+
+MACRO();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use default member initializer for 'P'
+
+struct S2 {
+  void *P;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use default member initializer for 'P'
+  S2() : P(nullptr) {}
+};
Index: docs/clang-tidy/checks/modernize-use-default-member-init.rst
===
--- docs/clang-tidy/checks/modernize-use-default-member-init.rst
+++ docs/clang-tidy/checks/modernize-use-default-member-init.rst
@@ -47,3 +47,8 @@
 int i = 5;
 double j = 10.0;
   };
+
+.. option:: IgnoreMacros
+
+   If this option is set to non-zero (default is `1`), the check will not warn
+   about members declared inside macros.
Index: clang-tidy/modernize/UseUsingCheck.cpp
===
--- clang-tidy/modernize/UseUsingCheck.cpp
+++ clang-tidy/modernize/UseUsingCheck.cpp
@@ -19,7 +19,7 @@
 
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  IgnoreMacros(Options.get("IgnoreMacros", true)) {}
+  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
 
 void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus11)
Index: clang-tidy/modernize/UseDefaultMemberInitCheck.h
===
--- clang-tidy/modernize/UseDefaultMemberInitCheck.h
+++ clang-tidy/modernize/UseDefaultMemberInitCheck.h
@@ -36,6 +36,7 @@
  const CXXCtorInitializer *Init);
 
   const bool UseAssignment;
+  const bool IgnoreMacros;
 };
 
 } // namespace modernize
Index: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -139,11 +139,13 @@
 UseDefaultMemberInitCheck::UseDefaultMemberInitCheck(StringRef Name,
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  UseAssignment(Options.get("UseAssignment", 0) != 0) {}
+  UseAssignment(Options.get("UseAssignment", 0) != 0),
+  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
 
 void UseDefaultMemberInitCheck::storeOptions(
 ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "UseAssignment", UseAssignment);
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
 }
 
 void UseDefaultMemberInitCheck::registerMatchers(MatchFinder *Finder) {
@@ -197,6 +199,10 @@
 const MatchFinder::MatchResult &Result, const CXXCtorInitializer *Init) {
   const FieldDecl *Field = Init->getMember();
 
+  SourceLocation StartLoc = Field->getLocStart();
+  if (StartLoc.isMacroID() && IgnoreMacros)
+return;
+
   SourceLocation FieldEnd =
   Lexer::getLocForEndOfToken(Field->getSourceRange().getEnd(), 0,
  *Result.SourceManager, getLangOpts());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32945: clang-tidy: add IgnoreMacros option to modernize-use-default-member-init

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

In https://reviews.llvm.org/D32945#748604, @malcolm.parsons wrote:

> Please change modernize-use-using to match.


Done.


https://reviews.llvm.org/D32945



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

[PATCH] D32945: clang-tidy: add IgnoreMacros option to modernize-use-default-member-init

[Miklos Vajna via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL302429: clang-tidy: add IgnoreMacros option to 
modernize-use-default-member-init (authored by vmiklos).

Changed prior to commit:
  https://reviews.llvm.org/D32945?vs=98163&id=98173#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D32945

Files:
  clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
  clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.h
  clang-tools-extra/trunk/clang-tidy/modernize/UseUsingCheck.cpp
  
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-default-member-init.rst
  
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init-macros.cpp
  clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp

Index: clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -139,11 +139,13 @@
 UseDefaultMemberInitCheck::UseDefaultMemberInitCheck(StringRef Name,
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  UseAssignment(Options.get("UseAssignment", 0) != 0) {}
+  UseAssignment(Options.get("UseAssignment", 0) != 0),
+  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
 
 void UseDefaultMemberInitCheck::storeOptions(
 ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "UseAssignment", UseAssignment);
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
 }
 
 void UseDefaultMemberInitCheck::registerMatchers(MatchFinder *Finder) {
@@ -197,6 +199,10 @@
 const MatchFinder::MatchResult &Result, const CXXCtorInitializer *Init) {
   const FieldDecl *Field = Init->getMember();
 
+  SourceLocation StartLoc = Field->getLocStart();
+  if (StartLoc.isMacroID() && IgnoreMacros)
+return;
+
   SourceLocation FieldEnd =
   Lexer::getLocForEndOfToken(Field->getSourceRange().getEnd(), 0,
  *Result.SourceManager, getLangOpts());
Index: clang-tools-extra/trunk/clang-tidy/modernize/UseUsingCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/UseUsingCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseUsingCheck.cpp
@@ -19,7 +19,7 @@
 
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  IgnoreMacros(Options.get("IgnoreMacros", true)) {}
+  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
 
 void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus11)
Index: clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.h
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.h
@@ -36,6 +36,7 @@
  const CXXCtorInitializer *Init);
 
   const bool UseAssignment;
+  const bool IgnoreMacros;
 };
 
 } // namespace modernize
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-default-member-init.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-default-member-init.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-default-member-init.rst
@@ -47,3 +47,8 @@
 int i = 5;
 double j = 10.0;
   };
+
+.. option:: IgnoreMacros
+
+   If this option is set to non-zero (default is `1`), the check will not warn
+   about members declared inside macros.
Index: clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp
@@ -380,3 +380,12 @@
 
 NegativeTemplateExisting ntei(0);
 NegativeTemplateExisting nted(0);
+
+// This resulted in a warning by default.
+#define MACRO() \
+  struct MacroS { \
+void *P; \
+MacroS() : P(nullptr) {} \
+  };
+
+MACRO();
Index: clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init-macros.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init-macros.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init-macros.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s modernize-use-default-member-init %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-default-member-init.IgnoreMacros, value: 0}]}" \
+// RUN: