[PATCH] D93182: [clang-tidy] Add linux kernel log functions checker

[Tom Rix via Phabricator via cfe-commits]

trixirt added a comment.

Yes, this change has problems, I will resubmit.
There are many log function checks in checkpatch, and i need to write another 
one to find the unattibuted decls. 
I don't think adding linux specific warnings is -Wformat is needed, that 
doesn't scale with all the other projects wanting special attention
There may be a lot of checks and fixits added to linuxkernel/  .  checkpatch 
only checks for new problems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93182

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

[PATCH] D93182: [clang-tidy] Add linux kernel log functions checker

[Tom Rix via Phabricator via cfe-commits]

trixirt created this revision.
trixirt added reviewers: nickdesaulniers, alexfh, hokein, aaron.ballman, 
njames93.
trixirt added a project: clang-tools-extra.
Herald added subscribers: Charusso, xazax.hun, mgorny.
trixirt requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The linux kernel's checkpatch.pl script does several checks
and fixits on the log functions.  This clang-tidy checker repeats
these checks so the fixits can be applied tree wide.

The first fixer looks for unneeded 'h' in format strings.

>From this reproducer

  printk("%hx\n", a);

checkpatch.pl produces this warning

WARNING: Integer promotion: Using 'h' in '%hx' is unnecessary
+  printk("%hx\n", a);

and this fix

- printk("%hx\n", a);

+  printk("%x\n", a);

LKML discussion
https://lore.kernel.org/lkml/5e3265c241602bb54286fbaae9222070daa4768e.ca...@perches.com/


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93182

Files:
  clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
  clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
  clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.h
  clang-tools-extra/test/clang-tidy/checkers/linuxkernel-logfunctions-h.c

Index: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-logfunctions-h.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/linuxkernel-logfunctions-h.c
@@ -0,0 +1,20 @@
+// RUN: %check_clang_tidy %s linuxkernel-log-functions %t
+
+extern printk(const char *, ...) __attribute__((format(printf, 1, 2)));
+
+// scripts/checkpatch.pl produces
+// WARNING: Integer promotion: Using 'h' in '%hx' is unnecessary
+// ... linuxkernel-logfunctions-h.c:10:
+// +	printk("%hx\n", a);
+//
+// with --fix-inplace, the change is
+//  void f(unsigned char a)
+// {
+// -  printk("%hx\n", a);
+// +  printk("%x\n", a);
+// }
+void f(unsigned char a) {
+  printk("%hx\n", a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: Integer promotion: Using 'h' in '%hx' is unnecessay
+  // CHECK-FIXES: printk("%x\n", a);{{$}}
+}
Index: clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.h
@@ -0,0 +1,39 @@
+//===--- LogFunctionsCheck.h - clang-tidy ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_LOGFUNCTIONSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_LOGFUNCTIONSCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/Lex/MacroInfo.h"
+#include 
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+class LogFunctionsCheck : public ClangTidyCheck {
+
+  enum LogFunctionsFixerKind { LFFK_None, LFFK_H };
+
+public:
+  LogFunctionsCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
+
+private:
+  enum LogFunctionsFixerKind FixerKind;
+  const std::string LogFunctionsFixerKindName;
+};
+
+} // namespace linuxkernel
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_LOGFUNCTIONSCHECK_H
Index: clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/LogFunctionsCheck.cpp
@@ -0,0 +1,95 @@
+//===--- LogFunctionsCheck.cpp - clang-tidy ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "LogFunctionsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+LogFunctionsCheck::LogFunctionsCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context), FixerKind(LFFK_H) {}
+
+void LogFunctionsCheck::registerMatchers(MatchFinder *Finder) {
+  if (FixerKind == LFFK_H) {
+// From the reproducer
+// extern int printk(const char *, ...) __attribute__((format(printf, 1, 2)));
+// void f(unsigned short a) { printk("%hx\n", a); }
+//
+ 

[PATCH] D91789: [clang-tidy] find/fix unneeded trailing semicolons in macros

[Tom Rix via Phabricator via cfe-commits]

trixirt added a comment.

How this is run in the kernel is a wip so adding it to the commit log is not 
very helpful.
Here is the lkml rfc
https://lkml.org/lkml/2020/11/21/190
This calling in the kernel needs needs to change because of the refactor.

The auto suggestions fail to build, so they are not helpful.

Yes eventually the first review should be dropped, i'll do that when this one 
lands.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91789

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

[PATCH] D91789: [clang-tidy] find/fix unneeded trailing semicolons in macros

[Tom Rix via Phabricator via cfe-commits]

trixirt updated this revision to Diff 309270.
trixirt added a comment.

move enum


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91789

Files:
  clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
  clang-tools-extra/clang-tidy/linuxkernel/ExtraSemiCheck.cpp
  clang-tools-extra/clang-tidy/linuxkernel/ExtraSemiCheck.h
  clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
  clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
  clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c

Index: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c
@@ -0,0 +1,41 @@
+// RUN: %check_clang_tidy %s linuxkernel-extra-semi %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: linuxkernel-extra-semi.Fixer, \
+// RUN:   value: 'Switch'}, \
+// RUN: ]}"
+
+int f(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  };
+  // CHECK-MESSAGES: :[[@LINE-1]]:4: warning: unneeded semicolon
+  // CHECK-FIXES:   }{{$}}
+  return 0;
+}
+
+// A normal switch, should not produce a warning
+int p1(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  }
+  return 0;
+}
+
+#define EMPTY_MACRO()
+// A corner case, do not fix an empty macro
+int p2(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  }
+  EMPTY_MACRO();
+  return 0;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
@@ -0,0 +1,32 @@
+// RUN: %check_clang_tidy %s linuxkernel-extra-semi %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: linuxkernel-extra-semi.Fixer, \
+// RUN:   value: 'TrailingMacro'}, \
+// RUN: ]}"
+
+#define M(a) a++;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: unneeded semicolon
+// CHECK-FIXES: #define M(a) a++{{$}}
+
+int f() {
+  int v = 0;
+  M(v);
+  return v;
+}
+
+// A corner case
+// An empty macro could conditionally contain another path so
+// do not warn or fix.
+#ifdef UNSET_CONDITION
+#define E() ;
+#else
+#define E()
+#endif
+#define N(a) a++;
+
+int g() {
+  int v = 0;
+  N(v)
+  E();
+  return v;
+}
Index: clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
+++ clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "ExtraSemiCheck.h"
 #include "MustCheckErrsCheck.h"
 
 namespace clang {
@@ -19,6 +20,7 @@
 class LinuxKernelModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck("linuxkernel-extra-semi");
 CheckFactories.registerCheck(
 "linuxkernel-must-check-errs");
   }
Index: clang-tools-extra/clang-tidy/linuxkernel/ExtraSemiCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/ExtraSemiCheck.h
@@ -0,0 +1,45 @@
+//===--- ExtraSemiCheck.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_EXTRASEMICHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_EXTRASEMICHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/Lex/MacroInfo.h"
+#include 
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+class ExtraSemiCheck : public ClangTidyCheck {
+
+  enum ExtraSemiFixerKind { ESFK_None, ESFK_Switch, ESFK_TrailingMacro };
+
+public:
+  ExtraSemiCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void registerPPCallbacks(const SourceManager , Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override final;
+
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void checkMacro(SourceManager , const Token ,
+  const MacroInfo *MI);
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
+
+private:
+  std::vector SuspectMacros;
+  enum ExtraSemiFixerKind FixerKind;
+  const std::string ExtraSemiFixerKindName;
+};
+
+} // namespace linuxkernel
+} // 

[PATCH] D91789: [clang-tidy] find/fix unneeded trailing semicolons in macros

[Tom Rix via Phabricator via cfe-commits]

trixirt updated this revision to Diff 308963.
trixirt added a comment.

address precheckin issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91789

Files:
  clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
  clang-tools-extra/clang-tidy/linuxkernel/ExtraSemiCheck.cpp
  clang-tools-extra/clang-tidy/linuxkernel/ExtraSemiCheck.h
  clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
  clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
  clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c

Index: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c
@@ -0,0 +1,41 @@
+// RUN: %check_clang_tidy %s linuxkernel-extra-semi %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: linuxkernel-extra-semi.Fixer, \
+// RUN:   value: 'Switch'}, \
+// RUN: ]}"
+
+int f(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  };
+  // CHECK-MESSAGES: :[[@LINE-1]]:4: warning: unneeded semicolon
+  // CHECK-FIXES:   }{{$}}
+  return 0;
+}
+
+// A normal switch, should not produce a warning
+int p1(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  }
+  return 0;
+}
+
+#define EMPTY_MACRO()
+// A corner case, do not fix an empty macro
+int p2(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  }
+  EMPTY_MACRO();
+  return 0;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
@@ -0,0 +1,32 @@
+// RUN: %check_clang_tidy %s linuxkernel-extra-semi %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: linuxkernel-extra-semi.Fixer, \
+// RUN:   value: 'TrailingMacro'}, \
+// RUN: ]}"
+
+#define M(a) a++;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: unneeded semicolon
+// CHECK-FIXES: #define M(a) a++{{$}}
+
+int f() {
+  int v = 0;
+  M(v);
+  return v;
+}
+
+// A corner case
+// An empty macro could conditionally contain another path so
+// do not warn or fix.
+#ifdef UNSET_CONDITION
+#define E() ;
+#else
+#define E()
+#endif
+#define N(a) a++;
+
+int g() {
+  int v = 0;
+  N(v)
+  E();
+  return v;
+}
Index: clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
+++ clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "ExtraSemiCheck.h"
 #include "MustCheckErrsCheck.h"
 
 namespace clang {
@@ -19,6 +20,7 @@
 class LinuxKernelModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck("linuxkernel-extra-semi");
 CheckFactories.registerCheck(
 "linuxkernel-must-check-errs");
   }
Index: clang-tools-extra/clang-tidy/linuxkernel/ExtraSemiCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/ExtraSemiCheck.h
@@ -0,0 +1,43 @@
+//===--- ExtraSemiCheck.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_EXTRASEMICHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_EXTRASEMICHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/Lex/MacroInfo.h"
+#include 
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+enum ExtraSemiFixerKind { ESFK_None, ESFK_Switch, ESFK_TrailingMacro };
+class ExtraSemiCheck : public ClangTidyCheck {
+public:
+  ExtraSemiCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void registerPPCallbacks(const SourceManager , Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override final;
+
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void checkMacro(SourceManager , const Token ,
+  const MacroInfo *MI);
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
+
+private:
+  std::vector SuspectMacros;
+  enum ExtraSemiFixerKind FixerKind;
+  const std::string ExtraSemiFixerKindName;
+};
+
+} // namespace linuxkernel
+} 

[PATCH] D91789: [clang-tidy] find/fix unneeded trailing semicolons in macros

[Tom Rix via Phabricator via cfe-commits]

trixirt updated this revision to Diff 308214.
trixirt edited the summary of this revision.
trixirt added a comment.

Refactor to combine switch and trailing macro into one fixer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91789

Files:
  clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
  clang-tools-extra/clang-tidy/linuxkernel/ExtraSemiCheck.cpp
  clang-tools-extra/clang-tidy/linuxkernel/ExtraSemiCheck.h
  clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
  clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
  clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c

Index: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c
@@ -0,0 +1,41 @@
+// RUN: %check_clang_tidy %s linuxkernel-extra-semi %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: linuxkernel-extra-semi.Fixer, \
+// RUN:   value: 'Switch'}, \
+// RUN: ]}"
+
+int f(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  };
+  // CHECK-MESSAGES: :[[@LINE-1]]:4: warning: unneeded semicolon
+  // CHECK-FIXES:   }{{$}}
+  return 0;
+}
+
+// A normal switch, should not produce a warning
+int p1(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  }
+  return 0;
+}
+
+#define EMPTY_MACRO()
+// A corner case, do not fix an empty macro
+int p2(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  }
+  EMPTY_MACRO();
+  return 0;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
@@ -0,0 +1,32 @@
+// RUN: %check_clang_tidy %s linuxkernel-extra-semi %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: linuxkernel-extra-semi.Fixer, \
+// RUN:   value: 'TrailingMacro'}, \
+// RUN: ]}"
+
+#define M(a) a++;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: unneeded semicolon
+// CHECK-FIXES: #define M(a) a++{{$}}
+
+int f() {
+  int v = 0;
+  M(v);
+  return v;
+}
+
+// A corner case
+// An empty macro could conditionally contain another path so
+// do not warn or fix.
+#ifdef UNSET_CONDITION
+#define E() ;
+#else
+#define E()
+#endif
+#define N(a) a++;
+
+int g() {
+  int v = 0;
+  N(v)
+  E();
+  return v;
+}
Index: clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
+++ clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "ExtraSemiCheck.h"
 #include "MustCheckErrsCheck.h"
 
 namespace clang {
@@ -19,6 +20,7 @@
 class LinuxKernelModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck("linuxkernel-extra-semi");
 CheckFactories.registerCheck(
 "linuxkernel-must-check-errs");
   }
Index: clang-tools-extra/clang-tidy/linuxkernel/ExtraSemiCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/ExtraSemiCheck.h
@@ -0,0 +1,43 @@
+//===--- ExtraSemiCheck.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_EXTRASEMICHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_EXTRASEMICHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/Lex/MacroInfo.h"
+#include 
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+enum ExtraSemiFixerKind { ESFK_None, ESFK_Switch, ESFK_TrailingMacro };
+class ExtraSemiCheck : public ClangTidyCheck {
+public:
+  ExtraSemiCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void registerPPCallbacks(const SourceManager , Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override final;
+
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void checkMacro(SourceManager , const Token ,
+  const MacroInfo *MI);
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
+
+private:
+  std::vector SuspectMacros;
+  enum ExtraSemiFixerKind FixerKind;
+  

[PATCH] D91789: [clang-tidy] find/fix unneeded trailing semicolons in macros

[Tom Rix via Phabricator via cfe-commits]

trixirt updated this revision to Diff 307602.
trixirt added a comment.

Addresses issues before refactoring.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91789

Files:
  clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
  clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
  clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.cpp
  clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.h
  clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c

Index: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
@@ -0,0 +1,27 @@
+// RUN: %check_clang_tidy %s linuxkernel-macro-trailing-semi %t
+
+#define M(a) a++;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: unneeded semicolon
+// CHECK-FIXES: #define M(a) a++{{$}}
+
+int f() {
+  int v = 0;
+  M(v);
+  return v;
+}
+
+// A corner case
+// An empty macro could conditionally contain another path so
+// do not warn or fix.
+#ifdef UNSET_CONDITION
+#define E() ;
+#else
+#define E()
+#endif
+#define N(a) a++;
+
+int g() {
+  int v = 0;
+  N(v) E();
+  return v;
+}
Index: clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.h
@@ -0,0 +1,40 @@
+//===--- MacroTrailingSemiCheck.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_MACROTRAILINGSEMICHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_MACROTRAILINGSEMICHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/Lex/MacroInfo.h"
+#include 
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+class MacroTrailingSemiCheck : public ClangTidyCheck {
+public:
+  MacroTrailingSemiCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void registerPPCallbacks(const SourceManager , Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override final;
+
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void checkMacro(SourceManager , const Token ,
+  const MacroInfo *MI);
+
+private:
+  std::vector SuspectMacros;
+};
+
+} // namespace linuxkernel
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_MACROTRAILINGSEMI_CHECK_H
Index: clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.cpp
@@ -0,0 +1,135 @@
+//===--- MacroTrailingSemiCheck.cpp - clang-tidy---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MacroTrailingSemiCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+class MacroTrailingSemiCheckPPCallbacks : public PPCallbacks {
+public:
+  MacroTrailingSemiCheckPPCallbacks(Preprocessor *PP,
+MacroTrailingSemiCheck *Check)
+  : PP(PP), Check(Check) {}
+
+  void MacroDefined(const Token ,
+const MacroDirective *MD) override {
+const MacroInfo *MI = MD->getMacroInfo();
+
+if (MI->isBuiltinMacro() || MI->isObjectLike())
+  return;
+Check->checkMacro(PP->getSourceManager(), MacroNameTok, MI);
+  }
+
+private:
+  Preprocessor *PP;
+  MacroTrailingSemiCheck *Check;
+};
+
+void MacroTrailingSemiCheck::registerMatchers(MatchFinder *Finder) {
+  // From the reproducer
+  // #define M(a) a++;
+  // int f() {
+  //   int v = 0;
+  //   M(v);
+  //   return v;
+  // }
+  // The AST
+  // `-FunctionDecl
+  //   `-CompoundStmt <--- "comp", 'C'
+  // ...
+  // |-UnaryOperator
+  // | `-DeclRefExpr < 'E'
+  // |-NullStmt <-- "null" 'N'
+  // ...
+  

[PATCH] D91789: [clang-tidy] find/fix unneeded trailing semicolons in macros

[Tom Rix via Phabricator via cfe-commits]

trixirt created this revision.
trixirt added reviewers: nickdesaulniers, alexfh, hokein, aaron.ballman.
trixirt added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
Herald added a project: clang.
trixirt requested review of this revision.

Cleaning up -Wextra-semi-stmt in the linux kernel shows a high
incidence of macros with trailing semicolons

#define M(a) a++; <-- clang-tidy fixes problem here
 int f() {

  int v = 0;
  M(v); <-- clang reports problem here
  return v;

}


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91789

Files:
  clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
  clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
  clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.cpp
  clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.h
  clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c

Index: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/linuxkernel-macro-trailing-semi.c
@@ -0,0 +1,27 @@
+// RUN: %check_clang_tidy %s linuxkernel-macro-trailing-semi %t
+
+#define M(a) a++;
+// CHECK-MESSAGES: warning: unneeded semicolon
+// CHECK-MESSAGES: note: FIX-IT applied suggested code changes
+
+int f() {
+  int v = 0;
+  M(v);
+  return v;
+}
+
+// A corner case
+// An empty macro could conditionally contain another path so
+// do not warn or fix.
+#ifdef UNSET_CONDITION
+#define E() ;
+#else
+#define E()
+#endif
+#define N(a) a++;
+
+int g() {
+  int v = 0;
+  N(v) E();
+  return v;
+}
Index: clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.h
@@ -0,0 +1,40 @@
+//===--- MacroTrailingSemiCheck.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_MACROTRAILINGSEMICHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_MACROTRAILINGSEMICHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/Lex/MacroInfo.h"
+#include 
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+class MacroTrailingSemiCheck : public ClangTidyCheck {
+public:
+  MacroTrailingSemiCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void registerPPCallbacks(const SourceManager , Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override final;
+
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void checkMacro(SourceManager , const Token ,
+  const MacroInfo *MI);
+
+private:
+  std::vector SuspectMacros;
+};
+
+} // namespace linuxkernel
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_MACROTRAILINGSEMI_CHECK_H
Index: clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.cpp
@@ -0,0 +1,139 @@
+//===--- MacroTrailingSemiCheck.cpp - clang-tidy---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MacroTrailingSemiCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+class MacroTrailingSemiCheckPPCallbacks : public PPCallbacks {
+public:
+  MacroTrailingSemiCheckPPCallbacks(Preprocessor *PP,
+MacroTrailingSemiCheck *Check)
+  : PP(PP), Check(Check) {}
+
+  void MacroDefined(const Token ,
+const MacroDirective *MD) override {
+auto *MI = MD->getMacroInfo();
+
+if (MI->isBuiltinMacro() || MI->isObjectLike())
+  return;
+Check->checkMacro(PP->getSourceManager(), MacroNameTok, MI);
+  }
+
+private:
+  Preprocessor *PP;
+  MacroTrailingSemiCheck *Check;
+};
+
+void MacroTrailingSemiCheck::registerMatchers(MatchFinder *Finder) {
+  // From the reproducer

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

[Tom Rix via Phabricator via cfe-commits]

trixirt marked 2 inline comments as done.
trixirt added a comment.

I am trying to address problems treewide so around 100 changes per fix.
Doing that requires consensus that the fix is generally ok for every subsystem 
in the code base.
So while clang will fix
switch () {} ; 
it will also fix
for () {

  // tbd : add something here
  ;

}
consensus needs to be found on as narrow a fix as possible.
so there will be a specialized fixer for each problem consensus is reached on.

As for leaving whitespace fixing as an exercise for the user.
No change will be accepted with a whitespace problem.
When there are 100 fixes, there are 100 files to open, find the line, fix the 
line.
This is slow and error prone.

Another slow part is manually commit and submitting the fixes and doing a 
review cycle.
But if your fixer can automagically do everything you can  hook it into a c/i 
and and
it will keep the codebase free of its set of fixers while we all do something 
more 
interesting.

i am working on the kernel build to do this now.
It would be helpful if this and future fixers could live in clang-tidy's 
linuxkernel/
which is already part of the kernel build so i don't have to reinvent how to 
find them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90180

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

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

[Tom Rix via Phabricator via cfe-commits]

trixirt added a comment.

Discussion of change on LKML
https://lkml.org/lkml/2020/10/27/2538

On why the existing clang fixit is not practical.
tl;dr 
10,000 changes to look for a treewide fix
The fixit has whitespace issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90180

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

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

[Tom Rix via Phabricator via cfe-commits]

trixirt updated this revision to Diff 301059.
trixirt added a comment.

fix precheckin lint issues with auto vs const auto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90180

Files:
  clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
  clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
  clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.cpp
  clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.h
  clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c

Index: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c
@@ -0,0 +1,37 @@
+// RUN: %check_clang_tidy %s linuxkernel-switch-semi %t
+
+int f(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  };
+  // CHECK-MESSAGES: warning: unneeded semicolon
+  // CHECK-MESSAGES: note: FIX-IT applied suggested code changes
+  return 0;
+}
+
+// A normal switch, should not produce a warning
+int p1(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  }
+  return 0;
+}
+
+#define EMPTY_MACRO()
+// A corner case, do not fix an empty macro
+int p2(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  }
+  EMPTY_MACRO();
+  return 0;
+}
Index: clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.h
@@ -0,0 +1,30 @@
+//===--- SwitchSemiCheck.h - clang-tidy -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_SWITCHSEMICHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_SWITCHSEMICHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+class SwitchSemiCheck : public ClangTidyCheck {
+public:
+  SwitchSemiCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace linuxkernel
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_SWITCHSEMICHECK_H
Index: clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.cpp
@@ -0,0 +1,76 @@
+//===--- SwitchSemiCheck.cpp - clang-tidy--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "SwitchSemiCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+void SwitchSemiCheck::registerMatchers(MatchFinder *Finder) {
+  // From the reproducer
+  // void foo (int a) {
+  //   switch (a) {};
+  // }
+  // The AST
+  // `-FunctionDecl
+  //   |-ParmVarDecl
+  //   `-CompoundStmt <--- "comp", 'C'
+  //|-SwitchStmt <-- "switch", 'S'
+  //| |-ImplicitCastExpr
+  //| | `-DeclRefExpr
+  //| `-CompoundStmt
+  //`-NullStmt <-- 'N'
+  Finder->addMatcher(
+  compoundStmt(has(switchStmt().bind("switch"))).bind("comp"), this);
+}
+
+void SwitchSemiCheck::check(const MatchFinder::MatchResult ) {
+  const auto *C = Result.Nodes.getNodeAs("comp");
+  const auto *S = Result.Nodes.getNodeAs("switch");
+
+  auto Current = C->body_begin();
+  auto Next = Current;
+  Next++;
+  while (Next != C->body_end()) {
+if (*Current == S) {
+  if (const auto *N = dyn_cast(*Next)) {
+// This code has the same AST as the reproducer
+//
+// #define EMPTY()
+// void foo (int a) {
+// switch (a) {} EMPTY();
+// }
+//
+// But we do not want to remove the ; because the
+// macro may only be conditionally empty.
+// ex/ the release version of a debug macro
+//
+// So check that the NullStmt does not have a
+// leading empty macro.
+if (!N->hasLeadingEmptyMacro() && S->getBody()->getEndLoc().isValid() &&
+

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

[Tom Rix via Phabricator via cfe-commits]

trixirt added a comment.

The 10,000 problems are for all the extra semicolon problems.
Those after switch are a small subset, chosen because is ok to automagically 
fix them.

The checker is in the linuxkernel/  because it is being used to fix the linux 
kernel now.
And to enforce it stays fixed.

Enforcement here is necessary because the linux kernel's enforcer checkpatch is
a line checker, not an ast checker.

If folks think this is a general fixer, I do not mind moving it to some other 
checker dir.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90180

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

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

[Tom Rix via Phabricator via cfe-commits]

trixirt updated this revision to Diff 300965.
trixirt added a comment.

precheckin lint fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90180

Files:
  clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
  clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
  clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.cpp
  clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.h
  clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c

Index: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c
@@ -0,0 +1,37 @@
+// RUN: %check_clang_tidy %s linuxkernel-switch-semi %t
+
+int f(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  };
+  // CHECK-MESSAGES: warning: unneeded semicolon
+  // CHECK-MESSAGES: note: FIX-IT applied suggested code changes
+  return 0;
+}
+
+// A normal switch, should not produce a warning
+int p1(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  }
+  return 0;
+}
+
+#define EMPTY_MACRO()
+// A corner case, do not fix an empty macro
+int p2(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  }
+  EMPTY_MACRO();
+  return 0;
+}
Index: clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.h
@@ -0,0 +1,30 @@
+//===--- SwitchSemiCheck.h - clang-tidy*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_SWITCHSEMICHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_SWITCHSEMICHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+class SwitchSemiCheck : public ClangTidyCheck {
+public:
+  SwitchSemiCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace linuxkernel
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_SWITCHSEMICHECK_H
Index: clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.cpp
@@ -0,0 +1,76 @@
+//===--- SwitchSemiCheck.cpp - clang-tidy--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "SwitchSemiCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+void SwitchSemiCheck::registerMatchers(MatchFinder *Finder) {
+  // From the reproducer
+  // void foo (int a) {
+  //   switch (a) {};
+  // }
+  // The AST
+  // `-FunctionDecl
+  //   |-ParmVarDecl
+  //   `-CompoundStmt <--- "comp", 'C'
+  //|-SwitchStmt <-- "switch", 'S'
+  //| |-ImplicitCastExpr
+  //| | `-DeclRefExpr
+  //| `-CompoundStmt
+  //`-NullStmt <-- 'N'
+  Finder->addMatcher(
+  compoundStmt(has(switchStmt().bind("switch"))).bind("comp"), this);
+}
+
+void SwitchSemiCheck::check(const MatchFinder::MatchResult ) {
+  auto *C = Result.Nodes.getNodeAs("comp");
+  auto *S = Result.Nodes.getNodeAs("switch");
+
+  auto Current = C->body_begin();
+  auto Next = Current;
+  Next++;
+  while (Next != C->body_end()) {
+if (*Current == S) {
+  if (const auto *N = dyn_cast(*Next)) {
+// This code has the same AST as the reproducer
+//
+// #define EMPTY()
+// void foo (int a) {
+// switch (a) {} EMPTY();
+// }
+//
+// But we do not want to remove the ; because the
+// macro may only be conditionally empty.
+// ex/ the release version of a debug macro
+//
+// So check that the NullStmt does not have a
+// leading empty macro.
+if (!N->hasLeadingEmptyMacro() && S->getBody()->getEndLoc().isValid() &&
+N->getSemiLoc().isValid()) {
+  

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

[Tom Rix via Phabricator via cfe-commits]

trixirt added a comment.

Yes, this automates the fixit to something precise.
There are about 10,000 problems in the kernel with -Wextra-semi, all of those 
fixes would be too many and diverse to practically submit.
Also the -Wextra-semi fix removes just the semi and so it's fix will fail the 
kernel formatting check for an empty line.

With adding a -fix to the 'make clang-tidy' logic, this will fix automagically 
100 problems in 50 files for x86_64.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90180

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

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

[Tom Rix via Phabricator via cfe-commits]

trixirt updated this revision to Diff 300830.
trixirt added a comment.

Comment on the matcher
Comment explaining need to look for empty macro
Add test for normal switch
Add test for switch with empty macro
Fix formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90180

Files:
  clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
  clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
  clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.cpp
  clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.h
  clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c

Index: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c
@@ -0,0 +1,37 @@
+// RUN: %check_clang_tidy %s linuxkernel-switch-semi %t
+
+int f(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  };
+  // CHECK-MESSAGES: warning: unneeded semicolon
+  // CHECK-MESSAGES: note: FIX-IT applied suggested code changes
+  return 0;
+}
+
+// A normal switch, should not produce a warning
+int p1(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  }
+  return 0;
+}
+
+#define EMPTY_MACRO()
+// A corner case, do not fix an empty macro
+int p2(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  }
+  EMPTY_MACRO();
+  return 0;
+}
Index: clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.h
@@ -0,0 +1,30 @@
+//===--- SwitchSemiCheck.h - clang-tidy*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_SWITCHSEMICHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_SWITCHSEMICHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+class SwitchSemiCheck : public ClangTidyCheck {
+public:
+  SwitchSemiCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace linuxkernel
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_SWITCHSEMICHECK_H
Index: clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.cpp
@@ -0,0 +1,76 @@
+//===--- SwitchSemiCheck.cpp - clang-tidy--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "SwitchSemiCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+void SwitchSemiCheck::registerMatchers(MatchFinder *Finder) {
+  // From the reproducer
+  // void foo (int a) {
+  //   switch (a) {};
+  // }
+  // The AST
+  // `-FunctionDecl
+  //   |-ParmVarDecl
+  //   `-CompoundStmt <--- "comp", 'C'
+  //|-SwitchStmt <-- "switch", 'S'
+  //| |-ImplicitCastExpr
+  //| | `-DeclRefExpr
+  //| `-CompoundStmt
+  //`-NullStmt <-- 'N'
+  Finder->addMatcher(
+  compoundStmt(has(switchStmt().bind("switch"))).bind("comp"), this);
+}
+
+void SwitchSemiCheck::check(const MatchFinder::MatchResult ) {
+  auto *C = Result.Nodes.getNodeAs("comp");
+  auto *S = Result.Nodes.getNodeAs("switch");
+
+  auto Current = C->body_begin();
+  auto Next = Current;
+  Next++;
+  while (Next != C->body_end()) {
+if (*Current == S) {
+  if (const auto *N = dyn_cast(*Next)) {
+// This code has the same AST as the reproducer
+//
+// #define EMPTY()
+// void foo (int a) {
+// switch (a) {} EMPTY();
+// }
+//
+// But we do not want to remove the ; because the
+// macro may only be conditionally empty.
+// ex/ the release version of a debug macro
+//
+// So check that the NullStmt does not have a
+// leading empty macro.
+

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

[Tom Rix via Phabricator via cfe-commits]

trixirt created this revision.
trixirt added reviewers: alexfh, nickdesaulniers.
trixirt added projects: clang, clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
trixirt requested review of this revision.

Cleaning up -Wextra-semi-stmt in the linux kernel shows a high
incidence of 'switch() {} ;' The ';' is not needed.

This is special case of the Parser::ConsumeExtraSemi() fixer.
This fixer allows the fixes to be batched and takes care of the
formatting issue of having an empty line when the ';' is removed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90180

Files:
  clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
  clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
  clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.cpp
  clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.h
  clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c

Index: clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/linuxkernel-switch-semi.c
@@ -0,0 +1,13 @@
+// RUN: %check_clang_tidy %s linuxkernel-switch-semi %t
+
+int f(int a) {
+  switch (a) {
+  case 1:
+return 0;
+  default:
+break;
+  };
+  // CHECK-MESSAGES: warning: unneeded semicolon
+  // CHECK-MESSAGES: note: FIX-IT applied suggested code changes
+  return 0;
+}
Index: clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.h
@@ -0,0 +1,30 @@
+//===--- SwitchSemiCheck.h - clang-tidy*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_SWITCHSEMICHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_SWITCHSEMICHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+class SwitchSemiCheck : public ClangTidyCheck {
+public:
+  SwitchSemiCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace linuxkernel
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_SWITCHSEMICHECK_H
Index: clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linuxkernel/SwitchSemiCheck.cpp
@@ -0,0 +1,50 @@
+//===--- SwitchSemiCheck.cpp - clang-tidy--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "SwitchSemiCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace linuxkernel {
+
+void SwitchSemiCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+  compoundStmt(has(switchStmt().bind("switch"))).bind("comp"), this);
+}
+
+void SwitchSemiCheck::check(const MatchFinder::MatchResult ) {
+  auto *C = Result.Nodes.getNodeAs("comp");
+  auto *S = Result.Nodes.getNodeAs("switch");
+
+  auto Current = C->body_begin();
+  auto Next = Current;
+  Next++;
+  while (Next != C->body_end()) {
+if (*Current == S) {
+  if (const auto *N = dyn_cast(*Next)) {
+if (!N->hasLeadingEmptyMacro() && S->getBody()->getEndLoc().isValid() &&
+N->getSemiLoc().isValid()) {
+  auto H = FixItHint::CreateReplacement(
+  SourceRange(S->getBody()->getEndLoc(), N->getSemiLoc()), "}");
+  diag(N->getSemiLoc(), "unneeded semicolon") << H;
+  break;
+}
+  }
+}
+Current = Next;
+Next++;
+  }
+}
+
+} // namespace linuxkernel
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
+++ clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "MustCheckErrsCheck.h"
+#include 

[PATCH] D88784: Improve ccc-analyzer error message

[Tom Rix via Phabricator via cfe-commits]

trixirt created this revision.
trixirt added a reviewer: dcoughlin.
trixirt added a project: clang.
Herald added subscribers: cfe-commits, steakhal, ASDenysPetrov, Charusso, 
dkrupp, donat.nagy, Szelethus, a.sidorin, baloghadamsoftware.
trixirt requested review of this revision.

When ccc-analyzer fails to handle cc args, it outputs only

  

could not find clang line

  

To improve this message, print out the errors so now the output
looks like

  

could not find clang line
Please check errors
clang-12: error: unknown argument: '-fno-ipa-cp-clone'
clang-12: error: unknown argument: '-fno-partial-inlining'


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88784

Files:
  clang/tools/scan-build/libexec/ccc-analyzer


Index: clang/tools/scan-build/libexec/ccc-analyzer
===
--- clang/tools/scan-build/libexec/ccc-analyzer
+++ clang/tools/scan-build/libexec/ccc-analyzer
@@ -188,12 +188,19 @@
   my $mode = shift;
   my $Args = shift;
   my $line;
+  my $err = "";
   my $OutputStream = silent_system($HtmlDir, $Clang, "-###", $mode, @$Args);
   while (<$OutputStream>) {
+$err = $err . $_ if ($_ =~ /error:/);
 next if (!/\s"?-cc1"?\s/);
 $line = $_;
   }
-  die "could not find clang line\n" if (!defined $line);
+  if (!defined $line) {
+print STDERR "could not find clang line\n";
+print STDERR "Please check errors\n";
+print STDERR $err;
+exit 1
+  }
   # Strip leading and trailing whitespace characters.
   $line =~ s/^\s+|\s+$//g;
   my @items = quotewords('\s+', 0, $line);


Index: clang/tools/scan-build/libexec/ccc-analyzer
===
--- clang/tools/scan-build/libexec/ccc-analyzer
+++ clang/tools/scan-build/libexec/ccc-analyzer
@@ -188,12 +188,19 @@
   my $mode = shift;
   my $Args = shift;
   my $line;
+  my $err = "";
   my $OutputStream = silent_system($HtmlDir, $Clang, "-###", $mode, @$Args);
   while (<$OutputStream>) {
+$err = $err . $_ if ($_ =~ /error:/);
 next if (!/\s"?-cc1"?\s/);
 $line = $_;
   }
-  die "could not find clang line\n" if (!defined $line);
+  if (!defined $line) {
+print STDERR "could not find clang line\n";
+print STDERR "Please check errors\n";
+print STDERR $err;
+exit 1
+  }
   # Strip leading and trailing whitespace characters.
   $line =~ s/^\s+|\s+$//g;
   my @items = quotewords('\s+', 0, $line);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83984: Explicitly use utf-8 in send_string

[Tom Rix via Phabricator via cfe-commits]

trixirt added a comment.

Yes, this break on py2, and py3 the change is a noop


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83984

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

[PATCH] D83984: Explicitly use utf-8 in send_string

[Tom Rix via Phabricator via cfe-commits]

trixirt created this revision.
trixirt added reviewers: serge-sans-paille, michaelplatings.
trixirt added a project: clang.
Herald added a subscriber: cfe-commits.

send_patched_file decodes with utf-8.
The default encoder for python 2 is ascii.

So it is necessary to also change send_string to use utf-8.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83984

Files:
  clang/tools/scan-view/share/ScanView.py


Index: clang/tools/scan-view/share/ScanView.py
===
--- clang/tools/scan-view/share/ScanView.py
+++ clang/tools/scan-view/share/ScanView.py
@@ -744,7 +744,7 @@
 return f
 
 def send_string(self, s, ctype='text/html', headers=True, mtime=None):
-encoded_s = s.encode()
+encoded_s = s.encode('utf-8')
 if headers:
 self.send_response(200)
 self.send_header("Content-type", ctype)


Index: clang/tools/scan-view/share/ScanView.py
===
--- clang/tools/scan-view/share/ScanView.py
+++ clang/tools/scan-view/share/ScanView.py
@@ -744,7 +744,7 @@
 return f
 
 def send_string(self, s, ctype='text/html', headers=True, mtime=None):
-encoded_s = s.encode()
+encoded_s = s.encode('utf-8')
 if headers:
 self.send_response(200)
 self.send_header("Content-type", ctype)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61508: [clang-tidy] bugprone-header-guard : a simple version of llvm-header-guard

[Tom Rix via Phabricator via cfe-commits]

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



Comment at: clang-tidy/bugprone/HeaderGuardCheck.cpp:30
+}
+std::string BugproneHeaderGuardCheck::getHeaderGuard(StringRef Filename,
+ StringRef OldGuard) {

aaron.ballman wrote:
> Can you add a newline for some visual separation?
I did not think ment a empty newline in the param-list, so  I assumed you ment 
the function above and this function could use a new line.  


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61508



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

[PATCH] D61508: [clang-tidy] bugprone-header-guard : a simple version of llvm-header-guard

[Tom Rix via Phabricator via cfe-commits]

trixirt updated this revision to Diff 200630.
trixirt added a comment.

Add a newline to separate functions


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61508

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/HeaderGuardCheck.cpp
  clang-tidy/bugprone/HeaderGuardCheck.h
  clang-tidy/llvm/CMakeLists.txt
  clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tidy/llvm/HeaderGuardCheck.h
  clang-tidy/llvm/LLVMTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-header-guard.rst
  docs/clang-tidy/checks/google-build-namespaces.rst
  docs/clang-tidy/checks/google-global-names-in-headers.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/llvm-header-guard.rst
  docs/clang-tidy/checks/misc-definitions-in-headers.rst
  test/clang-tidy/bugprone-header-guard.cpp
  test/clang-tidy/llvm-header-guard.cpp
  unittests/clang-tidy/CMakeLists.txt
  unittests/clang-tidy/LLVMModuleTest.cpp

Index: unittests/clang-tidy/LLVMModuleTest.cpp
===
--- unittests/clang-tidy/LLVMModuleTest.cpp
+++ unittests/clang-tidy/LLVMModuleTest.cpp
@@ -1,9 +1,10 @@
 #include "ClangTidyTest.h"
-#include "llvm/HeaderGuardCheck.h"
+#include "bugprone/HeaderGuardCheck.h"
 #include "llvm/IncludeOrderCheck.h"
 #include "gtest/gtest.h"
 
 using namespace clang::tidy::llvm_check;
+using namespace clang::tidy::bugprone;
 
 namespace clang {
 namespace tidy {
@@ -14,8 +15,10 @@
 static std::string runHeaderGuardCheck(StringRef Code, const Twine ,
Optional ExpectedWarning) {
   std::vector Errors;
-  std::string Result = test::runCheckOnCode(
-  Code, , Filename, std::string("-xc++-header"));
+  ClangTidyOptions Options;
+  Options.CheckOptions["test-check-0.GuardStyle"] = "llvm";
+  std::string Result = test::runCheckOnCode(
+  Code, , Filename, std::string("-xc++-header"), Options);
   if (Errors.size() != (size_t)ExpectedWarning.hasValue())
 return "invalid error count";
   if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
@@ -25,9 +28,9 @@
 }
 
 namespace {
-struct WithEndifComment : public LLVMHeaderGuardCheck {
+struct WithEndifComment : public BugproneHeaderGuardCheck {
   WithEndifComment(StringRef Name, ClangTidyContext *Context)
-  : LLVMHeaderGuardCheck(Name, Context) {}
+  : BugproneHeaderGuardCheck(Name, Context) {}
   bool shouldSuggestEndifComment(StringRef Filename) override { return true; }
 };
 } // namespace
@@ -36,8 +39,10 @@
 runHeaderGuardCheckWithEndif(StringRef Code, const Twine ,
  Optional ExpectedWarning) {
   std::vector Errors;
+  ClangTidyOptions Options;
+  Options.CheckOptions["test-check-0.GuardStyle"] = "llvm";
   std::string Result = test::runCheckOnCode(
-  Code, , Filename, std::string("-xc++-header"));
+  Code, , Filename, std::string("-xc++-header"), Options);
   if (Errors.size() != (size_t)ExpectedWarning.hasValue())
 return "invalid error count";
   if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
Index: unittests/clang-tidy/CMakeLists.txt
===
--- unittests/clang-tidy/CMakeLists.txt
+++ unittests/clang-tidy/CMakeLists.txt
@@ -29,6 +29,7 @@
   clangSerialization
   clangTidy
   clangTidyAndroidModule
+  clangTidyBugproneModule
   clangTidyGoogleModule
   clangTidyLLVMModule
   clangTidyObjCModule
Index: test/clang-tidy/llvm-header-guard.cpp
===
--- /dev/null
+++ test/clang-tidy/llvm-header-guard.cpp
@@ -0,0 +1,9 @@
+// RUN: %check_clang_tidy %s llvm-header-guard %t -- \
+// RUN:   -config="{CheckOptions: [{key: llvm-header-guard.HeaderFileExtensions, value: 'cpp'}]}" \
+// RUN:   -header-filter=.* --
+
+// CHECK-MESSAGES: 1:1: warning:  header is missing header guard
+
+// CHECK-FIXES: #ifndef LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_OUTPUT_LLVM_HEADER_GUARD_CPP_TMP_CPP
+// CHECK-FIXES-NEXT: #define LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_OUTPUT_LLVM_HEADER_GUARD_CPP_TMP_CPP
+// CHECK-FIXES: #endif
Index: test/clang-tidy/bugprone-header-guard.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-header-guard.cpp
@@ -0,0 +1,21 @@
+// GuardStyle=default (minimal), HeaderFileExtension=cpp
+// RUN: %check_clang_tidy %s bugprone-header-guard %t -- -config="{CheckOptions: [{key: bugprone-header-guard.HeaderFileExtensions, value: 'cpp'}]}" -header-filter=.* --
+// GuardStyle=llvm,  HeaderFileExtension=cpp
+// RUN: %check_clang_tidy -check-suffix=LLVM %s bugprone-header-guard %t -- -config="{CheckOptions: [{key: bugprone-header-guard.HeaderFileExtensions, value: 'cpp'}, {key: bugprone-header-guard.GuardStyle, value: 'llvm'}]}" -header-filter=.* --
+// 

[PATCH] D61508: [clang-tidy] bugprone-header-guard : a simple version of llvm-header-guard

[Tom Rix via Phabricator via cfe-commits]

trixirt added a comment.

Latest change addresses most of issues.
Outstanding is adding a test when garbage GuardStyle value is fed into config.
The trial testcase used only CHECK-*-NOT which caused complaining from test 
harness looking for the CHECK- cases.
Looking for something similar in the other checker tests did not turn up 
anything.
In general it seems like clang-tidy assumes developers get the string names of 
the options and their values correct and does not provide a lot of (any?)  
checking.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61508



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

[PATCH] D61508: [clang-tidy] bugprone-header-guard : a simple version of llvm-header-guard

[Tom Rix via Phabricator via cfe-commits]

trixirt updated this revision to Diff 200380.
trixirt added a comment.

Change the GuardStyle data type from a string to an enum.
Add test cases for explicitly setting valid style.
Document valid style values.
Reduce llvm-header-guard doc to point to bugprone-header-guard.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61508

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/HeaderGuardCheck.cpp
  clang-tidy/bugprone/HeaderGuardCheck.h
  clang-tidy/llvm/CMakeLists.txt
  clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tidy/llvm/HeaderGuardCheck.h
  clang-tidy/llvm/LLVMTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-header-guard.rst
  docs/clang-tidy/checks/google-build-namespaces.rst
  docs/clang-tidy/checks/google-global-names-in-headers.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/llvm-header-guard.rst
  docs/clang-tidy/checks/misc-definitions-in-headers.rst
  test/clang-tidy/bugprone-header-guard.cpp
  test/clang-tidy/llvm-header-guard.cpp
  unittests/clang-tidy/CMakeLists.txt
  unittests/clang-tidy/LLVMModuleTest.cpp

Index: unittests/clang-tidy/LLVMModuleTest.cpp
===
--- unittests/clang-tidy/LLVMModuleTest.cpp
+++ unittests/clang-tidy/LLVMModuleTest.cpp
@@ -1,9 +1,10 @@
 #include "ClangTidyTest.h"
-#include "llvm/HeaderGuardCheck.h"
+#include "bugprone/HeaderGuardCheck.h"
 #include "llvm/IncludeOrderCheck.h"
 #include "gtest/gtest.h"
 
 using namespace clang::tidy::llvm_check;
+using namespace clang::tidy::bugprone;
 
 namespace clang {
 namespace tidy {
@@ -14,8 +15,10 @@
 static std::string runHeaderGuardCheck(StringRef Code, const Twine ,
Optional ExpectedWarning) {
   std::vector Errors;
-  std::string Result = test::runCheckOnCode(
-  Code, , Filename, std::string("-xc++-header"));
+  ClangTidyOptions Options;
+  Options.CheckOptions["test-check-0.GuardStyle"] = "llvm";
+  std::string Result = test::runCheckOnCode(
+  Code, , Filename, std::string("-xc++-header"), Options);
   if (Errors.size() != (size_t)ExpectedWarning.hasValue())
 return "invalid error count";
   if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
@@ -25,9 +28,9 @@
 }
 
 namespace {
-struct WithEndifComment : public LLVMHeaderGuardCheck {
+struct WithEndifComment : public BugproneHeaderGuardCheck {
   WithEndifComment(StringRef Name, ClangTidyContext *Context)
-  : LLVMHeaderGuardCheck(Name, Context) {}
+  : BugproneHeaderGuardCheck(Name, Context) {}
   bool shouldSuggestEndifComment(StringRef Filename) override { return true; }
 };
 } // namespace
@@ -36,8 +39,10 @@
 runHeaderGuardCheckWithEndif(StringRef Code, const Twine ,
  Optional ExpectedWarning) {
   std::vector Errors;
+  ClangTidyOptions Options;
+  Options.CheckOptions["test-check-0.GuardStyle"] = "llvm";
   std::string Result = test::runCheckOnCode(
-  Code, , Filename, std::string("-xc++-header"));
+  Code, , Filename, std::string("-xc++-header"), Options);
   if (Errors.size() != (size_t)ExpectedWarning.hasValue())
 return "invalid error count";
   if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
Index: unittests/clang-tidy/CMakeLists.txt
===
--- unittests/clang-tidy/CMakeLists.txt
+++ unittests/clang-tidy/CMakeLists.txt
@@ -29,6 +29,7 @@
   clangSerialization
   clangTidy
   clangTidyAndroidModule
+  clangTidyBugproneModule
   clangTidyGoogleModule
   clangTidyLLVMModule
   clangTidyObjCModule
Index: test/clang-tidy/llvm-header-guard.cpp
===
--- /dev/null
+++ test/clang-tidy/llvm-header-guard.cpp
@@ -0,0 +1,9 @@
+// RUN: %check_clang_tidy %s llvm-header-guard %t -- \
+// RUN:   -config="{CheckOptions: [{key: llvm-header-guard.HeaderFileExtensions, value: 'cpp'}]}" \
+// RUN:   -header-filter=.* --
+
+// CHECK-MESSAGES: 1:1: warning:  header is missing header guard
+
+// CHECK-FIXES: #ifndef LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_OUTPUT_LLVM_HEADER_GUARD_CPP_TMP_CPP
+// CHECK-FIXES-NEXT: #define LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_OUTPUT_LLVM_HEADER_GUARD_CPP_TMP_CPP
+// CHECK-FIXES: #endif
Index: test/clang-tidy/bugprone-header-guard.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-header-guard.cpp
@@ -0,0 +1,21 @@
+// GuardStyle=default (minimal), HeaderFileExtension=cpp
+// RUN: %check_clang_tidy %s bugprone-header-guard %t -- -config="{CheckOptions: [{key: bugprone-header-guard.HeaderFileExtensions, value: 'cpp'}]}" -header-filter=.* --
+// GuardStyle=llvm,  HeaderFileExtension=cpp
+// RUN: %check_clang_tidy -check-suffix=LLVM %s bugprone-header-guard %t -- 

[PATCH] D61508: [clang-tidy] bugprone-header-guard : a simple version of llvm-header-guard

[Tom Rix via Phabricator via cfe-commits]

trixirt updated this revision to Diff 199358.
trixirt added a comment.

llvm-header-guard is an alias to bugprone-header-guard.
The style is passed through the option 'GuardStyle'

A regression test was added for llvm-header-guard.
llvm unit tests were modified to the new class and option interface.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61508

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/HeaderGuardCheck.cpp
  clang-tidy/bugprone/HeaderGuardCheck.h
  clang-tidy/llvm/CMakeLists.txt
  clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tidy/llvm/HeaderGuardCheck.h
  clang-tidy/llvm/LLVMTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-header-guard.rst
  docs/clang-tidy/checks/google-build-namespaces.rst
  docs/clang-tidy/checks/google-global-names-in-headers.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/llvm-header-guard.rst
  docs/clang-tidy/checks/misc-definitions-in-headers.rst
  test/clang-tidy/bugprone-header-guard.cpp
  test/clang-tidy/llvm-header-guard.cpp
  unittests/clang-tidy/CMakeLists.txt
  unittests/clang-tidy/LLVMModuleTest.cpp

Index: unittests/clang-tidy/LLVMModuleTest.cpp
===
--- unittests/clang-tidy/LLVMModuleTest.cpp
+++ unittests/clang-tidy/LLVMModuleTest.cpp
@@ -1,9 +1,10 @@
 #include "ClangTidyTest.h"
-#include "llvm/HeaderGuardCheck.h"
+#include "bugprone/HeaderGuardCheck.h"
 #include "llvm/IncludeOrderCheck.h"
 #include "gtest/gtest.h"
 
 using namespace clang::tidy::llvm_check;
+using namespace clang::tidy::bugprone;
 
 namespace clang {
 namespace tidy {
@@ -14,8 +15,10 @@
 static std::string runHeaderGuardCheck(StringRef Code, const Twine ,
Optional ExpectedWarning) {
   std::vector Errors;
-  std::string Result = test::runCheckOnCode(
-  Code, , Filename, std::string("-xc++-header"));
+  ClangTidyOptions Options;
+  Options.CheckOptions["test-check-0.GuardStyle"] = "llvm";
+  std::string Result = test::runCheckOnCode(
+  Code, , Filename, std::string("-xc++-header"), Options);
   if (Errors.size() != (size_t)ExpectedWarning.hasValue())
 return "invalid error count";
   if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
@@ -25,9 +28,9 @@
 }
 
 namespace {
-struct WithEndifComment : public LLVMHeaderGuardCheck {
+struct WithEndifComment : public BugproneHeaderGuardCheck {
   WithEndifComment(StringRef Name, ClangTidyContext *Context)
-  : LLVMHeaderGuardCheck(Name, Context) {}
+  : BugproneHeaderGuardCheck(Name, Context) {}
   bool shouldSuggestEndifComment(StringRef Filename) override { return true; }
 };
 } // namespace
@@ -36,8 +39,10 @@
 runHeaderGuardCheckWithEndif(StringRef Code, const Twine ,
  Optional ExpectedWarning) {
   std::vector Errors;
+  ClangTidyOptions Options;
+  Options.CheckOptions["test-check-0.GuardStyle"] = "llvm";
   std::string Result = test::runCheckOnCode(
-  Code, , Filename, std::string("-xc++-header"));
+  Code, , Filename, std::string("-xc++-header"), Options);
   if (Errors.size() != (size_t)ExpectedWarning.hasValue())
 return "invalid error count";
   if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
Index: unittests/clang-tidy/CMakeLists.txt
===
--- unittests/clang-tidy/CMakeLists.txt
+++ unittests/clang-tidy/CMakeLists.txt
@@ -29,6 +29,7 @@
   clangSerialization
   clangTidy
   clangTidyAndroidModule
+  clangTidyBugproneModule
   clangTidyGoogleModule
   clangTidyLLVMModule
   clangTidyObjCModule
Index: test/clang-tidy/llvm-header-guard.cpp
===
--- /dev/null
+++ test/clang-tidy/llvm-header-guard.cpp
@@ -0,0 +1,9 @@
+// RUN: %check_clang_tidy %s llvm-header-guard %t -- \
+// RUN:   -config="{CheckOptions: [{key: llvm-header-guard.HeaderFileExtensions, value: 'cpp'}]}" \
+// RUN:   -header-filter=.* --
+
+// CHECK-MESSAGES: 1:1: warning:  header is missing header guard
+
+// CHECK-FIXES: #ifndef LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_OUTPUT_LLVM_HEADER_GUARD_CPP_TMP_CPP
+// CHECK-FIXES-NEXT: #define LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_OUTPUT_LLVM_HEADER_GUARD_CPP_TMP_CPP
+// CHECK-FIXES: #endif
Index: test/clang-tidy/bugprone-header-guard.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-header-guard.cpp
@@ -0,0 +1,9 @@
+// RUN: %check_clang_tidy %s bugprone-header-guard %t -- \
+// RUN:   -config="{CheckOptions: [{key: bugprone-header-guard.HeaderFileExtensions, value: 'cpp'}]}" \
+// RUN:   -header-filter=.* --
+
+// CHECK-MESSAGES: 1:1: warning:  header is missing header guard
+
+// CHECK-FIXES: #ifndef BUGPRONE_HEADER_GUARD_CPP_TMP_CPP
+// 

[PATCH] D61508: [clang-tidy] bugprone-header-guard : a simple version of llvm-header-guard

[Tom Rix via Phabricator via cfe-commits]

trixirt added a comment.

All of the real work is the header-guard checks is done by their common base 
class utils/HeaderGuard.
The much smaller llvm and bugprone subclasses are only concerned with the style 
of the fix.

My understanding of the directory layout guidance in the docs is that different 
styles get different directories.

  Next, you need to decide which module the check belongs to. Modules
  are located in subdirectories of `clang-tidy/
  
`_
  and contain checks targeting a certain aspect of code quality (performance,
  readability, etc.), certain coding style or standard (Google, LLVM, CERT, 
etc.)
  or a widely used API (e.g. MPI). Their names are same as user-facing check
  groups names described :ref:`above `.

So keep as-is ?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61508



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

[PATCH] D61508: [clang-tidy] bugprone-header-guard : a simple version of llvm-header-guard

[Tom Rix via Phabricator via cfe-commits]

trixirt updated this revision to Diff 198352.
trixirt retitled this revision from "[clang-tidy] misc-header-guard : a simple 
version of  llvm-header-guard" to "[clang-tidy] bugprone-header-guard : a 
simple version of  llvm-header-guard".
trixirt added a comment.

Move from misc to bugprone


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61508

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/HeaderGuardCheck.cpp
  clang-tidy/bugprone/HeaderGuardCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-header-guard.rst
  docs/clang-tidy/checks/google-build-namespaces.rst
  docs/clang-tidy/checks/google-global-names-in-headers.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/llvm-header-guard.rst
  docs/clang-tidy/checks/misc-definitions-in-headers.rst
  test/clang-tidy/bugprone-header-guard.cpp

Index: test/clang-tidy/bugprone-header-guard.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-header-guard.cpp
@@ -0,0 +1,9 @@
+// RUN: %check_clang_tidy %s bugprone-header-guard %t -- \
+// RUN:   -config="{CheckOptions: [{key: bugprone-header-guard.HeaderFileExtensions, value: 'cpp'}]}" \
+// RUN:   -header-filter=.* -- 
+
+// CHECK-MESSAGES: 1:1: warning:  header is missing header guard
+
+// CHECK-FIXES: #ifndef BUGPRONE_HEADER_GUARD_CPP_TMP_CPP
+// CHECK-FIXES-NEXT: #define BUGPRONE_HEADER_GUARD_CPP_TMP_CPP
+// CHECK-FIXES: #endif
Index: docs/clang-tidy/checks/misc-definitions-in-headers.rst
===
--- docs/clang-tidy/checks/misc-definitions-in-headers.rst
+++ docs/clang-tidy/checks/misc-definitions-in-headers.rst
@@ -89,10 +89,10 @@
 .. option:: HeaderFileExtensions
 
A comma-separated list of filename extensions of header files (the filename
-   extensions should not include "." prefix). Default is "h,hh,hpp,hxx".
+   extensions should not include "." prefix). Default is `h,hh,hpp,hxx`.
For header files without an extension, use an empty string (if there are no
other desired extensions) or leave an empty element in the list. e.g.,
-   "h,hh,hpp,hxx," (note the trailing comma).
+   `h,hh,hpp,hxx,` (note the trailing comma).
 
 .. option:: UseHeaderFileExtension
 
Index: docs/clang-tidy/checks/llvm-header-guard.rst
===
--- docs/clang-tidy/checks/llvm-header-guard.rst
+++ docs/clang-tidy/checks/llvm-header-guard.rst
@@ -11,7 +11,7 @@
 .. option:: HeaderFileExtensions
 
A comma-separated list of filename extensions of header files (the filename
-   extensions should not include "." prefix). Default is "h,hh,hpp,hxx".
+   extensions should not include "." prefix). Default is `h,hh,hpp,hxx`.
For header files without an extension, use an empty string (if there are no
other desired extensions) or leave an empty element in the list. e.g.,
-   "h,hh,hpp,hxx," (note the trailing comma).
+   `h,hh,hpp,hxx,` (note the trailing comma).
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -44,6 +44,7 @@
bugprone-fold-init-type
bugprone-forward-declaration-namespace
bugprone-forwarding-reference-overload
+   bugprone-header-guard
bugprone-inaccurate-erase
bugprone-incorrect-roundings
bugprone-integer-division
Index: docs/clang-tidy/checks/google-global-names-in-headers.rst
===
--- docs/clang-tidy/checks/google-global-names-in-headers.rst
+++ docs/clang-tidy/checks/google-global-names-in-headers.rst
@@ -15,7 +15,7 @@
 .. option:: HeaderFileExtensions
 
A comma-separated list of filename extensions of header files (the filename
-   extensions should not contain "." prefix). Default is "h".
+   extensions should not include "." prefix). Default is `h,hh,hpp,hxx`.
For header files without an extension, use an empty string (if there are no
other desired extensions) or leave an empty element in the list. e.g.,
-   "h,hh,hpp,hxx," (note the trailing comma).
+   `h,hh,hpp,hxx,` (note the trailing comma).
Index: docs/clang-tidy/checks/google-build-namespaces.rst
===
--- docs/clang-tidy/checks/google-build-namespaces.rst
+++ docs/clang-tidy/checks/google-build-namespaces.rst
@@ -18,7 +18,7 @@
 .. option:: HeaderFileExtensions
 
A comma-separated list of filename extensions of header files (the filename
-   extensions should not include "." prefix). Default is "h,hh,hpp,hxx".
+   extensions should not include "." prefix). Default is `h,hh,hpp,hxx`.
For header files without an extension, use an empty string (if there are no
other desired extensions) or leave 

[PATCH] D61508: [clang-tidy] misc-header-guard : a simple version of llvm-header-guard

[Tom Rix via Phabricator via cfe-commits]

trixirt updated this revision to Diff 198146.
trixirt edited the summary of this revision.
trixirt added a comment.

Addressed issue with returning early and using backtick in doc.
Since misc-header-guard doc was a cut-n-paste of llvm-header-guard, that and 
other similar docs were changed.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61508

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/HeaderGuardCheck.cpp
  clang-tidy/misc/HeaderGuardCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/google-build-namespaces.rst
  docs/clang-tidy/checks/google-global-names-in-headers.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/llvm-header-guard.rst
  docs/clang-tidy/checks/misc-definitions-in-headers.rst
  docs/clang-tidy/checks/misc-header-guard.rst
  test/clang-tidy/misc-header-guard.cpp

Index: test/clang-tidy/misc-header-guard.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-header-guard.cpp
@@ -0,0 +1,9 @@
+// RUN: %check_clang_tidy %s misc-header-guard %t -- \
+// RUN:   -config="{CheckOptions: [{key: misc-header-guard.HeaderFileExtensions, value: 'cpp'}]}" \
+// RUN:   -header-filter=.* -- 
+
+// CHECK-MESSAGES: 1:1: warning:  header is missing header guard
+
+// CHECK-FIXES: #ifndef MISC_HEADER_GUARD_CPP_TMP_CPP
+// CHECK-FIXES-NEXT: #define MISC_HEADER_GUARD_CPP_TMP_CPP
+// CHECK-FIXES: #endif
Index: docs/clang-tidy/checks/misc-header-guard.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-header-guard.rst
@@ -0,0 +1,18 @@
+.. title:: clang-tidy - misc-header-guard
+
+misc-header-guard
+=
+
+Finds and fixes missing header guards and does not enforce any style.
+
+Options
+---
+
+.. option:: HeaderFileExtensions
+
+   A comma-separated list of filename extensions of header files (the filename
+   extensions should not include "." prefix). Default is `h,hh,hpp,hxx`.
+   For header files without an extension, use an empty string (if there are no
+   other desired extensions) or leave an empty element in the list. e.g.,
+   `h,hh,hpp,hxx,` (note the trailing comma).
+
Index: docs/clang-tidy/checks/misc-definitions-in-headers.rst
===
--- docs/clang-tidy/checks/misc-definitions-in-headers.rst
+++ docs/clang-tidy/checks/misc-definitions-in-headers.rst
@@ -89,10 +89,10 @@
 .. option:: HeaderFileExtensions
 
A comma-separated list of filename extensions of header files (the filename
-   extensions should not include "." prefix). Default is "h,hh,hpp,hxx".
+   extensions should not include "." prefix). Default is `h,hh,hpp,hxx`.
For header files without an extension, use an empty string (if there are no
other desired extensions) or leave an empty element in the list. e.g.,
-   "h,hh,hpp,hxx," (note the trailing comma).
+   `h,hh,hpp,hxx,` (note the trailing comma).
 
 .. option:: UseHeaderFileExtension
 
Index: docs/clang-tidy/checks/llvm-header-guard.rst
===
--- docs/clang-tidy/checks/llvm-header-guard.rst
+++ docs/clang-tidy/checks/llvm-header-guard.rst
@@ -11,7 +11,7 @@
 .. option:: HeaderFileExtensions
 
A comma-separated list of filename extensions of header files (the filename
-   extensions should not include "." prefix). Default is "h,hh,hpp,hxx".
+   extensions should not include "." prefix). Default is `h,hh,hpp,hxx`.
For header files without an extension, use an empty string (if there are no
other desired extensions) or leave an empty element in the list. e.g.,
-   "h,hh,hpp,hxx," (note the trailing comma).
+   `h,hh,hpp,hxx,` (note the trailing comma).
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -181,6 +181,7 @@
llvm-prefer-isa-or-dyn-cast-in-conditionals
llvm-twine-local
misc-definitions-in-headers
+   misc-header-guard
misc-misplaced-const
misc-new-delete-overloads
misc-non-copyable-objects
Index: docs/clang-tidy/checks/google-global-names-in-headers.rst
===
--- docs/clang-tidy/checks/google-global-names-in-headers.rst
+++ docs/clang-tidy/checks/google-global-names-in-headers.rst
@@ -15,7 +15,7 @@
 .. option:: HeaderFileExtensions
 
A comma-separated list of filename extensions of header files (the filename
-   extensions should not contain "." prefix). Default is "h".
+   extensions should not include "." prefix). Default is `h,hh,hpp,hxx`.
For header files without an extension, use an empty string (if there are no
other desired extensions) or leave an empty element in the list. e.g.,
-   "h,hh,hpp,hxx," (note the 

[PATCH] D61508: [clang-tidy] misc-header-guard : a simple version of llvm-header-guard

[Tom Rix via Phabricator via cfe-commits]

trixirt created this revision.
trixirt added reviewers: alexfh, bkramer.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
Herald added a project: clang.

A general use, missing header guard finder and fixer.
No style checks.  If there is already a working header guard, it is used.
If the header guard is missing or broken, it is created from the path name
using this method.

/foo.h ->

#ifndef FOO_H
 #define FOO_H

#endif

The motivation for this change is fixing a large, non llvm with many missing 
header guards.
As no style is checked, it should not conflict with llvm-header-guard.

The change is also in this dev branch
https://github.com/trixirt/clang-tools-extra/tree/misc-header-check


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61508

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/HeaderGuardCheck.cpp
  clang-tidy/misc/HeaderGuardCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-header-guard.rst
  test/clang-tidy/misc-header-guard.cpp

Index: test/clang-tidy/misc-header-guard.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-header-guard.cpp
@@ -0,0 +1,9 @@
+// RUN: %check_clang_tidy %s misc-header-guard %t -- \
+// RUN:   -config="{CheckOptions: [{key: misc-header-guard.HeaderFileExtensions, value: 'cpp'}]}" \
+// RUN:   -header-filter=.* -- 
+
+// CHECK-MESSAGES: 1:1: warning:  header is missing header guard
+
+// CHECK-FIXES: #ifndef MISC_HEADER_GUARD_CPP_TMP_CPP
+// CHECK-FIXES-NEXT: #define MISC_HEADER_GUARD_CPP_TMP_CPP
+// CHECK-FIXES: #endif
Index: docs/clang-tidy/checks/misc-header-guard.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-header-guard.rst
@@ -0,0 +1,18 @@
+.. title:: clang-tidy - misc-header-guard
+
+misc-header-guard
+=
+
+Finds and fixes missing header guards and does not enforce any style.
+
+Options
+---
+
+.. option:: HeaderFileExtensions
+
+   A comma-separated list of filename extensions of header files (the filename
+   extensions should not include "." prefix). Default is "h,hh,hpp,hxx".
+   For header files without an extension, use an empty string (if there are no
+   other desired extensions) or leave an empty element in the list. e.g.,
+   "h,hh,hpp,hxx," (note the trailing comma).
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -181,6 +181,7 @@
llvm-prefer-isa-or-dyn-cast-in-conditionals
llvm-twine-local
misc-definitions-in-headers
+   misc-header-guard
misc-misplaced-const
misc-new-delete-overloads
misc-non-copyable-objects
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -108,6 +108,11 @@
   Checks whether there are underscores in googletest test and test case names in
   test macros, which is prohibited by the Googletest FAQ.
 
+- New :doc:`misc-header-guard
+  ` check.
+
+  Finds and fixes missing header guards and does not enforce any style.
+
 - New :doc:`objc-super-self ` check.
 
   Finds invocations of ``-self`` on super instances in initializers of
Index: clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "DefinitionsInHeadersCheck.h"
+#include "HeaderGuardCheck.h"
 #include "MisplacedConstCheck.h"
 #include "NewDeleteOverloadsCheck.h"
 #include "NonCopyableObjects.h"
@@ -32,6 +33,8 @@
   void addCheckFactories(ClangTidyCheckFactories ) override {
 CheckFactories.registerCheck(
 "misc-definitions-in-headers");
+CheckFactories.registerCheck(
+"misc-header-guard");
 CheckFactories.registerCheck("misc-misplaced-const");
 CheckFactories.registerCheck(
 "misc-new-delete-overloads");
Index: clang-tidy/misc/HeaderGuardCheck.h
===
--- /dev/null
+++ clang-tidy/misc/HeaderGuardCheck.h
@@ -0,0 +1,33 @@
+//===--- HeaderGuardCheck.h - clang-tidy *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_HEADERGUARDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_HEADERGUARDCHECK_H
+
+#include "../utils/HeaderGuard.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Finds 

[PATCH] D60233: [clang-scan-deps] initial outline of the tool that runs preprocessor to find dependencies over a JSON compilation database

[Tom Rix via Phabricator via cfe-commits]

trixirt added a comment.

A comment by whisperity in the origin WIP did not seem to be addressed.
Have you looked at IWYU 
https://github.com/include-what-you-use/include-what-you-use ?
The end goals of clang-scan-deps and iwyu seem similar so their implementation 
problems would also be similar.
The iwyu problems doc is 
https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/WhyIWYUIsDifficult.md


Repository:
  rC Clang

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

https://reviews.llvm.org/D60233



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

[PATCH] D59998: [Sema] Remove unneeded call to LookupResult::resolveKind

[Tom Rix via Phabricator via cfe-commits]

trixirt created this revision.
trixirt added a reviewer: rjmccall.
trixirt added a project: clang.
Herald added a subscriber: cfe-commits.

>From code inspection it was noticed that CppNamespaceLookup's
caller CppLookupName calls LookupResult::resolveKind when
CppNamespaceLookup was successful or had an earlier success.


Repository:
  rC Clang

https://reviews.llvm.org/D59998

Files:
  lib/Sema/SemaLookup.cpp


Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -948,7 +948,7 @@
 if (LookupDirect(S, R, UUE.getNominatedNamespace()))
   Found = true;
 
-  R.resolveKind();
+  // Defer resolution to the caller.
 
   return Found;
 }


Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -948,7 +948,7 @@
 if (LookupDirect(S, R, UUE.getNominatedNamespace()))
   Found = true;
 
-  R.resolveKind();
+  // Defer resolution to the caller.
 
   return Found;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54047: Check TUScope is valid before use

[Tom Rix via Phabricator via cfe-commits]

trixirt added a comment.

Since the crash happens with the iwyu tool, a bit ago i added the testcase at 
the iwyu project here.
https://github.com/include-what-you-use/include-what-you-use/pull/601

I don't know if it makes sense to run iwyu from clang/test.


Repository:
  rC Clang

https://reviews.llvm.org/D54047



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

[PATCH] D54047: Check TUScope is valid before use

[Tom Rix via Phabricator via cfe-commits]

trixirt created this revision.
trixirt added reviewers: rsmith, stephenkelly.
Herald added a subscriber: cfe-commits.

Include-what-you-use crashes when run against llvm because of late use of 
TUScope.
The testcase is 
https://github.com/trixirt/include-what-you-use/commit/dfec8cf07015fb5fe2db10df2f0a005250953131

Enabling incremental processing, fixes this problem but causes 3 regression in 
iwyu.
https://github.com/include-what-you-use/include-what-you-use/pull/586

So the fix is moving to clang.


Repository:
  rC Clang

https://reviews.llvm.org/D54047

Files:
  lib/Sema/SemaExpr.cpp


Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -905,6 +905,10 @@
 UnqualifiedId Name;
 Name.setIdentifier(PP.getIdentifierInfo("__builtin_trap"),
E->getBeginLoc());
+
+if (TUScope == nullptr)
+  return ExprError();
+
 ExprResult TrapFn = ActOnIdExpression(TUScope, SS, TemplateKWLoc,
   Name, true, false);
 if (TrapFn.isInvalid())


Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -905,6 +905,10 @@
 UnqualifiedId Name;
 Name.setIdentifier(PP.getIdentifierInfo("__builtin_trap"),
E->getBeginLoc());
+
+if (TUScope == nullptr)
+  return ExprError();
+
 ExprResult TrapFn = ActOnIdExpression(TUScope, SS, TemplateKWLoc,
   Name, true, false);
 if (TrapFn.isInvalid())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47554: [analyzer] Check for dead/impossible status checks

[Tom Rix via Phabricator via cfe-commits]

trixirt added a comment.

Sorry for the delay, i will have something (hopefully) this weekend.


Repository:
  rC Clang

https://reviews.llvm.org/D47554



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

[PATCH] D47554: [analyzer] Check for dead/impossible status checks

[Tom Rix via Phabricator via cfe-commits]

trixirt updated this revision to Diff 150003.
trixirt added a comment.

cleanup of last patch


Repository:
  rC Clang

https://reviews.llvm.org/D47554

Files:
  include/clang/AST/ExprCXX.h
  include/clang/ASTMatchers/ASTMatchers.h
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/AST/ExprCXX.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  lib/CodeGen/CGExprCXX.cpp
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/DeadStatusChecker.cpp
  test/Analysis/dead-status-new-nullptr.cpp

Index: test/Analysis/dead-status-new-nullptr.cpp
===
--- /dev/null
+++ test/Analysis/dead-status-new-nullptr.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=deadcode.DeadStatus -verify %s
+
+void *t1(unsigned n, bool *fail);
+void *t1(unsigned n, bool *fail) {
+  char *m = new char[n];
+  if (nullptr == m) // expected-warning{{This variable can never be a nullptr}}
+*fail = true;
+  else
+*fail = false;
+  return m;
+}
+
+void *t2(unsigned n, bool *fail);
+void *t2(unsigned n, bool *fail) {
+  char *m = new char[n];
+  if (m == nullptr) // expected-warning{{This variable can never be a nullptr}}
+*fail = true;
+  else
+*fail = false;
+  return m;
+}
+
+// a variant of nullptr
+void *t3(unsigned n, bool *fail);
+void *t3(unsigned n, bool *fail) {
+  char *m = new char[n];
+  if (m == 0) // expected-warning{{This variable can never be a nullptr}}
+*fail = true;
+  else
+*fail = false;
+  return m;
+}
+
+// a variant of nullptr
+#define NULL __null
+void *t4(unsigned n, bool *fail);
+void *t4(unsigned n, bool *fail) {
+  char *m = new char[n];
+  if (m == NULL) // expected-warning{{This variable can never be a nullptr}}
+*fail = true;
+  else
+*fail = false;
+  return m;
+}
Index: lib/StaticAnalyzer/Checkers/DeadStatusChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/DeadStatusChecker.cpp
@@ -0,0 +1,140 @@
+//==- DeadStatusChecker.cpp - Check for impossible status -*- C++ -*-==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file defines a DeadStatus, a checker that looks for impossible
+//  status checks.
+//
+//===--===//
+
+#include "ClangSACheckers.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+class DeadStatusChecker : public Checker {
+  class Walker : public RecursiveASTVisitor {
+BugReporter 
+const DeadStatusChecker *Checker;
+AnalysisDeclContext *AC;
+llvm::SmallPtrSet throwingNews;
+
+const Expr *hasNullptr(const BinaryOperator *BO) const {
+  const Expr *ret = nullptr;
+  const Expr *E[2] = {BO->getLHS()->IgnoreParenCasts(),
+  BO->getRHS()->IgnoreParenCasts()};
+  const Expr *EO[2] = {E[1], E[0]};
+  for (int i = 0; i < 2; i++) {
+if (isa(E[i]) || isa(E[i]) ||
+(isa(E[i]) &&
+ dyn_cast(E[i])->getValue() == 0)) {
+  ret = EO[i];
+  break;
+}
+  }
+  return ret;
+}
+
+const Expr *hasThrowingNew(const BinaryOperator *BO) const {
+  const Expr *ret = nullptr;
+  const Expr *rhs = BO->getRHS()->IgnoreParenCasts();
+  if (isa(rhs)) {
+auto NE = static_cast(rhs);
+if (!NE->shouldNullCheckAllocation()) {
+  ret = BO->getLHS()->IgnoreParenCasts();
+}
+  }
+  return ret;
+}
+
+bool hasThrowingNew(const VarDecl *VD) const {
+  bool ret = false;
+  const Expr *E = VD->getInit();
+  if (E != nullptr) {
+E = E->IgnoreParenCasts();
+if (isa(E)) {
+  auto NE = static_cast(E);
+  if (!NE->shouldNullCheckAllocation()) {
+ret = true;
+  }
+}
+  }
+  return ret;
+}
+
+  public:
+explicit Walker(BugReporter , const DeadStatusChecker *C,
+AnalysisDeclContext *AC)
+: BR(BR), Checker(C), AC(AC) {}
+bool VisitBinaryOperator(const BinaryOperator *BO) {
+  const Expr *E = nullptr;
+  BinaryOperator::Opcode Op = BO->getOpcode();
+  if (BinaryOperator::isEqualityOp(Op)) {
+E = hasNullptr(BO);
+  } else if (BinaryOperator::isAssignmentOp(Op)) {
+E = hasThrowingNew(BO);
+  }
+  if (E != nullptr) {
+if (isa(E)) {
+  auto DR = static_cast(E);
+  if 

[PATCH] D47554: [analyzer] Check for dead/impossible status checks

[Tom Rix via Phabricator via cfe-commits]

trixirt updated this revision to Diff 149868.
trixirt added a comment.
Herald added a subscriber: klimek.

Convert to AST Visitor

Remove Ctx parameter from shouldNullCheckAllocation

Add a shouldNullCheckAllocation ASTMatcher


Repository:
  rC Clang

https://reviews.llvm.org/D47554

Files:
  include/clang/AST/ExprCXX.h
  include/clang/ASTMatchers/ASTMatchers.h
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/AST/ExprCXX.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  lib/CodeGen/CGExprCXX.cpp
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/DeadStatusChecker.cpp
  test/Analysis/dead-status-new-nullptr.cpp

Index: test/Analysis/dead-status-new-nullptr.cpp
===
--- /dev/null
+++ test/Analysis/dead-status-new-nullptr.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=deadcode.DeadStatus -verify %s
+
+void *t1(unsigned n, bool *fail);
+void *t1(unsigned n, bool *fail) {
+  char *m = new char[n];
+  if (nullptr == m) // expected-warning{{This variable can never be a nullptr}}
+*fail = true;
+  else
+*fail = false;
+  return m;
+}
+
+void *t2(unsigned n, bool *fail);
+void *t2(unsigned n, bool *fail) {
+  char *m = new char[n];
+  if (m == nullptr) // expected-warning{{This variable can never be a nullptr}}
+*fail = true;
+  else
+*fail = false;
+  return m;
+}
+
+// a variant of nullptr
+void *t3(unsigned n, bool *fail);
+void *t3(unsigned n, bool *fail) {
+  char *m = new char[n];
+  if (m == 0) // expected-warning{{This variable can never be a nullptr}}
+*fail = true;
+  else
+*fail = false;
+  return m;
+}
+
+// a variant of nullptr
+#define NULL __null
+void *t4(unsigned n, bool *fail);
+void *t4(unsigned n, bool *fail) {
+  char *m = new char[n];
+  if (m == NULL) // expected-warning{{This variable can never be a nullptr}}
+*fail = true;
+  else
+*fail = false;
+  return m;
+}
Index: lib/StaticAnalyzer/Checkers/DeadStatusChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/DeadStatusChecker.cpp
@@ -0,0 +1,145 @@
+//==- DeadStatusChecker.cpp - Check for impossible status -*- C++ -*-==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file defines a DeadStatus, a checker that looks for impossible
+//  status checks.
+//
+//===--===//
+
+#include "ClangSACheckers.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+class DeadStatusChecker : public Checker {
+  class Walker : public RecursiveASTVisitor {
+BugReporter 
+const DeadStatusChecker *Checker;
+AnalysisDeclContext *AC;
+llvm::SmallPtrSet throwingNews;
+
+const Expr *hasNullptr(const BinaryOperator *BO) const {
+  const Expr *ret = nullptr;
+  const Expr *E[2] = {BO->getLHS()->IgnoreParenCasts(),
+  BO->getRHS()->IgnoreParenCasts()};
+  const Expr *EO[2] = {E[1], E[0]};
+  for (int i = 0; i < 2; i++) {
+bool ok = false;
+if (isa(E[i]) || isa(E[i]) ||
+(isa(E[i]) &&
+ dyn_cast(E[i])->getValue() == 0)) {
+  ok = true;
+}
+if (ok) {
+  ret = EO[i];
+  break;
+}
+  }
+  return ret;
+}
+
+const Expr *hasThrowingNew(const BinaryOperator *BO) const {
+  const Expr *ret = nullptr;
+  const Expr *rhs = BO->getRHS()->IgnoreParenCasts();
+  if (isa(rhs)) {
+auto NE = static_cast(rhs);
+if (!NE->shouldNullCheckAllocation()) {
+  ret = BO->getLHS()->IgnoreParenCasts();
+}
+  }
+  return ret;
+}
+
+bool hasThrowingNew(const VarDecl *VD) const {
+  bool ret = false;
+  const Expr *E = VD->getInit();
+  if (E != nullptr) {
+E = E->IgnoreParenCasts();
+if (isa(E)) {
+  auto NE = static_cast(E);
+  if (!NE->shouldNullCheckAllocation()) {
+ret = true;
+  }
+}
+  }
+  return ret;
+}
+
+  public:
+explicit Walker(BugReporter , const DeadStatusChecker *C,
+AnalysisDeclContext *AC)
+: BR(BR), Checker(C), AC(AC) {}
+bool VisitBinaryOperator(const BinaryOperator *BO) {
+  const Expr *E = nullptr;
+  BinaryOperator::Opcode Op = BO->getOpcode();
+  foo();
+  if (BinaryOperator::isEqualityOp(Op)) {
+E = hasNullptr(BO);
+

[PATCH] D47554: [analyzer] Check for dead/impossible status checks

[Tom Rix via Phabricator via cfe-commits]

trixirt created this revision.
trixirt added a reviewer: dcoughlin.
Herald added subscribers: cfe-commits, a.sidorin, szepet, xazax.hun, mgorny.
Herald added a reviewer: george.karpenkov.

The c++ 'new' function has several variants.
Only the no-throw versions of these return a nullptr on failure.
Error handling based on tha nullptr check for the throw version
is dead code.

ex/

char *n = new char[x];
if (n == nullptr)  // this is dead/impossible code

  do_something();


Repository:
  rC Clang

https://reviews.llvm.org/D47554

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/DeadStatusChecker.cpp
  test/Analysis/dead-status-new-nullptr.cpp

Index: test/Analysis/dead-status-new-nullptr.cpp
===
--- /dev/null
+++ test/Analysis/dead-status-new-nullptr.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=deadcode.DeadStatus -verify %s
+
+void *t1(unsigned n, bool *fail);
+void *t1(unsigned n, bool *fail) {
+  char *m = new char[n];
+  if (nullptr == m) // expected-warning{{This variable can never be a nullptr}}
+*fail = true;
+  else
+*fail = false;
+  return m;
+}
+
+void *t2(unsigned n, bool *fail);
+void *t2(unsigned n, bool *fail) {
+  char *m = new char[n];
+  if (m == nullptr) // expected-warning{{This variable can never be a nullptr}}
+*fail = true;
+  else
+*fail = false;
+  return m;
+}
+
+// a variant of nullptr
+void *t3(unsigned n, bool *fail);
+void *t3(unsigned n, bool *fail) {
+  char *m = new char[n];
+  if (m == 0) // expected-warning{{This variable can never be a nullptr}}
+*fail = true;
+  else
+*fail = false;
+  return m;
+}
+
+// a variant of nullptr
+#define NULL __null
+void *t4(unsigned n, bool *fail);
+void *t4(unsigned n, bool *fail) {
+  char *m = new char[n];
+  if (m == NULL) // expected-warning{{This variable can never be a nullptr}}
+*fail = true;
+  else
+*fail = false;
+  return m;
+}
Index: lib/StaticAnalyzer/Checkers/DeadStatusChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/DeadStatusChecker.cpp
@@ -0,0 +1,117 @@
+//==- DeadStatusChecker.cpp - Check for impossible status -*- C++ -*-==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file defines DeadStatus, a checker that looks for impossible
+//  status checks.
+//
+//===--===//
+
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+class DeadStatusChecker : public Checker,
+ check::PreStmt> {
+  std::unique_ptr NullptrCheckBug;
+  void reportNullptrCheck(SymbolRef Sym, const BinaryOperator *B,
+  CheckerContext ) const;
+  bool isNullptr(const Expr *E, CheckerContext ) const;
+
+public:
+  DeadStatusChecker();
+  void checkPostStmt(const CXXNewExpr *NE, CheckerContext ) const;
+  void checkPreStmt(const BinaryOperator *B, CheckerContext ) const;
+};
+} // namespace
+
+REGISTER_SET_WITH_PROGRAMSTATE(NewThrowSymbols, SymbolRef)
+
+void DeadStatusChecker::reportNullptrCheck(SymbolRef Sym,
+   const BinaryOperator *B,
+   CheckerContext ) const {
+  ExplodedNode *N = C.generateErrorNode();
+  if (!N)
+return;
+
+  auto report = llvm::make_unique(
+  *NullptrCheckBug, "This variable can never be a nullptr", N);
+  report->addRange(B->getSourceRange());
+  C.emitReport(std::move(report));
+}
+
+bool DeadStatusChecker::isNullptr(const Expr *E, CheckerContext ) const {
+  bool ret = false;
+  if (E) {
+E = E->IgnoreParenImpCasts();
+SVal S = C.getSVal(E);
+if (S.getAs()) {
+  DefinedOrUnknownSVal V = S.castAs();
+  if (V.isZeroConstant())
+ret = true;
+}
+  }
+  return ret;
+}
+
+DeadStatusChecker::DeadStatusChecker() {
+  NullptrCheckBug.reset(
+  new BugType(this, "Dead nullptr check", "C++ API Error"));
+}
+
+void DeadStatusChecker::checkPreStmt(const BinaryOperator *B,
+ CheckerContext ) const {
+  if (!B->isEqualityOp())
+return;
+
+  const Expr *lhs = B->getLHS()->IgnoreParenImpCasts();
+  const Expr *rhs = B->getRHS()->IgnoreParenImpCasts();
+  const DeclRefExpr *DRE = nullptr;
+  SVal V;
+
+  if (isNullptr(B->getLHS(), C)) {
+if (rhs && isa(rhs)) {
+  DRE = 

[PATCH] D41881: [analyzer] Flag bcmp, bcopy and bzero as obsolete

[Tom Rix via Phabricator via cfe-commits]

trixirt added a comment.
Herald added a reviewer: george.karpenkov.

I need someone to commit this..


Repository:
  rC Clang

https://reviews.llvm.org/D41881



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

[PATCH] D47260: Testcase for dwarf 5 crash/assert when calculating a checksum for an expansion

[Tom Rix via Phabricator via cfe-commits]

trixirt added a comment.

commandeer away!
Sure that is fine.


Repository:
  rC Clang

https://reviews.llvm.org/D47260



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

[PATCH] D47260: Testcase for dwarf 5 crash/assert when calculating a checksum for an expansion

[Tom Rix via Phabricator via cfe-commits]

trixirt updated this revision to Diff 148228.
trixirt added a comment.

Rename test to dwarf5-expansion-checksum.c


Repository:
  rC Clang

https://reviews.llvm.org/D47260

Files:
  test/CodeGen/dwarf5-expansion-checksum.c


Index: test/CodeGen/dwarf5-expansion-checksum.c
===
--- /dev/null
+++ test/CodeGen/dwarf5-expansion-checksum.c
@@ -0,0 +1,10 @@
+// RUN: %clang -c -gdwarf-5 %s  -o /dev/null
+// Don't crash calculating the checksum of string.h
+#if __has_include("/usr/include/string.h")
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+#if 8
+void __NTH() {}
+#endif
+#endif


Index: test/CodeGen/dwarf5-expansion-checksum.c
===
--- /dev/null
+++ test/CodeGen/dwarf5-expansion-checksum.c
@@ -0,0 +1,10 @@
+// RUN: %clang -c -gdwarf-5 %s  -o /dev/null
+// Don't crash calculating the checksum of string.h
+#if __has_include("/usr/include/string.h")
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+#if 8
+void __NTH() {}
+#endif
+#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47260: Testcase for dwarf 5 crash/assert when calculating a checksum for an expansion

[Tom Rix via Phabricator via cfe-commits]

trixirt created this revision.
trixirt added a reviewer: probinson.
Herald added subscribers: cfe-commits, JDevlieghere, aprantl.

Test is a reduction from running csmith with -gdwarf-5
The checksums are required for all 'files'.
The clang source manager does not provide them for expansions.


Repository:
  rC Clang

https://reviews.llvm.org/D47260

Files:
  test/CodeGen/2018-05-20-ExpansionChecksum.c


Index: test/CodeGen/2018-05-20-ExpansionChecksum.c
===
--- /dev/null
+++ test/CodeGen/2018-05-20-ExpansionChecksum.c
@@ -0,0 +1,10 @@
+// RUN: %clang -c -gdwarf-5 %s  -o /dev/null
+// Don't crash calculating the checksum of string.h
+#if __has_include("/usr/include/string.h")
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+#if 8
+void __NTH() {}
+#endif
+#endif


Index: test/CodeGen/2018-05-20-ExpansionChecksum.c
===
--- /dev/null
+++ test/CodeGen/2018-05-20-ExpansionChecksum.c
@@ -0,0 +1,10 @@
+// RUN: %clang -c -gdwarf-5 %s  -o /dev/null
+// Don't crash calculating the checksum of string.h
+#if __has_include("/usr/include/string.h")
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+#if 8
+void __NTH() {}
+#endif
+#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46791: Make -gsplit-dwarf generally available

[Tom Rix via Phabricator via cfe-commits]

trixirt updated this revision to Diff 146970.
trixirt added a comment.

Improve comment in CLANG_DEFAULT_OBJCOPY to specify what is required of objcopy.
Remove the llvm- prefix from testing to allow for override of default value.


https://reviews.llvm.org/D46791

Files:
  CMakeLists.txt
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/FreeBSD.cpp
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/split-debug.c
  test/Driver/split-debug.s

Index: test/Driver/split-debug.s
===
--- test/Driver/split-debug.s
+++ test/Driver/split-debug.s
@@ -6,12 +6,17 @@
 // CHECK-ACTIONS: objcopy{{.*}}--extract-dwo{{.*}}"split-debug.dwo"
 // CHECK-ACTIONS: objcopy{{.*}}--strip-dwo{{.*}}"split-debug.o"
 
+// RUN: %clang -target x86_64-unknown-freebsd11.0 -gsplit-dwarf -c -### %s 2> %t
+// RUN: FileCheck -check-prefix=FREEBSD-CHECK-ACTIONS < %t %s
+//
+// FREEBSD-CHECK-ACTIONS: objcopy{{.*}}--extract-dwo{{.*}}"split-debug.dwo"
+// FREEBSD-CHECK-ACTIONS: objcopy{{.*}}--strip-dwo{{.*}}"split-debug.o"
 
 // RUN: %clang -target x86_64-macosx -gsplit-dwarf -c -### %s 2> %t
-// RUN: FileCheck -check-prefix=CHECK-NO-ACTIONS < %t %s
+// RUN: FileCheck -check-prefix=MACOSX-CHECK-ACTIONS < %t %s
 //
-// CHECK-NO-ACTIONS-NOT: -split-dwarf
-
+// MACOSX-CHECK-ACTIONS: objcopy{{.*}}--extract-dwo{{.*}}"split-debug.dwo"
+// MACOSX-CHECK-ACTIONS: objcopy{{.*}}--strip-dwo{{.*}}"split-debug.o"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -o Bad.x -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-BAD < %t %s
Index: test/Driver/split-debug.c
===
--- test/Driver/split-debug.c
+++ test/Driver/split-debug.c
@@ -1,17 +1,31 @@
 // Check that we split debug output properly
 //
+// Linux
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -c -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-ACTIONS < %t %s
 //
 // CHECK-ACTIONS: objcopy{{.*}}--extract-dwo{{.*}}"split-debug.dwo"
 // CHECK-ACTIONS: objcopy{{.*}}--strip-dwo{{.*}}"split-debug.o"
 
+// FreeBSD
+// RUN: %clang -fno-integrated-as -target x86_64-unknown-freebsd11.0 -gsplit-dwarf -c -### %s 2> %t
+// RUN: FileCheck -check-prefix=FREEBSD-NOIA-CHECK-ACTIONS < %t %s
+//
+// FREEBSD-NOIA-CHECK-ACTIONS: objcopy{{.*}}--extract-dwo{{.*}}"split-debug.dwo"
+// FREEBSD-NOIA-CHECK-ACTIONS: objcopy{{.*}}--strip-dwo{{.*}}"split-debug.o"
+//
+// RUN: %clang -fintegrated-as -target x86_64-unknown-freebsd11.0 -gsplit-dwarf -c -### %s 2> %t
+// RUN: FileCheck -check-prefix=FREEBSD-IA-CHECK-ACTIONS < %t %s
+//
+// FREEBSD-IA-CHECK-ACTIONS: objcopy{{.*}}--extract-dwo{{.*}}"split-debug.dwo"
+// FREEBSD-IA-CHECK-ACTIONS: objcopy{{.*}}--strip-dwo{{.*}}"split-debug.o"
 
+// Macosx
 // RUN: %clang -target x86_64-macosx -gsplit-dwarf -c -### %s 2> %t
-// RUN: FileCheck -check-prefix=CHECK-NO-ACTIONS < %t %s
+// RUN: FileCheck -check-prefix=MACOSX-CHECK-ACTIONS < %t %s
 //
-// CHECK-NO-ACTIONS-NOT: -split-dwarf
-
+// MACOSX-CHECK-ACTIONS: objcopy{{.*}}--extract-dwo{{.*}}"split-debug.dwo"
+// MACOSX-CHECK-ACTIONS: objcopy{{.*}}--strip-dwo{{.*}}"split-debug.o"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -o Bad.x -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-BAD < %t %s
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -784,9 +784,7 @@
 
   // Handle the debug info splitting at object creation time if we're
   // creating an object.
-  // TODO: Currently only works on linux with newer objcopy.
-  if (Args.hasArg(options::OPT_gsplit_dwarf) &&
-  getToolChain().getTriple().isOSLinux())
+  if (Args.hasArg(options::OPT_gsplit_dwarf))
 SplitDebugInfo(getToolChain(), C, *this, JA, Args, Output,
SplitDebugName(Args, Inputs[0]));
 }
Index: lib/Driver/ToolChains/FreeBSD.cpp
===
--- lib/Driver/ToolChains/FreeBSD.cpp
+++ lib/Driver/ToolChains/FreeBSD.cpp
@@ -115,6 +115,12 @@
 
   const char *Exec = Args.MakeArgString(getToolChain().GetProgramPath("as"));
   C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs));
+
+  // Handle the debug info splitting at object creation time if we're
+  // creating an object.
+  if (Args.hasArg(options::OPT_gsplit_dwarf))
+SplitDebugInfo(getToolChain(), C, *this, JA, Args, Output,
+   SplitDebugName(Args, Inputs[0]));
 }
 
 void freebsd::Linker::ConstructJob(Compilation , const JobAction ,
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3020,16 +3020,13 @@
 
   // -gsplit-dwarf should turn on -g and enable the backend dwarf
   // splitting and extraction.
-  // FIXME: Currently only works on Linux.
-  if (T.isOSLinux()) {
-if 

[PATCH] D46791: Make -gsplit-dwarf generally available

[Tom Rix via Phabricator via cfe-commits]

trixirt created this revision.
trixirt added reviewers: echristo, jakehehrlich, emaste.
Herald added subscribers: cfe-commits, JDevlieghere, krytarowski, aprantl, 
mgorny.
Herald added a reviewer: alexshap.

Change CLANG_DEFAULT_OBJCOPY from objcopy to llvm-objcopy
Remove is-linux checks
Add dwarf splitting to FreeBSD's assembler job.


Repository:
  rC Clang

https://reviews.llvm.org/D46791

Files:
  CMakeLists.txt
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/FreeBSD.cpp
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/split-debug.c
  test/Driver/split-debug.s

Index: test/Driver/split-debug.s
===
--- test/Driver/split-debug.s
+++ test/Driver/split-debug.s
@@ -6,12 +6,17 @@
 // CHECK-ACTIONS: objcopy{{.*}}--extract-dwo{{.*}}"split-debug.dwo"
 // CHECK-ACTIONS: objcopy{{.*}}--strip-dwo{{.*}}"split-debug.o"
 
+// RUN: %clang -target x86_64-unknown-freebsd11.0 -gsplit-dwarf -c -### %s 2> %t
+// RUN: FileCheck -check-prefix=FREEBSD-CHECK-ACTIONS < %t %s
+//
+// FREEBSD-CHECK-ACTIONS: llvm-objcopy{{.*}}--extract-dwo{{.*}}"split-debug.dwo"
+// FREEBSD-CHECK-ACTIONS: llvm-objcopy{{.*}}--strip-dwo{{.*}}"split-debug.o"
 
 // RUN: %clang -target x86_64-macosx -gsplit-dwarf -c -### %s 2> %t
-// RUN: FileCheck -check-prefix=CHECK-NO-ACTIONS < %t %s
+// RUN: FileCheck -check-prefix=MACOSX-CHECK-ACTIONS < %t %s
 //
-// CHECK-NO-ACTIONS-NOT: -split-dwarf
-
+// MACOSX-CHECK-ACTIONS: llvm-objcopy{{.*}}--extract-dwo{{.*}}"split-debug.dwo"
+// MACOSX-CHECK-ACTIONS: llvm-objcopy{{.*}}--strip-dwo{{.*}}"split-debug.o"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -o Bad.x -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-BAD < %t %s
Index: test/Driver/split-debug.c
===
--- test/Driver/split-debug.c
+++ test/Driver/split-debug.c
@@ -1,17 +1,31 @@
 // Check that we split debug output properly
 //
+// Linux
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -c -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-ACTIONS < %t %s
 //
 // CHECK-ACTIONS: objcopy{{.*}}--extract-dwo{{.*}}"split-debug.dwo"
 // CHECK-ACTIONS: objcopy{{.*}}--strip-dwo{{.*}}"split-debug.o"
 
+// FreeBSD
+// RUN: %clang -fno-integrated-as -target x86_64-unknown-freebsd11.0 -gsplit-dwarf -c -### %s 2> %t
+// RUN: FileCheck -check-prefix=FREEBSD-NOIA-CHECK-ACTIONS < %t %s
+//
+// FREEBSD-NOIA-CHECK-ACTIONS: llvm-objcopy{{.*}}--extract-dwo{{.*}}"split-debug.dwo"
+// FREEBSD-NOIA-CHECK-ACTIONS: llvm-objcopy{{.*}}--strip-dwo{{.*}}"split-debug.o"
+//
+// RUN: %clang -fintegrated-as -target x86_64-unknown-freebsd11.0 -gsplit-dwarf -c -### %s 2> %t
+// RUN: FileCheck -check-prefix=FREEBSD-IA-CHECK-ACTIONS < %t %s
+//
+// FREEBSD-IA-CHECK-ACTIONS: llvm-objcopy{{.*}}--extract-dwo{{.*}}"split-debug.dwo"
+// FREEBSD-IA-CHECK-ACTIONS: llvm-objcopy{{.*}}--strip-dwo{{.*}}"split-debug.o"
 
+// Macosx
 // RUN: %clang -target x86_64-macosx -gsplit-dwarf -c -### %s 2> %t
-// RUN: FileCheck -check-prefix=CHECK-NO-ACTIONS < %t %s
+// RUN: FileCheck -check-prefix=MACOSX-CHECK-ACTIONS < %t %s
 //
-// CHECK-NO-ACTIONS-NOT: -split-dwarf
-
+// MACOSX-CHECK-ACTIONS: llvm-objcopy{{.*}}--extract-dwo{{.*}}"split-debug.dwo"
+// MACOSX-CHECK-ACTIONS: llvm-objcopy{{.*}}--strip-dwo{{.*}}"split-debug.o"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -o Bad.x -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-BAD < %t %s
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -784,9 +784,7 @@
 
   // Handle the debug info splitting at object creation time if we're
   // creating an object.
-  // TODO: Currently only works on linux with newer objcopy.
-  if (Args.hasArg(options::OPT_gsplit_dwarf) &&
-  getToolChain().getTriple().isOSLinux())
+  if (Args.hasArg(options::OPT_gsplit_dwarf))
 SplitDebugInfo(getToolChain(), C, *this, JA, Args, Output,
SplitDebugName(Args, Inputs[0]));
 }
Index: lib/Driver/ToolChains/FreeBSD.cpp
===
--- lib/Driver/ToolChains/FreeBSD.cpp
+++ lib/Driver/ToolChains/FreeBSD.cpp
@@ -115,6 +115,12 @@
 
   const char *Exec = Args.MakeArgString(getToolChain().GetProgramPath("as"));
   C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs));
+
+  // Handle the debug info splitting at object creation time if we're
+  // creating an object.
+  if (Args.hasArg(options::OPT_gsplit_dwarf))
+SplitDebugInfo(getToolChain(), C, *this, JA, Args, Output,
+   SplitDebugName(Args, Inputs[0]));
 }
 
 void freebsd::Linker::ConstructJob(Compilation , const JobAction ,
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3020,16 +3020,13 

[PATCH] D44964: Change order of libclang_rt.profile link for freebsd

[Tom Rix via Phabricator via cfe-commits]

trixirt created this revision.
trixirt added reviewers: emaste, rsmith.
Herald added a subscriber: cfe-commits.

clang -static -coverage foo.c  fails because the -lclang_rt.profile-*.a has a 
dependency on libc but is placed after libc in the link line.
This change place -lclang_rt.profile before -lc.


Repository:
  rC Clang

https://reviews.llvm.org/D44964

Files:
  lib/Driver/ToolChains/FreeBSD.cpp


Index: lib/Driver/ToolChains/FreeBSD.cpp
===
--- lib/Driver/ToolChains/FreeBSD.cpp
+++ lib/Driver/ToolChains/FreeBSD.cpp
@@ -262,6 +262,8 @@
   bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
+  ToolChain.addProfileRTLibs(Args, CmdArgs);
+
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
 addOpenMPRuntime(CmdArgs, ToolChain, Args);
 if (D.CCCIsCXX()) {
@@ -329,8 +331,6 @@
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtn.o")));
   }
 
-  ToolChain.addProfileRTLibs(Args, CmdArgs);
-
   const char *Exec = Args.MakeArgString(getToolChain().GetLinkerPath());
   C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs));
 }


Index: lib/Driver/ToolChains/FreeBSD.cpp
===
--- lib/Driver/ToolChains/FreeBSD.cpp
+++ lib/Driver/ToolChains/FreeBSD.cpp
@@ -262,6 +262,8 @@
   bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
+  ToolChain.addProfileRTLibs(Args, CmdArgs);
+
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
 addOpenMPRuntime(CmdArgs, ToolChain, Args);
 if (D.CCCIsCXX()) {
@@ -329,8 +331,6 @@
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtn.o")));
   }
 
-  ToolChain.addProfileRTLibs(Args, CmdArgs);
-
   const char *Exec = Args.MakeArgString(getToolChain().GetLinkerPath());
   C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs));
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44447: Treat libclang_rt.* as normal library if the user uses --sysroot=

[Tom Rix via Phabricator via cfe-commits]

trixirt created this revision.
trixirt added reviewers: rsmith, atanasyan, vkalintiris.
trixirt added a project: clang.
Herald added a subscriber: cfe-commits.

In cross compiling, one normally assumes that the user supplied sysroot 
contains all of the libraries needed in the link.  And so the linker adjusts 
the -L paths to look in the sysroot.
However because libclang_rt.* is supplied at link time as a full path, the link 
can not find it in the sysroot.

This change will churn the user's current experience and the unit tests.
I will take take of the tests, if folks are ok with the change.


Repository:
  rC Clang

https://reviews.llvm.org/D7

Files:
  lib/Driver/ToolChain.cpp


Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -356,11 +356,17 @@
   TT.isWindowsMSVCEnvironment() || TT.isWindowsItaniumEnvironment();
 
   StringRef Arch = getArchNameForCompilerRTLib(*this, Args);
-  const char *Prefix = IsITANMSVCWindows ? "" : "lib";
-  const char *Suffix = Shared ? (Triple.isOSWindows() ? ".dll" : ".so")
+  const char *Prefix = "-l";
+  const char *Suffix = "";
+  SmallString<128> Path;
+
+  if (getDriver().SysRoot.empty()) {
+Prefix = IsITANMSVCWindows ? "" : "lib";
+Suffix = Shared ? (Triple.isOSWindows() ? ".dll" : ".so")
   : (IsITANMSVCWindows ? ".lib" : ".a");
+Path = getCompilerRTPath();
+  }
 
-  SmallString<128> Path(getCompilerRTPath());
   llvm::sys::path::append(Path, Prefix + Twine("clang_rt.") + Component + "-" +
 Arch + Env + Suffix);
   return Path.str();


Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -356,11 +356,17 @@
   TT.isWindowsMSVCEnvironment() || TT.isWindowsItaniumEnvironment();
 
   StringRef Arch = getArchNameForCompilerRTLib(*this, Args);
-  const char *Prefix = IsITANMSVCWindows ? "" : "lib";
-  const char *Suffix = Shared ? (Triple.isOSWindows() ? ".dll" : ".so")
+  const char *Prefix = "-l";
+  const char *Suffix = "";
+  SmallString<128> Path;
+
+  if (getDriver().SysRoot.empty()) {
+Prefix = IsITANMSVCWindows ? "" : "lib";
+Suffix = Shared ? (Triple.isOSWindows() ? ".dll" : ".so")
   : (IsITANMSVCWindows ? ".lib" : ".a");
+Path = getCompilerRTPath();
+  }
 
-  SmallString<128> Path(getCompilerRTPath());
   llvm::sys::path::append(Path, Prefix + Twine("clang_rt.") + Component + "-" +
 Arch + Env + Suffix);
   return Path.str();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42310: Formalize FreeBSD support of compiler rt

[Tom Rix via Phabricator via cfe-commits]

trixirt added a comment.

The symlinks were useful in the transition from gcc to clang.
Now they don't serve any purpose.
Clang uses libcompiler_rt, not libgcc.


Repository:
  rC Clang

https://reviews.llvm.org/D42310



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

[PATCH] D42310: Formalize FreeBSD support of compiler rt

[Tom Rix via Phabricator via cfe-commits]

trixirt added a comment.

I think it is overkill to look for fbsd 8, it was eol-ed 2015, and 9 in 2016.
A native build of clang 7 on fbsd 8 would be difficult to pull off,  needing at 
least 1-3 intermediate clang's/gcc's


Repository:
  rC Clang

https://reviews.llvm.org/D42310



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

[PATCH] D42310: Formalize FreeBSD support of compiler rt

[Tom Rix via Phabricator via cfe-commits]

trixirt created this revision.
trixirt added reviewers: emaste, filcab, rsmith.
Herald added subscribers: cfe-commits, krytarowski.

FreeBSD's libgcc and libgcc_p are symlinks to libcompiler_rt and 
libcompiler_rt_p
Start using the compiler rt libs directly.


Repository:
  rC Clang

https://reviews.llvm.org/D42310

Files:
  lib/Driver/ToolChains/CommonArgs.cpp
  lib/Driver/ToolChains/FreeBSD.cpp
  lib/Driver/ToolChains/FreeBSD.h
  test/Driver/freebsd.c

Index: test/Driver/freebsd.c
===
--- test/Driver/freebsd.c
+++ test/Driver/freebsd.c
@@ -4,23 +4,38 @@
 // RUN:   | FileCheck --check-prefix=CHECK-ARM64 %s
 // CHECK-ARM64: "-cc1" "-triple" "aarch64-pc-freebsd11"
 // CHECK-ARM64: ld{{.*}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-ARM64: "--eh-frame-hdr" "-dynamic-linker" "{{.*}}ld-elf{{.*}}" "-o" "a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" "-L[[SYSROOT]]/usr/lib" "{{.*}}.o" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" "{{.*}}crtend.o" "{{.*}}crtn.o"
+// CHECK-ARM64: "--eh-frame-hdr" "-dynamic-linker" "{{.*}}ld-elf{{.*}}" "-o" "a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" "-L[[SYSROOT]]/usr/lib" "{{.*}}.o" "-lcompiler_rt" "--as-needed" "-lgcc_s" "--no-as-needed" "-lc" "-lcompiler_rt" "--as-needed" "-lgcc_s" "--no-as-needed" "{{.*}}crtend.o" "{{.*}}crtn.o"
+// RUN: %clang -no-canonical-prefixes \
+// RUN:   -target aarch64-pc-freebsd11 %s  \
+// RUN:   --sysroot=%S/Inputs/basic_freebsd64_tree -### -pg 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ARM64_PG %s
+// CHECK-ARM64_PG: "-cc1" "-triple" "aarch64-pc-freebsd11"
+// CHECK-ARM64_PG: ld{{.*}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-ARM64_PG: "--eh-frame-hdr" "-dynamic-linker" "{{.*}}ld-elf{{.*}}" "-o" "a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" "-L[[SYSROOT]]/usr/lib" "{{.*}}.o" "-lcompiler_rt_p" "-lgcc_eh_p" "-lc_p" "-lcompiler_rt_p" "-lgcc_eh_p" "{{.*}}crtend.o" "{{.*}}crtn.o"
+//
+// RUN: %clang -no-canonical-prefixes \
+// RUN:   -target aarch64-pc-freebsd11 %s  \
+// RUN:   --sysroot=%S/Inputs/basic_freebsd64_tree -### -pg -static 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ARM64_PG_STATIC %s
+// CHECK-ARM64_PG_STATIC: "-cc1" "-triple" "aarch64-pc-freebsd11"
+// CHECK-ARM64_PG_STATIC: ld{{.*}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-ARM64_PG_STATIC: "--eh-frame-hdr" "-Bstatic" "-o" "a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbeginT.o" "-L[[SYSROOT]]/usr/lib" "{{.*}}.o" "-lcompiler_rt_p" "-lgcc_eh" "-lc_p" "-lcompiler_rt_p" "-lgcc_eh" "{{.*}}crtend.o" "{{.*}}crtn.o"
 //
 // RUN: %clang -no-canonical-prefixes \
 // RUN:   -target powerpc-pc-freebsd8 %s\
 // RUN:   --sysroot=%S/Inputs/basic_freebsd_tree -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-PPC %s
 // CHECK-PPC: "-cc1" "-triple" "powerpc-pc-freebsd8"
 // CHECK-PPC: ld{{.*}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-PPC: "--eh-frame-hdr" "-dynamic-linker" "{{.*}}ld-elf{{.*}}" "-o" "a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" "-L[[SYSROOT]]/usr/lib" "{{.*}}.o" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" "{{.*}}crtend.o" "{{.*}}crtn.o"
+// CHECK-PPC: "--eh-frame-hdr" "-dynamic-linker" "{{.*}}ld-elf{{.*}}" "-o" "a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" "-L[[SYSROOT]]/usr/lib" "{{.*}}.o" "-lcompiler_rt" "--as-needed" "-lgcc_s" "--no-as-needed" "-lc" "-lcompiler_rt" "--as-needed" "-lgcc_s" "--no-as-needed" "{{.*}}crtend.o" "{{.*}}crtn.o"
 //
 // RUN: %clang -no-canonical-prefixes \
 // RUN:   -target powerpc64-pc-freebsd8 %s  \
 // RUN:   --sysroot=%S/Inputs/basic_freebsd64_tree -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-PPC64 %s
 // CHECK-PPC64: "-cc1" "-triple" "powerpc64-pc-freebsd8"
 // CHECK-PPC64: ld{{.*}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-PPC64: "--eh-frame-hdr" "-dynamic-linker" "{{.*}}ld-elf{{.*}}" "-o" "a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" "-L[[SYSROOT]]/usr/lib" "{{.*}}.o" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" "{{.*}}crtend.o" "{{.*}}crtn.o"
+// CHECK-PPC64: "--eh-frame-hdr" "-dynamic-linker" "{{.*}}ld-elf{{.*}}" "-o" "a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" "-L[[SYSROOT]]/usr/lib" "{{.*}}.o" "-lcompiler_rt" "--as-needed" "-lgcc_s" "--no-as-needed" "-lc" "-lcompiler_rt" "--as-needed" "-lgcc_s" "--no-as-needed" "{{.*}}crtend.o" "{{.*}}crtn.o"
 //
 //
 // Check that -m32 properly adjusts the toolchain flags.
Index: lib/Driver/ToolChains/FreeBSD.h
===
--- lib/Driver/ToolChains/FreeBSD.h
+++ lib/Driver/ToolChains/FreeBSD.h
@@ -74,7 +74,7 @@
   // Until dtrace (via CTF) and LLDB can deal with distributed debug info,
   // FreeBSD defaults to standalone/full 

[PATCH] D41881: [analyzer] Flag bcmp, bcopy and bzero as obsolete

[Tom Rix via Phabricator via cfe-commits]

trixirt created this revision.
trixirt added a reviewer: dcoughlin.
Herald added subscribers: cfe-commits, a.sidorin, szepet, xazax.hun.

bcmp, bcopy and bzero are obsolete functions.
Flag them as such so users will not use them.


Repository:
  rC Clang

https://reviews.llvm.org/D41881

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  test/Analysis/security-syntax-checks.m
  www/analyzer/available_checks.html

Index: www/analyzer/available_checks.html
===
--- www/analyzer/available_checks.html
+++ www/analyzer/available_checks.html
@@ -1173,6 +1173,40 @@
 
 
 
+security.insecureAPI.bcmp
+(C)
+Warn on uses of the bcmp function.
+
+
+void test() {
+  bcmp(ptr0, ptr1, n); // warn
+}
+
+
+
+security.insecureAPI.bcopy
+(C)
+Warn on uses of the bcopy function.
+
+
+void test() {
+  bcopy(src, dst, n); // warn
+}
+
+
+
+security.insecureAPI.bzero
+(C)
+Warn on uses of the bzero function.
+
+
+void test() {
+  bzero(ptr, n); // warn
+}
+
+
+
+
 security.insecureAPI.getpw
 (C)
 Warn on uses of the getpw function.
Index: test/Analysis/security-syntax-checks.m
===
--- test/Analysis/security-syntax-checks.m
+++ test/Analysis/security-syntax-checks.m
@@ -37,6 +37,27 @@
   for (FooType x = 10001.0f; x <= 10010.0f; x++ ) {} // expected-warning{{Variable 'x' with floating point type 'FooType'}}
 }
 
+// Obsolete function bcmp
+int bcmp(void *, void *, size_t);
+
+int test_bcmp(void *a, void *b, size_t n) {
+  return bcmp(a, b, n); // expected-warning{{The bcmp() function is obsoleted by memcmp()}}
+}
+
+// Obsolete function bcopy
+void bcopy(void *, void *, size_t);
+
+void test_bcopy(void *a, void *b, size_t n) {
+  bcopy(a, b, n); // expected-warning{{The bcopy() function is obsoleted by memcpy() or memmove(}}
+}
+
+// Obsolete function bzero
+void bzero(void *, size_t);
+
+void test_bzero(void *a, size_t n) {
+  bzero(a, n); // expected-warning{{The bzero() function is obsoleted by memset()}}
+}
+
 //  rule request: gets() buffer overflow
 // Part of recommendation: 300-BSI (buildsecurityin.us-cert.gov)
 char* gets(char *buf);
Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -37,6 +37,9 @@
 
 namespace {
 struct ChecksFilter {
+  DefaultBool check_bcmp;
+  DefaultBool check_bcopy;
+  DefaultBool check_bzero;
   DefaultBool check_gets;
   DefaultBool check_getpw;
   DefaultBool check_mktemp;
@@ -47,6 +50,9 @@
   DefaultBool check_FloatLoopCounter;
   DefaultBool check_UncheckedReturn;
 
+  CheckName checkName_bcmp;
+  CheckName checkName_bcopy;
+  CheckName checkName_bzero;
   CheckName checkName_gets;
   CheckName checkName_getpw;
   CheckName checkName_mktemp;
@@ -89,6 +95,9 @@
 
   // Checker-specific methods.
   void checkLoopConditionForFloat(const ForStmt *FS);
+  void checkCall_bcmp(const CallExpr *CE, const FunctionDecl *FD);
+  void checkCall_bcopy(const CallExpr *CE, const FunctionDecl *FD);
+  void checkCall_bzero(const CallExpr *CE, const FunctionDecl *FD);
   void checkCall_gets(const CallExpr *CE, const FunctionDecl *FD);
   void checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD);
   void checkCall_mktemp(const CallExpr *CE, const FunctionDecl *FD);
@@ -129,6 +138,9 @@
 
   // Set the evaluation function by switching on the callee name.
   FnCheck evalFunction = llvm::StringSwitch(Name)
+.Case("bcmp", ::checkCall_bcmp)
+.Case("bcopy", ::checkCall_bcopy)
+.Case("bzero", ::checkCall_bzero)
 .Case("gets", ::checkCall_gets)
 .Case("getpw", ::checkCall_getpw)
 .Case("mktemp", ::checkCall_mktemp)
@@ -296,6 +308,132 @@
 }
 
 //===--===//
+// Check: Any use of bcmp.
+// CWE-477: Use of Obsolete Functions
+// bcmp was deprecated in POSIX.1-2008
+//===--===//
+
+void WalkAST::checkCall_bcmp(const CallExpr *CE, const FunctionDecl *FD) {
+  if (!filter.check_bcmp)
+return;
+
+  const FunctionProtoType *FPT = FD->getType()->getAs();
+  if (!FPT)
+return;
+
+  // Verify that the function takes three arguments.
+  if (FPT->getNumParams() != 3)
+return;
+
+  for (int i = 0; i < 2; i++) {
+// Verify the first and second argument type is void*.
+const PointerType *PT = FPT->getParamType(i)->getAs();
+if (!PT)
+  return;
+
+if (PT->getPointeeType().getUnqualifiedType() != BR.getContext().VoidTy)
+  return;
+  }
+
+  // Verify the third argument type is integer.
+  if (!FPT->getParamType(2)->isIntegralOrUnscopedEnumerationType())
+return;
+
+  // Issue a warning.
+  PathDiagnosticLocation CELoc =
+