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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D99646#2663733 , @mgartmann wrote:

> 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()`?

I mean any `DeclRefExpr` that reference those functions.
We don't actually want to only match on call expressions to those functions, we 
also want the other ways they can be called.

  auto Print = 
  Print("This is using stdio");

I could imagine this is the kind of matcher expression you need.

  declRefExpr(
   hasDeclaration(functionDecl(
   anyOf(hasDeclContext(translationUnitDecl()), isInStdNamespace()),
   hasAnyName("printf", "vprintf", ...))),
   unless(forFunction(isMain(

As for flagging the same rules should apply for references to `cin`, `cout` etc.

> 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.

You could include it in this check, then maybe rename this check to a more 
general name like `misc-avoid-stdio-outside-main`.




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

should probably use varDecl here instead of namedDecl.


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-std-stream-objects-outside-main: a new check

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



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

mgartmann wrote:
> 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? 
That was my point. I don't think that `my_namespace::cin` should be matched.


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 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 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 <<(string Message);
+};
+
+struct Istream {
+  Istream >>(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.";
+
+int 

[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 <<(string Message);
+};
+
+struct Istream {
+  Istream >>(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.rst
@@ -210,6 

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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.h:30
+return LangOpts.CPlusPlus;
+  };
+

Semicolon is not needed and should cause `-Wextra-semi` warnings.


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 <<(string Message);
+};
+
+struct Istream {
+  Istream >>(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
+++ 

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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

It looks like new patch was not uploaded.


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 , const DynTypedNode ) {

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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

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.

If you make this change the name of the check would likely need updating as 
they aren't stream objects.

Going on a tangent here, but on POSIX platforms you can redirect `stdin`, 
`stdout`, and `stderr` using `dup` and `dup2` (Windows is `_dup` and `_dup2` 
respectively), gtest uses this functionality.




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

This all seems unnecessary, you can add `unless(forFunction(isMain()))` to your 
declRefExpr matcher instead.



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
+

Why won't this work in c++11 mode?


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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.cpp:61
+} // namespace clang
\ No newline at end of file


Please add newline.



Comment at: 
clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.h:24
+class StdStreamObjectsOutsideMainCheck : public ClangTidyCheck {
+public:
+  StdStreamObjectsOutsideMainCheck(StringRef Name, ClangTidyContext *Context)

Check must work only for C++.  Please add `isLanguageVersionSupported`.



Comment at: 
clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.h:39
+#endif // 
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STDSTREAMOBJECTSOUTSIDEMAINCHECK_H
\ No newline at end of file


Please add newline.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:101
+
+  Diagnoses if a predefined standard stream object (cin, wcin,
+  cout, wcout, cerr or wcerr) is used outside the main function.

Please highlight `cin` and other with double back-ticks.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst:6
+
+The check diagnoses when a predefined standard stream object (i.e., ``cin``, 
``wcin``,
+``cout``, ``wcout``, ``cerr`` and ``wcerr``) is used outside the ``main`` 
function.

Please synchronize with Release Notes and omit //The check//.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst:46
+mocked stream objects into ``some_function()`` during testing.
\ No newline at end of file


Please add newline.


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

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



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

Will this match `my_namespace::cin`?



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

You can use `FunctionDecl::isMain`. Additionally you might want to also use 
`FunctionDecl::isMSVCRTEntryPoint` for the Windows-specific main functions.


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.

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 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++
+
+