[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 344692.
mgartmann added a comment.

Revert `ReleaseNotes.rst` to its initial content in a try to fix the pre-build 
tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-avoid-std-io-outside-main.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s misc-avoid-std-io-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+} // namespace std
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+
+namespace arbitrary_namespace {
+std::Ostream cout{};
+std::Istream cin{};
+} // namespace arbitrary_namespace
+
+void anyNonMainFunction() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcin >> Foo;
+
+  arbitrary_namespace::cin >> Foo; // OK
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::printf("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  printf("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::puts("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  puts("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::putchar('m');
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  putchar('m');
+
+  char Input[5];
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::scanf("%s", Input);
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  scanf("%s", Input);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::getchar();
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be us

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 344711.
mgartmann added a comment.

Re-add description for this check in `ReleaseNotes.rst`.
Adjust `AvoidStdIoOutsideMainCheck.cpp` third matcher to call `hasAnyName()` 
with a vector.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-avoid-std-io-outside-main.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s misc-avoid-std-io-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+} // namespace std
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+
+namespace arbitrary_namespace {
+std::Ostream cout{};
+std::Istream cin{};
+} // namespace arbitrary_namespace
+
+void anyNonMainFunction() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcin >> Foo;
+
+  arbitrary_namespace::cin >> Foo; // OK
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::printf("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  printf("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::puts("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  puts("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::putchar('m');
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  putchar('m');
+
+  char Input[5];
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::scanf("%s", Input);
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  scanf("%s", Input);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 344726.
mgartmann added a comment.

Remove any parentheses and slashes from the check's section in 
`ReleaseNotes.rst` in order to try to fix the build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-avoid-std-io-outside-main.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s misc-avoid-std-io-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+} // namespace std
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+
+namespace arbitrary_namespace {
+std::Ostream cout{};
+std::Istream cin{};
+} // namespace arbitrary_namespace
+
+void anyNonMainFunction() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcin >> Foo;
+
+  arbitrary_namespace::cin >> Foo; // OK
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::printf("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  printf("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::puts("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  puts("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::putchar('m');
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  putchar('m');
+
+  char Input[5];
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::scanf("%s", Input);
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  scanf("%s", Input);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::getchar();
+  // CH

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 344731.
mgartmann added a comment.

Remove text from `ReleaseNotes.rst` to narrow down the cause for the failing 
build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-avoid-std-io-outside-main.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s misc-avoid-std-io-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+} // namespace std
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+
+namespace arbitrary_namespace {
+std::Ostream cout{};
+std::Istream cin{};
+} // namespace arbitrary_namespace
+
+void anyNonMainFunction() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcin >> Foo;
+
+  arbitrary_namespace::cin >> Foo; // OK
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::printf("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  printf("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::puts("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  puts("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::putchar('m');
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  putchar('m');
+
+  char Input[5];
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::scanf("%s", Input);
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  scanf("%s", Input);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::getchar();
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: wa

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann created this revision.
mgartmann added reviewers: aaron.ballman, njames93, alexfh.
mgartmann added a project: clang-tools-extra.
Herald added subscribers: shchenz, kbarton, xazax.hun, mgorny, nemanjai.
mgartmann requested review of this revision.

Finds base classes and structs whose destructor is neither public and virtual 
nor protected and non-virtual.
A base class's destructor should be specified in one of these ways to prevent 
undefined behaviour.

Fixes are available for user-declared and implicit destructors that are either 
public
and non-virtual or protected and virtual.

This check implements C.35 

 from the CppCoreGuidelines.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102325

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-base-class-destructor.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp
@@ -0,0 +1,131 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-virtual-base-class-destructor %t
+
+// CHECK-MESSAGES: :[[@LINE+1]]:8: warning: destructor of struct 'PrivateVirtualBaseStruct' is private and prevents using the type. Consider making it public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+struct PrivateVirtualBaseStruct {
+  virtual void f();
+
+private:
+  virtual ~PrivateVirtualBaseStruct(){};
+};
+
+struct PublicVirtualBaseStruct { // OK
+  virtual void f();
+  virtual ~PublicVirtualBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:8: warning: destructor of struct 'ProtectedVirtualBaseStruct' is protected and virtual. It should either be public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+struct ProtectedVirtualBaseStruct {
+  virtual void f();
+
+protected:
+  virtual ~ProtectedVirtualBaseStruct(){};
+  // CHECK-FIXES: ~ProtectedVirtualBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:8: warning: destructor of struct 'PrivateNonVirtualBaseStruct' is private and prevents using the type. Consider making it public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+struct PrivateNonVirtualBaseStruct {
+  virtual void f();
+
+private:
+  ~PrivateNonVirtualBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:8: warning: destructor of struct 'PublicNonVirtualBaseStruct' is public and non-virtual. It should either be public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+struct PublicNonVirtualBaseStruct {
+  virtual void f();
+  ~PublicNonVirtualBaseStruct(){};
+  // CHECK-FIXES: virtual ~PublicNonVirtualBaseStruct(){};
+};
+
+struct PublicNonVirtualNonBaseStruct { // OK according to C.35, since this struct does not have any virtual methods.
+  void f();
+  ~PublicNonVirtualNonBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+3]]:8: warning: destructor of struct 'PublicImplicitNonVirtualBaseStruct' is public and non-virtual. It should either be public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-FIXES: struct PublicImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicImplicitNonVirtualBaseStruct() = default;
+struct PublicImplicitNonVirtualBaseStruct {
+  virtual void f();
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of struct 'PublicASImplicitNonVirtualBaseStruct' is public and non-virtual. It should either be public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-FIXES: struct PublicASImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicASImplicitNonVirtualBaseStruct() = default;
+// CHECK-FIXES-NEXT: private:
+struct PublicASImplicitNonVirtualBaseStruct {
+private:
+  virtual void f();
+};
+
+struct ProtectedNonVirtualBaseStruct { // OK
+  virtual void f();
+
+protected:
+  ~ProtectedNonVirtualBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:7: warning: destructor of class 'PrivateVirtualBaseClass' is private and prevents using the type. Consider making it public and virtual or prot

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann marked 17 inline comments as done.
mgartmann added a comment.

In D102325#2754241 , @njames93 wrote:

> Whats the intended behaviour for derived classes and their destructors? Can 
> test be added to demonstrate that behaviour?

If a derived class inherits a `virtual` method from a base class, according to 
guideline C.35, it is a potential base class as well.
Therefore, such a derived class gets flagged by the check if its destructor is 
not specified correctly.

In order to flag derived classes that only inherit a `virtual` method but do 
not override it, the following matcher had to be added:

  ast_matchers::internal::Matcher inheritsVirtualMethod =
  hasAnyBase(hasType(cxxRecordDecl(has(cxxMethodDecl(isVirtual());

Test cases were added to demonstrate this behaviour.




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:129
+
+AccessSpecDecl *VirtualBaseClassDestructorCheck::getPublicASDecl(
+const CXXRecordDecl &StructOrClass) const {

njames93 wrote:
> Can be made static again. Also should probably return a `const AccessSpecDecl 
> *`
@njames93 If I get this correctly, only `check` and `addMatcher` should be 
member functions of a check's class. All the other aid methods should be static 
and not part of the class. 

Did I get this right?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-base-class-destructor.rst:55
+
+.. option:: IndentationWidth
+

Eugene.Zelenko wrote:
> Shouldn't `Clang-format` take care about such things?
As @njames93 also suggested leaving the indentation to `clang-format`, I 
removed this option and the indentation functionality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

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

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 345623.
mgartmann marked 2 inline comments as done.
mgartmann added a comment.

Incorporated Phabricator review feedback:

- added matchers and tests for subclasses with inherited virtual methods
- made aid methods static and not part of the check's class
- replaced `auto` with types where it was suggested
- adjusted diagnostic messages

Included new commits of upstream `main` branch to resolve build failure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-base-class-destructor.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp
@@ -0,0 +1,173 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-virtual-base-class-destructor %t
+
+// CHECK-MESSAGES: :[[@LINE+1]]:8: warning: destructor of 'PrivateVirtualBaseStruct' is private and prevents using the type. Consider making it public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+struct PrivateVirtualBaseStruct {
+  virtual void f();
+
+private:
+  virtual ~PrivateVirtualBaseStruct(){};
+};
+
+struct PublicVirtualBaseStruct { // OK
+  virtual void f();
+  virtual ~PublicVirtualBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:8: warning: destructor of 'ProtectedVirtualBaseStruct' is protected and virtual. It should either be public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+struct ProtectedVirtualBaseStruct {
+  virtual void f();
+
+protected:
+  virtual ~ProtectedVirtualBaseStruct(){};
+  // CHECK-FIXES: ~ProtectedVirtualBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:8: warning: destructor of 'PrivateNonVirtualBaseStruct' is private and prevents using the type. Consider making it public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+struct PrivateNonVirtualBaseStruct {
+  virtual void f();
+
+private:
+  ~PrivateNonVirtualBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:8: warning: destructor of 'PublicNonVirtualBaseStruct' is public and non-virtual. It should either be public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+struct PublicNonVirtualBaseStruct {
+  virtual void f();
+  ~PublicNonVirtualBaseStruct(){};
+  // CHECK-FIXES: virtual ~PublicNonVirtualBaseStruct(){};
+};
+
+struct PublicNonVirtualNonBaseStruct { // OK according to C.35, since this struct does not have any virtual methods.
+  void f();
+  ~PublicNonVirtualNonBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+3]]:8: warning: destructor of 'PublicImplicitNonVirtualBaseStruct' is public and non-virtual. It should either be public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-FIXES: struct PublicImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicImplicitNonVirtualBaseStruct() = default;
+struct PublicImplicitNonVirtualBaseStruct {
+  virtual void f();
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PublicASImplicitNonVirtualBaseStruct' is public and non-virtual. It should either be public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-FIXES: struct PublicASImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicASImplicitNonVirtualBaseStruct() = default;
+// CHECK-FIXES-NEXT: private:
+struct PublicASImplicitNonVirtualBaseStruct {
+private:
+  virtual void f();
+};
+
+struct ProtectedNonVirtualBaseStruct { // OK
+  virtual void f();
+
+protected:
+  ~ProtectedNonVirtualBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:7: warning: destructor of 'PrivateVirtualBaseClass' is private and prevents using the type. Consider making it public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+class PrivateVirtualBaseClass {
+  virtual void f();
+  virtual ~PrivateVirtualBaseClass(){};
+};
+
+class PublicVirtualBaseClass { // OK
+  virtual void f();
+
+public:
+  virtual ~PublicVirtualBaseClass(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:7: warning: destructor of 'Protect

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 345634.
mgartmann added a comment.

Fetched new commits from upstream `main` branch and resolved merge conflicts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-avoid-std-io-outside-main.rst

Index: clang-tools-extra/docs/clang-tidy/checks/misc-avoid-std-io-outside-main.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-avoid-std-io-outside-main.rst
@@ -0,0 +1,52 @@
+.. title:: clang-tidy - misc-avoid-std-io-outside-main
+
+misc-avoid-std-io-outside-main
+=
+
+Finds predefined standard stream objects like ``cin``, ``wcin``, ``cout``,
+``wcout``, ``cerr`` or ``wcerr``, that are used outside the ``main``
+function. It also finds uses of ``cstdio`` and ``stdio.h`` functions like
+``printf()`` outside the ``main`` function.
+
+For instance, in the following code, the use of ``std::cout`` and ``printf()``
+outside of ``main()`` would get flagged whereas the use of them inside
+``main()`` is not flagged:
+
+.. code-block:: c++
+
+  #include 
+  #include 
+
+  void some_function() {
+std::cout << "This triggers the check."; // NOK
+std::printf("This triggers the check."); // NOK
+  }
+
+  int main() {
+std::cout << "This does not trigger the check."; // OK
+std::printf("This does not trigger the check."); // OK
+  }
+
+Since the predefined standard stream objects are global objects, their use
+outside of ``main()`` worsens a program's testability and is thus discouraged.
+Instead, those objects should only be used inside ``main()``. They can then be
+passed as arguments to other functions like so:
+
+.. code-block:: c++
+
+  #include 
+
+  void some_function(std::istream & in, std::ostream & out) {
+out << "This does not trigger the check.";
+
+int i{0};
+in >> i;
+  }
+
+  int main() {
+some_function(std::cin, std::cout);
+  }
+
+In contrast to using ``std::cin`` and ``std::cout`` directly, in the above
+example, it is possible to inject mocked stream objects into
+``some_function()`` during testing.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -205,6 +205,7 @@
`llvmlibc-callee-namespace `_,
`llvmlibc-implementation-in-namespace `_,
`llvmlibc-restrict-system-libc-headers `_, "Yes"
+   `misc-misc-avoid-std-io-outside-main `_,
`misc-definitions-in-headers `_, "Yes"
`misc-misplaced-const `_,
`misc-new-delete-overloads `_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -119,6 +119,14 @@
 
   Finds calls to ``new`` with missing exception handler for ``std::bad_alloc``.
 
+- New :doc:`misc-avoid-std-io-outside-main
+  ` check.
+
+  Finds predefined standard stream objects like ``cin``, ``wcin``, ``cout``,
+  ``wcout``, ``cerr`` or ``wcerr``, that are used outside the ``main``
+  function. It also finds uses of ``cstdio`` and ``stdio.h`` functions like
+  ``printf()`` outside the ``main`` function.
+
 New check aliases
 ^
 
Index: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "AvoidStdIoOutsideMainCheck.h"
 #include "DefinitionsInHeadersCheck.h"
 #include "MisplacedConstCheck.h"
 #include "NewDeleteOverloadsCheck.h"
@@ -31,6 +32,8 @@
 class MiscModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+CheckFactories.registerCheck(
+"misc-avoid-std-io-outside-main");
 CheckFactories.registerCheck(
 "misc-definitions-in-headers");
 CheckFactories.registerCheck("misc-misplaced-const");
Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -4,6 +4,7 @@
   )
 
 add_clang_library(clangTidyMiscModule
+  AvoidStdIoOutsideMainCheck.c

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 345635.
mgartmann added a comment.

Resolved readability-identifier-naming warning, adjusted check's documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-base-class-destructor.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp
@@ -0,0 +1,173 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-virtual-base-class-destructor %t
+
+// CHECK-MESSAGES: :[[@LINE+1]]:8: warning: destructor of 'PrivateVirtualBaseStruct' is private and prevents using the type. Consider making it public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+struct PrivateVirtualBaseStruct {
+  virtual void f();
+
+private:
+  virtual ~PrivateVirtualBaseStruct(){};
+};
+
+struct PublicVirtualBaseStruct { // OK
+  virtual void f();
+  virtual ~PublicVirtualBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:8: warning: destructor of 'ProtectedVirtualBaseStruct' is protected and virtual. It should either be public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+struct ProtectedVirtualBaseStruct {
+  virtual void f();
+
+protected:
+  virtual ~ProtectedVirtualBaseStruct(){};
+  // CHECK-FIXES: ~ProtectedVirtualBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:8: warning: destructor of 'PrivateNonVirtualBaseStruct' is private and prevents using the type. Consider making it public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+struct PrivateNonVirtualBaseStruct {
+  virtual void f();
+
+private:
+  ~PrivateNonVirtualBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:8: warning: destructor of 'PublicNonVirtualBaseStruct' is public and non-virtual. It should either be public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+struct PublicNonVirtualBaseStruct {
+  virtual void f();
+  ~PublicNonVirtualBaseStruct(){};
+  // CHECK-FIXES: virtual ~PublicNonVirtualBaseStruct(){};
+};
+
+struct PublicNonVirtualNonBaseStruct { // OK according to C.35, since this struct does not have any virtual methods.
+  void f();
+  ~PublicNonVirtualNonBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+3]]:8: warning: destructor of 'PublicImplicitNonVirtualBaseStruct' is public and non-virtual. It should either be public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-FIXES: struct PublicImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicImplicitNonVirtualBaseStruct() = default;
+struct PublicImplicitNonVirtualBaseStruct {
+  virtual void f();
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PublicASImplicitNonVirtualBaseStruct' is public and non-virtual. It should either be public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-FIXES: struct PublicASImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicASImplicitNonVirtualBaseStruct() = default;
+// CHECK-FIXES-NEXT: private:
+struct PublicASImplicitNonVirtualBaseStruct {
+private:
+  virtual void f();
+};
+
+struct ProtectedNonVirtualBaseStruct { // OK
+  virtual void f();
+
+protected:
+  ~ProtectedNonVirtualBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:7: warning: destructor of 'PrivateVirtualBaseClass' is private and prevents using the type. Consider making it public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+class PrivateVirtualBaseClass {
+  virtual void f();
+  virtual ~PrivateVirtualBaseClass(){};
+};
+
+class PublicVirtualBaseClass { // OK
+  virtual void f();
+
+public:
+  virtual ~PublicVirtualBaseClass(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:7: warning: destructor of 'ProtectedVirtualBaseClass' is protected and virtual. It should either be public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+class ProtectedVirtualBaseClass {
+  virtual void f();
+
+protected:
+  virtual ~ProtectedVirtualBaseClass(){};
+  // CHECK-FIXES: ~

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Marco Gartmann via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-base-class-destructor.rst:16
+
+This check implements `C.35 
`_
 from the CppCoreGuidelines.
+

Eugene.Zelenko wrote:
> Links to documentation usually placed at the end. See other checks 
> documentation.
Thanks for pointing this out to me! 
I previously followed the style of other `cppcoreguideline` checks which did 
not have this line at the end of the file (e.g., [[ 
https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.html
 | cppcoreguidelines-prefer-member-initializer ]] or [[ 
https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.html
 | cppcoreguidelines-avoid-non-const-global-variables]].

I adjusted the documentation according to your suggestion and now placed this 
line at the end.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

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

[PATCH] D102779: [clang-tidy] cppcoreguidelines-explicit-constructor-and-conversion: new alias

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann created this revision.
Herald added subscribers: jeroen.dobbelaere, shchenz, kbarton, xazax.hun, 
nemanjai.
mgartmann requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Since the `google-explicit-constructor` check coincides with rule

Created alias `cppcoreguidelines-explicit-constructor-and-conversion`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102779

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst


Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -407,6 +407,7 @@
`cppcoreguidelines-avoid-c-arrays 
`_, `modernize-avoid-c-arrays 
`_,
`cppcoreguidelines-avoid-magic-numbers 
`_, `readability-magic-numbers 
`_,
`cppcoreguidelines-c-copy-assignment-signature 
`_, 
`misc-unconventional-assign-operator 
`_,
+   `cppcoreguidelines-explicit-constructor-and-conversion 
`_, 
`google-explicit-constructor `_, "Yes"
`cppcoreguidelines-explicit-virtual-functions 
`_, `modernize-use-override 
`_, "Yes"
`cppcoreguidelines-non-private-member-variables-in-classes 
`_, 
`misc-non-private-member-variables-in-classes 
`_,
`fuchsia-header-anon-namespaces `_, 
`google-build-namespaces `_,
Index: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion.rst
===
--- /dev/null
+++ 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion.rst
@@ -0,0 +1,14 @@
+.. title:: clang-tidy - cppcoreguidelines-explicit-constructor-and-conversion
+.. meta::
+   :http-equiv=refresh: 5;URL=google-explicit-constructor.html
+
+cppcoreguidelines-explicit-constructor
+==
+
+The cppcoreguidelines-explicit-constructor-and-conversion check is an alias,
+please see `google-explicit-constructor `_
+for more information.
+
+This check implements `C.46 
`_
+and `C.164 
`_
+from the CppCoreGuidelines.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -127,6 +127,11 @@
   :doc:`concurrency-thread-canceltype-asynchronous
   ` was added.
 
+- New alias :doc:`cppcoreguidelines-explicit-constructor-and-conversion
+  ` to
+  :doc:`google-explicit-constructor
+  ` was added.
+
 Changes in existing checks
 ^^
 
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "../google/ExplicitConstructorCheck.h"
 #include "../misc/NonPrivateMemberVariablesInClassesCheck.h"
 #include "../misc/UnconventionalAssignOperatorCheck.h"
 #include "../modernize/AvoidCArraysCheck.h"
@@ -52,6 +53,8 @@
 "cppcoreguidelines-avoid-magic-numbers");
 CheckFactories.registerCheck(
 "cppcoreguidelines-avoid-non-const-global-variables");
+CheckFactories.registerCheck(
+"cppcoreguidelines-explicit-constructor-and-conversion");
 CheckFactories.registerCheck(
 "cppcoreguidelines-explicit-virtual-functions");
 CheckFactories.registerCheck(


Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -407,6 +407,7 @@
`cppcoreguidelines-avoid-c-arrays `_, `modernize-avoid-c-arrays `_,
`cppcoreguidelines-avoid-magic-numbers `_, `readability-magic-numbers `_,
`cppcoreguidelines-c-copy-assignment-signature `_, `misc-unconventional-assign-operator `_,
+   `cppcoreguidelines-explicit-constructor-and-conversion `_, `google-explicit-constructor `_, "Yes"
`cppcoreguidelines-explicit-virtual-functions `_, `modernize-use-override `_, "Yes"
`cppcoreguidelines-non-private-member-variables-in-classes `_, `misc-non-private-member-variables-in

[PATCH] D102779: [clang-tidy] cppcoreguidelines-explicit-constructor-and-conversion: new alias

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 346640.
mgartmann edited the summary of this revision.
mgartmann added a comment.

Added `ConstructorWhitelist`  option to the `google-explicit-constructor` check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102779

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp
  clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion.rst
  clang-tools-extra/docs/clang-tidy/checks/google-explicit-constructor.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-constructorwhitelist-option.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-constructorwhitelist-option.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-constructorwhitelist-option.cpp
@@ -0,0 +1,99 @@
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: google-explicit-constructor %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN: ]}'
+
+// RUN: %check_clang_tidy -check-suffix=WHITELISTED %s \
+// RUN: google-explicit-constructor %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN:   {key: google-explicit-constructor.ConstructorWhitelist, value: "A;B"} \
+// RUN: ]}'
+
+struct A {
+  A() {}
+  A(int x, int y) {}
+
+  explicit A(void *x) {}
+  explicit A(void *x, void *y) {}
+
+  explicit A(const A &a) {}
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:12: warning: copy constructor should not be declared explicit [google-explicit-constructor]
+  // CHECK-FIXES-DEFAULT: {{^  }}A(const A &a) {}
+  // WHITELISTED: Warning disabled with ConstructorWhitelist=A
+
+  A(int x1);
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES-DEFAULT: {{^  }}explicit A(int x1);
+  // WHITELISTED: Warning disabled with ConstructorWhitelist=A
+
+  A(double x2, double y = 3.14) {}
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: constructors that are callable with a single argument must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES-DEFAULT: {{^  }}explicit A(double x2, double y = 3.14) {}
+  // WHITELISTED: Warning disabled with ConstructorWhitelist=A
+
+  template 
+  A(T &&...args);
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: constructors that are callable with a single argument
+  // CHECK-FIXES-DEFAULT: {{^  }}explicit A(T &&...args);
+  // WHITELISTED: Warning disabled with ConstructorWhitelist=A
+};
+
+inline A::A(int x1) {}
+
+struct B {
+  B() {}
+  B(int x, int y) {}
+
+  explicit B(void *x) {}
+  explicit B(void *x, void *y) {}
+
+  explicit B(const B &b) {}
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:12: warning: copy constructor should not be declared explicit [google-explicit-constructor]
+  // CHECK-FIXES-DEFAULT: {{^  }}B(const B &b) {}
+  // WHITELISTED: Warning disabled with ConstructorWhitelist=B
+
+  B(int x1);
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES-DEFAULT: {{^  }}explicit B(int x1);
+  // WHITELISTED: Warning disabled with ConstructorWhitelist=B
+
+  B(double x2, double y = 3.14) {}
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: constructors that are callable with a single argument must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES-DEFAULT: {{^  }}explicit B(double x2, double y = 3.14) {}
+  // WHITELISTED: Warning disabled with ConstructorWhitelist=B
+
+  template 
+  B(T &&...args);
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: constructors that are callable with a single argument
+  // CHECK-FIXES-DEFAULT: {{^  }}explicit B(T &&...args);
+  // WHITELISTED: Warning disabled with ConstructorWhitelist=B
+};
+
+struct C {
+  C() {}
+  C(int x, int y) {}
+
+  explicit C(void *x) {}
+  explicit C(void *x, void *y) {}
+
+  explicit C(const C &c) {}
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:12: warning: copy constructor should not be declared explicit [google-explicit-constructor]
+  // CHECK-MESSAGES-WHITELISTED: :[[@LINE-2]]:12: warning: copy constructor should not be declared explicit [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}C(const C &c) {}
+
+  C(int x1);
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional 

[PATCH] D102836: [clang] Fix Typo in AST Matcher Reference

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann created this revision.
mgartmann added a reviewer: klimek.
mgartmann added a project: clang.
mgartmann requested review of this revision.

In AST Matcher Reference 
, the example of 
matcher `hasDeclContext` contained a typo.

`cxxRcordDecl` was changed to `cxxRecordDecl`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102836

Files:
  clang/docs/LibASTMatchersReference.html


Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -7439,7 +7439,7 @@
 }
   }
 
-cxxRcordDecl(hasDeclContext(namedDecl(hasName("M" matches the
+cxxRecordDecl(hasDeclContext(namedDecl(hasName("M" matches the
 declaration of class D.
 
 


Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -7439,7 +7439,7 @@
 }
   }
 
-cxxRcordDecl(hasDeclContext(namedDecl(hasName("M" matches the
+cxxRecordDecl(hasDeclContext(namedDecl(hasName("M" matches the
 declaration of class D.
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann added a comment.

Dear reviewers,

Since this work was conducted as part of a Bachelor's thesis, which has to be
handed in on the 18th of June, 2021, we wanted to add a friendly ping to this
patch.

It would make us, the thesis team, proud to state that some of our work was
accepted and included into the LLVM project.

Therefore, we wanted to ask if we can further improve anything in this patch
for it to get a LGTM?

Thanks a lot in advance!
Yours faithfully,
Fabian & Marco


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

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

[PATCH] D100972: [clang-tidy] cppcoreguidelines-avoid-non-const-global-variables: add fixes to checks

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann added a comment.

Dear reviewers,

Since this work was conducted as part of a Bachelor's thesis, which has to be
handed in on the 18th of June, 2021, we wanted to add a friendly ping to this
patch.

It would make us, the thesis team, proud to state that some of our work was
accepted and included into the LLVM project.

Therefore, we wanted to ask if we can further improve anything in this patch
for it to get a LGTM?

Thanks a lot in advance!
Yours faithfully,
Fabian & Marco


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100972

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

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Marco Gartmann via Phabricator via cfe-commits]

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

Dear reviewers,

Since this work was conducted as part of a Bachelor's thesis, which has to be
handed in on the 18th of June, 2021, we wanted to add a friendly ping to this
patch.

It would make us, the thesis team, proud to state that some of our work was
accepted and included into the LLVM project.

Therefore, we wanted to ask if we can further improve anything in this patch
for it to get a LGTM?

Thanks a lot in advance!
Yours faithfully,
Fabian & Marco


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

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

[PATCH] D102779: [clang-tidy] cppcoreguidelines-explicit-constructor-and-conversion: new alias

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann added a comment.

Dear reviewers,

Since this work was conducted as part of a Bachelor's thesis, which has to be
handed in on the 18th of June, 2021, we wanted to add a friendly ping to this
patch.

It would make us, the thesis team, proud to state that some of our work was
accepted and included into the LLVM project.

Therefore, we wanted to ask if we can further improve anything in this patch
for it to get a LGTM?

Thanks a lot in advance!
Yours faithfully,
Fabian & Marco


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102779

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

[PATCH] D102779: [clang-tidy] cppcoreguidelines-explicit-constructor-and-conversion: new alias

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 347970.
mgartmann marked 5 inline comments as done.
mgartmann added a comment.

- added option to ignore conversion operators
- added tests for new and existing options
- renamed options to `Ignore...`
- ensured that option's strings only get parsed once


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102779

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp
  clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion.rst
  clang-tools-extra/docs/clang-tidy/checks/google-explicit-constructor.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconstructors-option.cpp
  
clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconversionoperators-option.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconversionoperators-option.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconversionoperators-option.cpp
@@ -0,0 +1,88 @@
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: google-explicit-constructor %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN: ]}'
+
+// RUN: %check_clang_tidy -check-suffix=IGNORED %s \
+// RUN: google-explicit-constructor %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN:   {key: google-explicit-constructor.IgnoredConversionOperators, value: "A::operator bool;B::operator double;B::operator A"} \
+// RUN: ]}'
+
+struct A {
+  A() {}
+  A(int x, int y) {}
+
+  explicit A(void *x) {}
+  explicit A(void *x, void *y) {}
+
+  A(int x1);
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}explicit A(int x1);
+
+  operator bool() const { return true; }
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES-DEFAULT: {{^  }}explicit operator bool() const { return true; }
+
+  operator double() const;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}explicit operator double() const;
+};
+
+inline A::A(int x1) {}
+
+struct B {
+  B() {}
+  B(int x, int y) {}
+
+  explicit B(void *x) {}
+  explicit B(void *x, void *y) {}
+
+  B(int x1);
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}explicit B(int x1);
+
+  operator bool() const { return true; }
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}explicit operator bool() const { return true; }
+
+  operator double() const;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES-DEFAULT: {{^  }}explicit operator double() const;
+
+  operator A() const;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator A' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES-DEFAULT: {{^  }}explicit operator A() const;
+};
+
+struct C {
+  C() {}
+  C(int x, int y) {}
+
+  explicit C(void *x) {}
+  explicit C(void *x, void *y) {}
+
+  C(int x1);
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [go

[PATCH] D102779: [clang-tidy] cppcoreguidelines-explicit-constructor-and-conversion: new alias

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann added a comment.

@aaron.ballman Thanks a lot for your valuable feedback! I incorporated it 
accordingly.

Is there anything else that should be improved?




Comment at: clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp:102
   if (const auto *Conversion =
-  Result.Nodes.getNodeAs("conversion")) {
+  Result.Nodes.getNodeAs("conversion")) {
 if (Conversion->isOutOfLine())

aaron.ballman wrote:
> Unintended formatting change?
This formatting change was done by `clang-format` when `git clang-format 
HEAD~1` was run. Therefore, I assumed that it is now correctly formatted. 

Am I wrong?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102779

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

[PATCH] D102779: [clang-tidy] cppcoreguidelines-explicit-constructor-and-conversion: new alias

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 348178.
mgartmann marked an inline comment as done.
mgartmann added a comment.

- added testcase of explicit operator


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102779

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp
  clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion.rst
  clang-tools-extra/docs/clang-tidy/checks/google-explicit-constructor.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconstructors-option.cpp
  
clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconversionoperators-option.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconversionoperators-option.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconversionoperators-option.cpp
@@ -0,0 +1,90 @@
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: google-explicit-constructor %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN: ]}'
+
+// RUN: %check_clang_tidy -check-suffix=IGNORED %s \
+// RUN: google-explicit-constructor %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN:   {key: google-explicit-constructor.IgnoredConversionOperators, value: "A::operator bool;B::operator double;B::operator A"} \
+// RUN: ]}'
+
+struct A {
+  A() {}
+  A(int x, int y) {}
+
+  explicit A(void *x) {}
+  explicit A(void *x, void *y) {}
+
+  A(int x1);
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}explicit A(int x1);
+
+  operator bool() const { return true; }
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES-DEFAULT: {{^  }}explicit operator bool() const { return true; }
+
+  operator double() const;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}explicit operator double() const;
+};
+
+inline A::A(int x1) {}
+
+struct B {
+  B() {}
+  B(int x, int y) {}
+
+  explicit B(void *x) {}
+  explicit B(void *x, void *y) {}
+
+  B(int x1);
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}explicit B(int x1);
+
+  operator bool() const { return true; }
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}explicit operator bool() const { return true; }
+
+  operator double() const;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES-DEFAULT: {{^  }}explicit operator double() const;
+
+  operator A() const;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator A' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES-DEFAULT: {{^  }}explicit operator A() const;
+};
+
+struct C {
+  C() {}
+  C(int x, int y) {}
+
+  explicit C(void *x) {}
+  explicit C(void *x, void *y) {}
+
+  C(int x1);
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: single-argument constructors must be marked explicit to

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 348988.
mgartmann added a comment.

- added fixes for private destructors
- separated fixes for private destructors into notes
- added a test case for a `= default;` constructor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-base-class-destructor.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp
@@ -0,0 +1,203 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-virtual-base-class-destructor %t -- --fix-notes
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PrivateVirtualBaseStruct' is private and prevents using the type [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+2]]:8: note: make it protected
+// As we have 2 conflicting fixes in notes, no fix is applied.
+struct PrivateVirtualBaseStruct {
+  virtual void f();
+private:
+  virtual ~PrivateVirtualBaseStruct() {}
+};
+
+struct PublicVirtualBaseStruct { // OK
+  virtual void f();
+  virtual ~PublicVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'ProtectedVirtualBaseStruct' is protected and virtual [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it protected and non-virtual
+struct ProtectedVirtualBaseStruct {
+  virtual void f();
+
+protected:
+  virtual ~ProtectedVirtualBaseStruct() {}
+  // CHECK-FIXES: ~ProtectedVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'ProtectedVirtualDefaultBaseStruct' is protected and virtual [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it protected and non-virtual
+struct ProtectedVirtualDefaultBaseStruct {
+  virtual void f();
+
+protected:
+  virtual ~ProtectedVirtualDefaultBaseStruct() = default;
+  // CHECK-FIXES: ~ProtectedVirtualDefaultBaseStruct() = default;
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PrivateNonVirtualBaseStruct' is private and prevents using the type [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+2]]:8: note: make it protected
+// As we have 2 conflicting fixes in notes, no fix is applied.
+struct PrivateNonVirtualBaseStruct {
+  virtual void f();
+
+private:
+  ~PrivateNonVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'PublicNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it public and virtual
+struct PublicNonVirtualBaseStruct {
+  virtual void f();
+  ~PublicNonVirtualBaseStruct() {}
+  // CHECK-FIXES: virtual ~PublicNonVirtualBaseStruct() {}
+};
+
+struct PublicNonVirtualNonBaseStruct { // OK according to C.35, since this struct does not have any virtual methods.
+  void f();
+  ~PublicNonVirtualNonBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PublicImplicitNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-FIXES: struct PublicImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicImplicitNonVirtualBaseStruct() = default;
+struct PublicImplicitNonVirtualBaseStruct {
+  virtual void f();
+};
+
+// CHECK-MESSAGES: :[[@LINE+5]]:8: warning: destructor of 'PublicASImplicitNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+4]]:8: note: make it public and virtual
+// CHECK-FIXES: struct PublicASImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicASImplicitNonVirtualBaseStruct() = default;
+// CHECK-FIXES-NEXT: private:
+struct PublicASImplicitNonVirtualBaseStruct {
+private:
+  virtual void f();
+};
+
+struct ProtectedNonVirtualBaseStruct { // OK
+  virtual void f();
+
+protected:
+  ~ProtectedNonVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:7

[PATCH] D110614: [clang-tidy] Fix false positives in cppcoreguidelines-virtual-class-destructor

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann added a comment.

Please excuse my late reply! I have been on vacation for the last two weeks and 
didn't have the time to respond to this thread until now.

In D110614#3032760 , @carlosgalvezp 
wrote:

> So the derived destructor only shows up in `ClassTemplateSpecializationDecl` 
> - the `CXXRecordDecl` doesn't have it (therefore implicit public non-virtual) 
> and that's why the check triggers.

I came to the same conclusion with a quick troubleshoot. Honestly, I didn't 
expect SEMA to behave this way and to generate CXXRecordDecl nodes without any 
CXXDestructorDecl sub-node. 
As a rather novice C++ programmer, I didn't have the described scenarios in 
mind when I initially created the check. Hence, the needed tests to find these 
false-positives beforehand were missing. 
I would like to apologise to everyone for the trouble this has caused.

@carlosgalvezp Thanks a lot for putting your time and effort into fix this bug!

In D110614#3038519 , @carlosgalvezp 
wrote:

> Anyway this check will need to be extended in the future, since the C++ Core 
> Guidelines has added a new bullet in their "Enforcement" section:
> https://github.com/isocpp/CppCoreGuidelines/commit/e44a9fcbd40923e9d5d342e444cf8a811e4a3eae

Thanks for pointing this out to me. Let me know if I can help you with this in 
any way. I would be available to implement the new enforcement from 2021-10-11 
on.

Again, my dearest apologies for this bug.


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

https://reviews.llvm.org/D110614

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

[PATCH] D102779: [clang-tidy] cppcoreguidelines-explicit-constructor-and-conversion: new alias

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 351838.
mgartmann marked 5 inline comments as done.
mgartmann added a comment.

- removed empty configs from tests
- moved documentation to Google's check
- extended matchers so that namespaces for classes and conversion operators can 
be specified
- adjusted documentation and tests

Unfortunately, I was not able to implement functionality to ignore constructors 
and operates by using Regex in the given time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102779

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp
  clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion.rst
  clang-tools-extra/docs/clang-tidy/checks/google-explicit-constructor.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconstructors-option.cpp
  
clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconversionoperators-option.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconversionoperators-option.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconversionoperators-option.cpp
@@ -0,0 +1,95 @@
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s google-explicit-constructor %t
+
+// RUN: %check_clang_tidy -check-suffix=IGNORED %s \
+// RUN: google-explicit-constructor %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN:   {key: google-explicit-constructor.IgnoredConversionOperators, value: "Foo::A::operator bool;Foo::A::operator type-parameter-0-0;B::operator double;B::operator A"} \
+// RUN: ]}'
+
+namespace Foo {
+struct A {
+  A() {}
+  A(int x, int y) {}
+
+  explicit A(void *x) {}
+  explicit A(void *x, void *y) {}
+
+  A(int x1);
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}explicit A(int x1);
+
+  operator bool() const { return true; }
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES-DEFAULT: {{^  }}explicit operator bool() const { return true; }
+
+  operator double() const;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}explicit operator double() const;
+
+  template 
+  operator Ty() const;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator type-parameter-0-0' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES-DEFAULT: {{^  }}explicit operator Ty() const;
+};
+
+inline A::A(int x1) {}
+} // namespace Foo
+
+using Foo::A;
+struct B {
+  B() {}
+  B(int x, int y) {}
+
+  explicit B(void *x) {}
+  explicit B(void *x, void *y) {}
+
+  B(int x1);
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}explicit B(int x1);
+
+  operator bool() const { return true; }
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}explicit operator bool() const { return true; }
+
+  operator double() const;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES-DEFAULT: {{^  }}explicit operator double() const;
+
+  operator A() const;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]

[PATCH] D102779: [clang-tidy] cppcoreguidelines-explicit-constructor-and-conversion: new alias

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp:28-30
+  std::string FullOperatorName =
+  Node.getParent()->getNameAsString().append("::").append(
+  Node.getNameAsString());

aaron.ballman wrote:
> Rather than trying to do this by hand, does `Decl::print()` give you the 
> functionality you need? For example, this will likely not work well for 
> classes defined within a namespace (it may ignore the wrong class due to not 
> checking the namespace). Another thing to consider are templates and how to 
> handle those. e.g.,
> ```
> struct Foo {
>   template 
>   operator Ty() const; // How to silence the diagnostic here?
> };
> ```
> Thankfully, specializations can't differ in their explicitness, so you don't 
> have to also worry about:
> ```
> struct Foo {
>   template 
>   explicit operator Ty() const; // This one's explicit
> };
> 
> template <>
> Foo::operator int() const; // Thankfully, this inherits the explicit from the 
> primary template.
> ```
Thanks for your comment, @aaron.ballman!

I was able to use `printQualifiedName` to get a `Node`'s qualified name, 
including its namespace. 

Templated operators are not represented by their name visible in the source 
code (e.g., `Ty`) in the AST. Instead, their name in the AST is something like 
`type-parameter-0-0`. As it is now, templated operators have to be ignored with 
the latter name, which is also displayed in the check's diagnostic message.
This was described in the documentation accordingly.

I was not able to find a feasible way to enable users to exclude templated 
operators by their original name (e.g., `Ty`). Does anyone have an idea how 
this could be done?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102779

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

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 351869.
mgartmann marked 10 inline comments as done.
mgartmann added a comment.

- changed name of check to `cppcoreguidelines-virtual-class-destructor`
- removed matcher for class or struct in AST matcher
- changed string concatenations to use `llvm::twine`
- adjusted documentation and removed unnecessary semicolons in it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-class-destructor.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
@@ -0,0 +1,204 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-virtual-class-destructor %t -- --fix-notes
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PrivateVirtualBaseStruct' is private and prevents using the type [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+2]]:8: note: make it protected
+// As we have 2 conflicting fixes in notes, no fix is applied.
+struct PrivateVirtualBaseStruct {
+  virtual void f();
+
+private:
+  virtual ~PrivateVirtualBaseStruct() {}
+};
+
+struct PublicVirtualBaseStruct { // OK
+  virtual void f();
+  virtual ~PublicVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'ProtectedVirtualBaseStruct' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it protected and non-virtual
+struct ProtectedVirtualBaseStruct {
+  virtual void f();
+
+protected:
+  virtual ~ProtectedVirtualBaseStruct() {}
+  // CHECK-FIXES: ~ProtectedVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'ProtectedVirtualDefaultBaseStruct' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it protected and non-virtual
+struct ProtectedVirtualDefaultBaseStruct {
+  virtual void f();
+
+protected:
+  virtual ~ProtectedVirtualDefaultBaseStruct() = default;
+  // CHECK-FIXES: ~ProtectedVirtualDefaultBaseStruct() = default;
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PrivateNonVirtualBaseStruct' is private and prevents using the type [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+2]]:8: note: make it protected
+// As we have 2 conflicting fixes in notes, no fix is applied.
+struct PrivateNonVirtualBaseStruct {
+  virtual void f();
+
+private:
+  ~PrivateNonVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'PublicNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it public and virtual
+struct PublicNonVirtualBaseStruct {
+  virtual void f();
+  ~PublicNonVirtualBaseStruct() {}
+  // CHECK-FIXES: virtual ~PublicNonVirtualBaseStruct() {}
+};
+
+struct PublicNonVirtualNonBaseStruct { // OK according to C.35, since this struct does not have any virtual methods.
+  void f();
+  ~PublicNonVirtualNonBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PublicImplicitNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-FIXES: struct PublicImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicImplicitNonVirtualBaseStruct() = default;
+struct PublicImplicitNonVirtualBaseStruct {
+  virtual void f();
+};
+
+// CHECK-MESSAGES: :[[@LINE+5]]:8: warning: destructor of 'PublicASImplicitNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+4]]:8: note: make it public and virtual
+// CHECK-FIXES: struct PublicASImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicASImplicitNonVirtualBaseStruct() = default;
+// CHECK-FIXES-NEXT: private:
+struct PublicASImplicitNonVirtualBaseStruct {
+private:
+  virtual void f();
+};
+
+struct ProtectedNonVirtualBaseStruct { // OK
+  virtual void f();
+
+protected:

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann added a comment.

Thanks a lot for your feedback, @aaron.ballman!

I was able to incorporate it in the updated diff.

On your advice, I also ran this check on a large open source project using the 
`run-clang-tidy.py` script. I decided to do it on the llvm project itself as 
follows:

  marco@nb:~/llvm-project/build$ python3 run-clang-tidy.py 
-checks="-*,cppcoreguidelines-virtual-class-destructor"  -header-filter=".*" > 
result.txt
  marco@nb:~/llvm-project/build$ cat result.txt | grep "private and prevents" | 
wc -l
  797

Most of these 797 lines are duplicates, since these warnings come from header 
files included in (apparently) many other files. 
Deduplicated, the following seven destructors were private and triggered this 
check's warning:

- llvm-project/llvm/include/llvm/Analysis/RegionInfo.h:1024:23: warning: 
destructor of 'RegionInfoBase>' is private 
...
- llvm-project/llvm/include/llvm/Analysis/RegionInfo.h:674:7: warning: 
destructor of 'RegionInfoBase' is private ...
- llvm-project/llvm/include/llvm/CodeGen/MachineRegionInfo.h:177:23: warning: 
destructor of 'RegionInfoBase>' is 
private ...
- llvm-project/llvm/lib/Target/AVR/MCTargetDesc/AVRMCExpr.h:19:7: warning: 
destructor of 'AVRMCExpr' is private ...
- llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:1257:18: 
warning: destructor of 'UnitTest' is private ...
- llvm-project/llvm/lib/CodeGen/InlineSpiller.cpp:159:7: warning: destructor of 
'InlineSpiller' is private ...
- llvm-project/llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp:387:7: 
warning: destructor of 'X86MCInstrAnalysis' is private ...

I assume that these destructors are private by on purpose. Please correct me if 
I am wrong. 
A programmer working on the LLVM project would therefore see seven warnings for 
private destructors over the whole project.

Is this number of private destructors which are flagged acceptable to you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

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

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 365161.
mgartmann added a comment.

- Updated this patch with the current state of the LLVM project GitHub 
repository


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-class-destructor.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
@@ -0,0 +1,204 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-virtual-class-destructor %t -- --fix-notes
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PrivateVirtualBaseStruct' is private and prevents using the type [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+2]]:8: note: make it protected
+// As we have 2 conflicting fixes in notes, no fix is applied.
+struct PrivateVirtualBaseStruct {
+  virtual void f();
+
+private:
+  virtual ~PrivateVirtualBaseStruct() {}
+};
+
+struct PublicVirtualBaseStruct { // OK
+  virtual void f();
+  virtual ~PublicVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'ProtectedVirtualBaseStruct' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it protected and non-virtual
+struct ProtectedVirtualBaseStruct {
+  virtual void f();
+
+protected:
+  virtual ~ProtectedVirtualBaseStruct() {}
+  // CHECK-FIXES: ~ProtectedVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'ProtectedVirtualDefaultBaseStruct' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it protected and non-virtual
+struct ProtectedVirtualDefaultBaseStruct {
+  virtual void f();
+
+protected:
+  virtual ~ProtectedVirtualDefaultBaseStruct() = default;
+  // CHECK-FIXES: ~ProtectedVirtualDefaultBaseStruct() = default;
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PrivateNonVirtualBaseStruct' is private and prevents using the type [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+2]]:8: note: make it protected
+// As we have 2 conflicting fixes in notes, no fix is applied.
+struct PrivateNonVirtualBaseStruct {
+  virtual void f();
+
+private:
+  ~PrivateNonVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'PublicNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it public and virtual
+struct PublicNonVirtualBaseStruct {
+  virtual void f();
+  ~PublicNonVirtualBaseStruct() {}
+  // CHECK-FIXES: virtual ~PublicNonVirtualBaseStruct() {}
+};
+
+struct PublicNonVirtualNonBaseStruct { // OK according to C.35, since this struct does not have any virtual methods.
+  void f();
+  ~PublicNonVirtualNonBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PublicImplicitNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-FIXES: struct PublicImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicImplicitNonVirtualBaseStruct() = default;
+struct PublicImplicitNonVirtualBaseStruct {
+  virtual void f();
+};
+
+// CHECK-MESSAGES: :[[@LINE+5]]:8: warning: destructor of 'PublicASImplicitNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+4]]:8: note: make it public and virtual
+// CHECK-FIXES: struct PublicASImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicASImplicitNonVirtualBaseStruct() = default;
+// CHECK-FIXES-NEXT: private:
+struct PublicASImplicitNonVirtualBaseStruct {
+private:
+  virtual void f();
+};
+
+struct ProtectedNonVirtualBaseStruct { // OK
+  virtual void f();
+
+protected:
+  ~ProtectedNonVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:7: warning: destructor of 'PrivateVirtualBaseClass' is private and prevents using the type [cppcoreguidelines-virtual-class-dest

[PATCH] D100972: [clang-tidy] cppcoreguidelines-avoid-non-const-global-variables: add fixes to checks

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 339565.
mgartmann added a comment.

- Renamed `printCleanedType()` to `cleanType()`
- Extended `cleanType()` to also remove `(anonymous)` from a type
- Made `hasSpaceAfterType()` more error-robust.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100972

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp
@@ -2,20 +2,25 @@
 
 int nonConstInt = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'nonConstInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int nonConstInt = 0;
 
 int &nonConstIntReference = nonConstInt;
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: variable 'nonConstIntReference' provides global access to a non-const object; consider making the referenced data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int &nonConstIntReference = nonConstInt;
 
 int *pointerToNonConstInt = &nonConstInt;
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: variable 'pointerToNonConstInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
 // CHECK-MESSAGES: :[[@LINE-2]]:6: warning: variable 'pointerToNonConstInt' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const pointerToNonConstInt = &nonConstInt;
 
 int *const constPointerToNonConstInt = &nonConstInt;
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'constPointerToNonConstInt' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const constPointerToNonConstInt = &nonConstInt;
 
 namespace namespace_name {
 int nonConstNamespaceInt = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'nonConstNamespaceInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int nonConstNamespaceInt = 0;
 
 const int constNamespaceInt = 0;
 } // namespace namespace_name
@@ -24,6 +29,7 @@
 
 const int *pointerToConstInt = &constInt;
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'pointerToConstInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const pointerToConstInt = &constInt;
 
 const int *const constPointerToConstInt = &constInt;
 
@@ -39,6 +45,7 @@
 namespace {
 int nonConstAnonymousNamespaceInt = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'nonConstAnonymousNamespaceInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int nonConstAnonymousNamespaceInt = 0;
 } // namespace
 
 class DummyClass {
@@ -53,27 +60,35 @@
 
 DummyClass nonConstClassInstance;
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'nonConstClassInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyClass nonConstClassInstance;
 
 DummyClass *pointerToNonConstDummyClass = &nonConstClassInstance;
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable 'pointerToNonConstDummyClass' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
 // CHECK-MESSAGES: :[[@LINE-2]]:13: warning: variable 'pointerToNonConstDummyClass' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyClass *const pointerToNonConstDummyClass = &nonConstClassInstance;
 
 DummyClass &referenceToNonConstDummyClass = nonConstClassInstance;
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable 'referenceToNonConstDummyClass' provides global access to a non-const object; consider making the referenced data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHE

[PATCH] D100972: [clang-tidy] cppcoreguidelines-avoid-non-const-global-variables: add fixes to checks

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 339574.
mgartmann added a comment.

Fixed one-off error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100972

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp
@@ -2,20 +2,25 @@
 
 int nonConstInt = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'nonConstInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int nonConstInt = 0;
 
 int &nonConstIntReference = nonConstInt;
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: variable 'nonConstIntReference' provides global access to a non-const object; consider making the referenced data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int &nonConstIntReference = nonConstInt;
 
 int *pointerToNonConstInt = &nonConstInt;
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: variable 'pointerToNonConstInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
 // CHECK-MESSAGES: :[[@LINE-2]]:6: warning: variable 'pointerToNonConstInt' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const pointerToNonConstInt = &nonConstInt;
 
 int *const constPointerToNonConstInt = &nonConstInt;
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'constPointerToNonConstInt' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const constPointerToNonConstInt = &nonConstInt;
 
 namespace namespace_name {
 int nonConstNamespaceInt = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'nonConstNamespaceInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int nonConstNamespaceInt = 0;
 
 const int constNamespaceInt = 0;
 } // namespace namespace_name
@@ -24,6 +29,7 @@
 
 const int *pointerToConstInt = &constInt;
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'pointerToConstInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const pointerToConstInt = &constInt;
 
 const int *const constPointerToConstInt = &constInt;
 
@@ -39,6 +45,7 @@
 namespace {
 int nonConstAnonymousNamespaceInt = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'nonConstAnonymousNamespaceInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int nonConstAnonymousNamespaceInt = 0;
 } // namespace
 
 class DummyClass {
@@ -53,27 +60,35 @@
 
 DummyClass nonConstClassInstance;
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'nonConstClassInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyClass nonConstClassInstance;
 
 DummyClass *pointerToNonConstDummyClass = &nonConstClassInstance;
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable 'pointerToNonConstDummyClass' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
 // CHECK-MESSAGES: :[[@LINE-2]]:13: warning: variable 'pointerToNonConstDummyClass' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyClass *const pointerToNonConstDummyClass = &nonConstClassInstance;
 
 DummyClass &referenceToNonConstDummyClass = nonConstClassInstance;
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable 'referenceToNonConstDummyClass' provides global access to a non-const object; consider making the referenced data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyClass &referenceToNonConstDummyClass = nonConstClassInstance;
 
 int *nonConstPointerToMember = &nonConstClassInstance.n

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann added a comment.

Friendly ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

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

[PATCH] D100972: [clang-tidy] cppcoreguidelines-avoid-non-const-global-variables: add fixes to checks

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 341155.
mgartmann added a comment.

Replaced string comparison to check if a character is a space with 
`std::isspace()`. 
Added test case for this scenario.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100972

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp
@@ -2,20 +2,33 @@
 
 int nonConstInt = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'nonConstInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int nonConstInt = 0;
 
 int &nonConstIntReference = nonConstInt;
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: variable 'nonConstIntReference' provides global access to a non-const object; consider making the referenced data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int &nonConstIntReference = nonConstInt;
 
 int *pointerToNonConstInt = &nonConstInt;
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: variable 'pointerToNonConstInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
 // CHECK-MESSAGES: :[[@LINE-2]]:6: warning: variable 'pointerToNonConstInt' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const pointerToNonConstInt = &nonConstInt;
+
+// clang-format off
+// because the space after * is intended.
+int * pointerToNonConstIntWithSpace = &nonConstInt;
+// clang-format on
+// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: variable 'pointerToNonConstIntWithSpace' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-MESSAGES: :[[@LINE-3]]:7: warning: variable 'pointerToNonConstIntWithSpace' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const pointerToNonConstIntWithSpace = &nonConstInt;
 
 int *const constPointerToNonConstInt = &nonConstInt;
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'constPointerToNonConstInt' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const constPointerToNonConstInt = &nonConstInt;
 
 namespace namespace_name {
 int nonConstNamespaceInt = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'nonConstNamespaceInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int nonConstNamespaceInt = 0;
 
 const int constNamespaceInt = 0;
 } // namespace namespace_name
@@ -24,6 +37,7 @@
 
 const int *pointerToConstInt = &constInt;
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'pointerToConstInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const pointerToConstInt = &constInt;
 
 const int *const constPointerToConstInt = &constInt;
 
@@ -39,6 +53,7 @@
 namespace {
 int nonConstAnonymousNamespaceInt = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'nonConstAnonymousNamespaceInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int nonConstAnonymousNamespaceInt = 0;
 } // namespace
 
 class DummyClass {
@@ -53,27 +68,35 @@
 
 DummyClass nonConstClassInstance;
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'nonConstClassInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyClass nonConstClassInstance;
 
 DummyClass *pointerToNonConstDummyClass = &nonConstClassInstance;
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable 'pointerToNonConstDummyClass' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
 // CHECK-MESSAGES: 

[PATCH] D100972: [clang-tidy] cppcoreguidelines-avoid-non-const-global-variables: add fixes to checks

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann added a comment.

Friendly ping, any feedback would be appreciated :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100972

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

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 343911.
mgartmann added a comment.

Extracted STD IO stream and C-like IO function names into vectors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-avoid-std-io-outside-main.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s misc-avoid-std-io-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+} // namespace std
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+
+namespace arbitrary_namespace {
+std::Ostream cout{};
+std::Istream cin{};
+} // namespace arbitrary_namespace
+
+void anyNonMainFunction() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcin >> Foo;
+
+  arbitrary_namespace::cin >> Foo; // OK
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::printf("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  printf("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::puts("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  puts("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::putchar('m');
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  putchar('m');
+
+  char Input[5];
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::scanf("%s", Input);
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  scanf("%s", Input);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::getchar();
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio func

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

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

@njames93 thanks a lot for your answer! I extracted the STD IO stream and 
C-like function names according to your comment.

In your opinion, is there something else in this patch that has to be improved 
to make this request mergeable and to get a LGTM?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

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

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 344013.
mgartmann added a comment.

Remove trailing whitespaces from documentation file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-avoid-std-io-outside-main.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s misc-avoid-std-io-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+} // namespace std
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+
+namespace arbitrary_namespace {
+std::Ostream cout{};
+std::Istream cin{};
+} // namespace arbitrary_namespace
+
+void anyNonMainFunction() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcin >> Foo;
+
+  arbitrary_namespace::cin >> Foo; // OK
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::printf("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  printf("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::puts("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  puts("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::putchar('m');
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  putchar('m');
+
+  char Input[5];
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::scanf("%s", Input);
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  scanf("%s", Input);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::getchar();
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should n

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 344023.
mgartmann added a comment.

Updated diff to fix the build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-avoid-std-io-outside-main.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s misc-avoid-std-io-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+} // namespace std
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+
+namespace arbitrary_namespace {
+std::Ostream cout{};
+std::Istream cin{};
+} // namespace arbitrary_namespace
+
+void anyNonMainFunction() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcin >> Foo;
+
+  arbitrary_namespace::cin >> Foo; // OK
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::printf("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  printf("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::puts("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  puts("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::putchar('m');
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  putchar('m');
+
+  char Input[5];
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::scanf("%s", Input);
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  scanf("%s", Input);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::getchar();
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 344030.
mgartmann added a comment.

Change encoding of patch to UTF-8 in order to fix build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-avoid-std-io-outside-main.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s misc-avoid-std-io-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+} // namespace std
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+
+namespace arbitrary_namespace {
+std::Ostream cout{};
+std::Istream cin{};
+} // namespace arbitrary_namespace
+
+void anyNonMainFunction() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcin >> Foo;
+
+  arbitrary_namespace::cin >> Foo; // OK
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::printf("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  printf("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::puts("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  puts("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::putchar('m');
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  putchar('m');
+
+  char Input[5];
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::scanf("%s", Input);
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  scanf("%s", Input);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::getchar();
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions shou

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 344043.
mgartmann added a comment.

Revert `ReleaseNotes.rst` to a point where the build worked.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-avoid-std-io-outside-main.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s misc-avoid-std-io-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+} // namespace std
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+
+namespace arbitrary_namespace {
+std::Ostream cout{};
+std::Istream cin{};
+} // namespace arbitrary_namespace
+
+void anyNonMainFunction() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcin >> Foo;
+
+  arbitrary_namespace::cin >> Foo; // OK
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::printf("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  printf("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::puts("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  puts("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::putchar('m');
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  putchar('m');
+
+  char Input[5];
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::scanf("%s", Input);
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  scanf("%s", Input);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::getchar();
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions 

[PATCH] D99646: [clang-tidy] misc-std-stream-objects-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann created this revision.
mgartmann added reviewers: aaron.ballman, njames93, alexfh.
mgartmann added a project: clang-tools-extra.
Herald added subscribers: xazax.hun, mgorny.
mgartmann requested review of this revision.

**Problem Description**

The `iostream` header file defines multiple global objects like `std:cin`, 
`std::cout`, etc.

The pitfall of using these global objects outside of the `main` function is 
that it negatively affects the code's testability. They cannot be replaced with 
a mocked input stream during testing. Therefore,  `std::cin`, `std::cout` etc. 
should only be used inside the main function. If other functions need an input 
or an output stream, one is encouraged to use the `std::istream` and 
`std::ostream` interfaces and to receive the stream object files as function 
parameters. 
Thus, during testing, mocked stream objects can be handed to the function.

**What this Check Does**

The goal of this check is to find any uses of predefined standard stream 
objects (i.e., `cout, wcout, cerr, wcerr, cin, wcin`) and to check whether they 
are used inside the `main` function or not. If any uses are found outside the 
`main` function, they get flagged.

The use of `clog` and `wclog` does not get flagged by this check. The rationale 
for this is that code with logging functionality rarely needs to be tested.

**Context**

The idea for this check is adopted from the checks implemented in Cevelop IDE 
.

This contribution is part of a project which aims to port Cevelop's built-in 
checks to other IDEs.

We are happy to receive any suggestions for improvement.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst
  
clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp
@@ -0,0 +1,36 @@
+// RUN: %check_clang_tidy -std=c++14-or-later %s misc-std-stream-objects-outside-main %t
+
+#include 
+
+void problematic() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  int I{0};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::cin >> I;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::wcin >> I;
+}
+
+int main() {
+  std::cout << "This should not trigger the check";  // OK
+  std::wcout << "This should not trigger the check"; // OK
+  std::cerr << "This should not trigger the check";  // OK
+  std::wcerr << "This should not trigger the check"; // OK
+
+  int I{0};
+  std::cin >> I;  // OK
+  std::wcin >> I; // OK
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst
@@ -0,0 +1,45 @@
+.. title:: clang-tidy - misc-std-stream-objects-outside-main
+
+misc-std-stream-objects-outside-main
+=
+
+The check diagnoses when a predefined standard stream object (i.e., ``cin``, ``wcin``,
+``cout``, ``wcout``, ``cerr`` and ``wcerr``) is used outside the ``main`` function.
+
+For instance, in the following code, the use of ``std::cout`` outside of ``main()`` would get
+flagged whereas the use of ``std::cout`` inside ``main()`` is not flagged:
+
+.. code-block:: c++
+
+  #include

[PATCH] D99646: [clang-tidy] misc-std-stream-objects-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann added a comment.

I am working on fixing the failing build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

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

[PATCH] D99646: [clang-tidy] misc-std-stream-objects-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann marked 10 inline comments as done.
mgartmann added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.cpp:25
+  .bind("match"),
+  this);
+}

riccibruno wrote:
> Will this match `my_namespace::cin`?
Yes, at the moment this would be matched as well.



Comment at: 
clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.cpp:42
+
+bool StdStreamObjectsOutsideMainCheck::isInsideMainFunction(
+const MatchFinder::MatchResult &Result, const DynTypedNode &Node) {

njames93 wrote:
> This all seems unnecessary, you can add `unless(forFunction(isMain()))` to 
> your declRefExpr matcher instead.
Thanks for pointing this out to me. `forFunction()` is exactly what I was 
initially looking for. Unfortunately, I did not find it. 

I now refactored the code to use this matcher instead.

I guess `isMain()` would not match if a MSVCRT entry point (see @riccibruno's 
answer on line 47) is used, is it? Do you think it makes sense to also match 
this kind of entry point? How could this be done? I was not able to find 
anything related in the [[ 
https://clang.llvm.org/docs/LibASTMatchersReference.html | AST Matcher 
Reference ]].



Comment at: 
clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.cpp:47
+  if (AsFunctionDecl && AsFunctionDecl->getIdentifier() &&
+  AsFunctionDecl->getName().equals("main")) {
+return true;

riccibruno wrote:
> You can use `FunctionDecl::isMain`. Additionally you might want to also use 
> `FunctionDecl::isMSVCRTEntryPoint` for the Windows-specific main functions.
Due to @njames93's suggestion to use a `unless(forFunction(isMain()))` matcher, 
this function is not needed anymore.

Thank you anyways for pointing this out to me. 



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp:1
+// RUN: %check_clang_tidy -std=c++14-or-later %s 
misc-std-stream-objects-outside-main %t
+

njames93 wrote:
> Why won't this work in c++11 mode?
When not specifying C++14 as the standard, the corresponding tests failed.
Some errors in the test output pointed out that e.g.,// auto return without 
trailing return types// is a C++14 extension.

I created a [[ 
https://gist.github.com/mgartmann/f8a876ebbed6f7c1b8e7a9c901f36508 | gist 
(click me) ]] which contains the test ouput of this clang-tidy check. There, 
all occured errors can be seen.

IMHO, those errors seem to come from the directly or indirectly included header 
files rather than from this check. Please correct me if I am wrong.

After adding `-std=c++14-or-later`, no errors occured anymore. That is the 
reason why I added it.

Anyways, I now refactored the test file, creating a `std` namespace and adding 
my own "mocked" stream objects. I adpoted this approach from existing tests 
(e.g., `readability-string-compare`).

Thus, no include is needed anymore and `-std=c++14-or-later` can be omitted.

If I overlooked something, I would be glad if you could point this out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

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

[PATCH] D99646: [clang-tidy] misc-std-stream-objects-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 334527.
mgartmann marked 4 inline comments as done.
mgartmann added a comment.

Refactored the code and documentation files according to the feedback received 
on the first diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst
  
clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp
@@ -0,0 +1,53 @@
+// RUN: %check_clang_tidy %s misc-std-stream-objects-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+} // namespace std
+
+void problematic() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::wcin >> Foo;
+}
+
+int main() {
+  std::cout << "This should not trigger the check";  // OK
+  std::wcout << "This should not trigger the check"; // OK
+  std::cerr << "This should not trigger the check";  // OK
+  std::wcerr << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+  std::cin >> Foo;  // OK
+  std::wcin >> Foo; // OK
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst
@@ -0,0 +1,45 @@
+.. title:: clang-tidy - misc-std-stream-objects-outside-main
+
+misc-std-stream-objects-outside-main
+=
+
+Diagnoses if a predefined standard stream object (``cin``, ``wcin``,
+``cout``, ``wcout``, ``cerr`` or ``wcerr``) is used outside the ``main`` function.
+
+For instance, in the following code, the use of ``std::cout`` outside of ``main()`` would get
+flagged whereas the use of ``std::cout`` inside ``main()`` is not flagged:
+
+.. code-block:: c++
+
+  #include 
+
+  void some_function() {
+std::cout << "This triggers the check.";
+ 
+  }
+
+  int main() {
+std::cout << "This does not trigger the check.";
+  }
+
+Since the predefined standard stream objects are global objects, their use outside of ``main()`` worsens a
+program's testability and is thus discouraged. Instead, those objects should only be used inside ``main()``.
+They can then be passed as arguments to other functions like so:
+
+.. code-block:: c++
+
+  #include 
+
+  void some_function(std::istream & in, std::ostream & out) {
+out << "This does not trigger the check.";
+
+int i{0};
+in >> i;
+  }
+
+  int main() {
+some_function(std::cin, std::cout);
+  }
+
+In contrast to using ``std::cin`` and ``std::cout`` directly, in the above example, it is possible to inject 
+mocked stream objects into ``some_function()`` during testing.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ c

[PATCH] D99646: [clang-tidy] misc-std-stream-objects-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 334600.
mgartmann added a comment.

Removed superfluous semicolon in `StdStreamObjectsOutsideMainCheck.cpp` 
according to feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst
  
clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp
@@ -0,0 +1,53 @@
+// RUN: %check_clang_tidy %s misc-std-stream-objects-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+} // namespace std
+
+void problematic() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::wcin >> Foo;
+}
+
+int main() {
+  std::cout << "This should not trigger the check";  // OK
+  std::wcout << "This should not trigger the check"; // OK
+  std::cerr << "This should not trigger the check";  // OK
+  std::wcerr << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+  std::cin >> Foo;  // OK
+  std::wcin >> Foo; // OK
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst
@@ -0,0 +1,45 @@
+.. title:: clang-tidy - misc-std-stream-objects-outside-main
+
+misc-std-stream-objects-outside-main
+=
+
+Diagnoses if a predefined standard stream object (``cin``, ``wcin``,
+``cout``, ``wcout``, ``cerr`` or ``wcerr``) is used outside the ``main`` function.
+
+For instance, in the following code, the use of ``std::cout`` outside of ``main()`` would get
+flagged whereas the use of ``std::cout`` inside ``main()`` is not flagged:
+
+.. code-block:: c++
+
+  #include 
+
+  void some_function() {
+std::cout << "This triggers the check.";
+ 
+  }
+
+  int main() {
+std::cout << "This does not trigger the check.";
+  }
+
+Since the predefined standard stream objects are global objects, their use outside of ``main()`` worsens a
+program's testability and is thus discouraged. Instead, those objects should only be used inside ``main()``.
+They can then be passed as arguments to other functions like so:
+
+.. code-block:: c++
+
+  #include 
+
+  void some_function(std::istream & in, std::ostream & out) {
+out << "This does not trigger the check.";
+
+int i{0};
+in >> i;
+  }
+
+  int main() {
+some_function(std::cin, std::cout);
+  }
+
+In contrast to using ``std::cin`` and ``std::cout`` directly, in the above example, it is possible to inject 
+mocked stream objects into ``some_function()`` during testing.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rs

[PATCH] D99646: [clang-tidy] misc-std-stream-objects-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 334632.
mgartmann marked an inline comment as done.
mgartmann added a comment.

Add isInStdNamespace to matcher so that only global objects in namespace `std` 
are matched and add corresponding tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst
  
clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp
@@ -0,0 +1,64 @@
+// RUN: %check_clang_tidy %s misc-std-stream-objects-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+} // namespace std
+
+namespace arbitrary_namespace {
+std::Ostream cout{};
+std::Istream cin{};
+} // namespace arbitrary_namespace
+
+void problematic() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::wcin >> Foo;
+
+  arbitrary_namespace::cin >> Foo; // OK
+}
+
+int main() {
+  std::cout << "This should not trigger the check"; // OK
+  std::wcout << "This should not trigger the check";// OK
+  std::cerr << "This should not trigger the check"; // OK
+  std::wcerr << "This should not trigger the check";// OK
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+  std::cin >> Foo; // OK
+  std::wcin >> Foo;// OK
+  arbitrary_namespace::cin >> Foo; // OK
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst
@@ -0,0 +1,45 @@
+.. title:: clang-tidy - misc-std-stream-objects-outside-main
+
+misc-std-stream-objects-outside-main
+=
+
+Diagnoses if a predefined standard stream object (``cin``, ``wcin``,
+``cout``, ``wcout``, ``cerr`` or ``wcerr``) is used outside the ``main`` function.
+
+For instance, in the following code, the use of ``std::cout`` outside of ``main()`` would get
+flagged whereas the use of ``std::cout`` inside ``main()`` is not flagged:
+
+.. code-block:: c++
+
+  #include 
+
+  void some_function() {
+std::cout << "This triggers the check.";
+ 
+  }
+
+  int main() {
+std::cout << "This does not trigger the check.";
+  }
+
+Since the predefined standard stream objects are global objects, their use outside of ``main()`` worsens a
+program's testability and is thus discouraged. Instead, those objects should only be used inside ``main()``.
+They can then be passed as arguments to other functions like so:
+
+.. code-block:: c++
+
+  #include 
+
+  void some_function(std::istream & in, std::ostream & out) {
+out << "This does not trigger the check.

[PATCH] D99646: [clang-tidy] misc-std-stream-objects-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.cpp:25
+  .bind("match"),
+  this);
+}

mgartmann wrote:
> riccibruno wrote:
> > Will this match `my_namespace::cin`?
> Yes, at the moment this would be matched as well.
Thinking about it, in my opinion, only matching those objects if they are 
coming form the `std` namespace would make more sense and lead to less 
"false-positive" diagnostics. 
Furthermore, [[ https://www.cevelop.com/ | Cevelop IDE ]]'s check, which is 
where the idea for this check came form, also behaves like this.

Thus, I added the `isInStdNamespace()` matcher in the latest diff.

@riccibruno What is your opinion on this? Would it make more sense to also 
match e.g., `my_namespace::cin` in your opinion? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

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

[PATCH] D99646: [clang-tidy] misc-std-stream-objects-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann added a comment.

In D99646#2661651 , @njames93 wrote:

> Is it not wise to also check the c standard library.
> So check for function refs to these names in the global or std namespace.
> `printf`, `vprintf`, `puts`, `putchar`, `scanf`, `scanf`, `getchar` and `gets`
> It may be a bit of a pain checking for usages of `stdin` and  `stdout` due to 
> them being defined as macros.

Hi @njames93,
I can see your point, I am going to add this functionality.

However, I do not completely understand what you mean with //check for function 
refs to these names in the global or std namespace//.  
Could you explain this a bit further?

E.g., should all calls to these functions be flagged if they happen inside the 
`std` namespace or in the `global` namespace?
And if they happen inside `my_namespace` e.g., they should not be flagged. Am I 
understanding this correctly?
How should the check behave if the functions are called inside `main()`?

Would it make more sense to put this functionality into a separat check?

Thanks for your effort in advance.
Looking forward to hearing from you soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

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

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 335750.
mgartmann retitled this revision from "[clang-tidy] 
misc-std-stream-objects-outside-main: a new check" to "[clang-tidy] 
misc-avoid-std-io-outside-main: a new check".
mgartmann added a comment.

- Added two new matchers to flag uses of `stdio.h`/`csdtio` functions outside 
of main.
- Renamed the check to fit those new functionalities.

**Rationale:**

When analyzing the AST dump of a program using `stdio.h`'s `printf()` e.g., I 
found that those functions are not declared directly under `translationUnit`, 
but under `translationUnitDecl/linkageSpecDecl`. A 
`hasDeclContext(linkageSpecDecl())` matcher worked with both `stdio.h` and 
`cstdio` functions. 
In the check's test, I tried to include a own header file from the //Inputs// 
directory. However, the functions declared in this header file were then direct 
AST children of `translationUnitDecl` and not of `linkageSpecDecl`. 
Due to the fact that I was not able to reproduce the original AST in the tests, 
I decided to omit this part of the matcher.

Furthermore, when using `cstdio`'s `std::printf()` e.g., these functions are in 
fact //UsingShadowDecl//s. Despite them being declared in the `std` namespace, 
a call of `std::printf("...")` does not match with a `isInStdNamespace()` 
matcher.
Thus, this part of the matcher was omitted as well.

The resulting matcher matches any reference to a function which is called 
"printf", "vprintf", "puts", "putchar", "scanf", "getchar" or "gets".

@njames93: I would be interested to hear your opinion on this newest update. If 
I overlooked something, I would be more than happy to incorporate any further 
feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-avoid-std-io-outside-main.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s misc-avoid-std-io-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+} // namespace std
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+
+namespace arbitrary_namespace {
+std::Ostream cout{};
+std::Istream cin{};
+} // namespace arbitrary_namespace
+
+void anyNonMainFunction() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcin >> Foo;
+
+  arbitrary_namespace::cin >> Foo; // OK
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-av

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 335839.
mgartmann added a comment.

Corrected check's entry in `list.rst` after renaming the check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-avoid-std-io-outside-main.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s misc-avoid-std-io-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+} // namespace std
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+
+namespace arbitrary_namespace {
+std::Ostream cout{};
+std::Istream cin{};
+} // namespace arbitrary_namespace
+
+void anyNonMainFunction() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcin >> Foo;
+
+  arbitrary_namespace::cin >> Foo; // OK
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::printf("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  printf("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::puts("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  puts("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::putchar('m');
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  putchar('m');
+
+  char Input[5];
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::scanf("%s", Input);
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  scanf("%s", Input);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::getchar();
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functio

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.cpp:22
+  Finder->addMatcher(
+  declRefExpr(to(varDecl(hasAnyName("cin", "wcin", "cout", "wcout", "cerr",
+"wcerr"),

Would there be a way to extract these names (`cin`, `cout`, ...) into a 
separate variable? In my opinion, this would make the AST matchers cleaner and 
easier to read.

E.g., I was not able to find an overload of `hasAnyName()` which takes a 
`std:.vector` as argument.

Looking forward to hearing from you.
Thanks for any feedback in advance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

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

[PATCH] D100972: [clang-tidy] cppcoreguidelines-avoid-non-const-global-variables: add fixes to checks

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann created this revision.
mgartmann added reviewers: aaron.ballman, njames93, alexfh.
mgartmann added a project: clang-tools-extra.
Herald added subscribers: shchenz, kbarton, xazax.hun, nemanjai.
mgartmann requested review of this revision.

I added fixes to the existing checks of 
`cppcoreguidelines-avoid-non-const-global-variables`.

To fix the `non-const_variable` case, it utilizes QualType::addConst 

 to make the non-const type constant.

For the `indirection_to_non-const` case, for the sake of simplicity of this 
check, the `const` keyword is simply inserted at the variable declaration's 
beginning. If someone knows a more elegant solution to this, I am happy to 
adapt it.

Thanks in advance for any feedback!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100972

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp
@@ -2,20 +2,25 @@
 
 int nonConstInt = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'nonConstInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int nonConstInt = 0;
 
 int &nonConstIntReference = nonConstInt;
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: variable 'nonConstIntReference' provides global access to a non-const object; consider making the referenced data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int &nonConstIntReference = nonConstInt;
 
 int *pointerToNonConstInt = &nonConstInt;
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: variable 'pointerToNonConstInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
 // CHECK-MESSAGES: :[[@LINE-2]]:6: warning: variable 'pointerToNonConstInt' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const pointerToNonConstInt = &nonConstInt;
 
 int *const constPointerToNonConstInt = &nonConstInt;
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'constPointerToNonConstInt' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const constPointerToNonConstInt = &nonConstInt;
 
 namespace namespace_name {
 int nonConstNamespaceInt = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'nonConstNamespaceInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int nonConstNamespaceInt = 0;
 
 const int constNamespaceInt = 0;
 } // namespace namespace_name
@@ -24,6 +29,7 @@
 
 const int *pointerToConstInt = &constInt;
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'pointerToConstInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const pointerToConstInt = &constInt;
 
 const int *const constPointerToConstInt = &constInt;
 
@@ -39,6 +45,7 @@
 namespace {
 int nonConstAnonymousNamespaceInt = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'nonConstAnonymousNamespaceInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int nonConstAnonymousNamespaceInt = 0;
 } // namespace
 
 class DummyClass {
@@ -53,27 +60,35 @@
 
 DummyClass nonConstClassInstance;
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'nonConstClassInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyClass nonConstClassInstance;
 
 DummyClass *pointerToNonConstDummyClass = &nonConstClassInstance;
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable 'pointerToNonConstDummyClass' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
 // CHECK-MESSAGES: :[[@LINE-2]]:13: warning: variable 'pointerToNonConstDummyClass' provides global access 

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann added a comment.

@aaron.ballman Thanks a lot for your review!

Can somebody help me merging this change? I do not have the rights to do so.
Thanks in advanve!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

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

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann added a comment.

@whisperity Thanks a lot for helping me out!
Name: Marco Gartmann
E-Mail: gartmannma...@hotmail.com


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

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