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
