gamesh411 marked 13 inline comments as done.
gamesh411 added a comment.
Note that there are no negative test cases that assert that we do NOT report in
case a custom or an anonymous namespace is used. For that I would need a small
patch in the testing infrastructure.
Patch needed in check_clang_tidy.py:
--- a/clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ b/clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -167,7 +167,7 @@ def run_test_once(args, extra_args):
subprocess.check_output(
['FileCheck', '-input-file=' + temp_file_name, input_file_name,
'-check-prefixes=' + ','.join(check_fixes_prefixes),
- '-strict-whitespace'],
+ '-strict-whitespace', '--allow-empty'],
stderr=subprocess.STDOUT)
except subprocess.CalledProcessError as e:
print('FileCheck failed:\n' + e.output.decode())
@@ -180,7 +180,7 @@ def run_test_once(args, extra_args):
subprocess.check_output(
['FileCheck', '-input-file=' + messages_file, input_file_name,
'-check-prefixes=' + ','.join(check_messages_prefixes),
- '-implicit-check-not={{warning|error}}:'],
+ '-implicit-check-not={{warning|error}}:', '--allow-empty'],
stderr=subprocess.STDOUT)
except subprocess.CalledProcessError as e:
print('FileCheck failed:\n' + e.output.decode())
@@ -195,7 +195,7 @@ def run_test_once(args, extra_args):
subprocess.check_output(
['FileCheck', '-input-file=' + notes_file, input_file_name,
'-check-prefixes=' + ','.join(check_notes_prefixes),
- '-implicit-check-not={{note|warning|error}}:'],
+ '-implicit-check-not={{note|warning|error}}:', '--allow-empty'],
stderr=subprocess.STDOUT)
except subprocess.CalledProcessError as e:
print('FileCheck failed:\n' + e.output.decode())
And then I can assert the non-reports by adding the following runlines:
// Functions in anonymous or custom namespace should not be considered as exit
// functions.
//
// RUN: %check_clang_tidy -assume-filename=%s.cpp %s -check-suffix=CUSTOM \
// RUN: cert-env32-c %t -- -- -DCPPMODE -DTEST_NS_NAME=custom
// CHECK-NOTES-CUSTOM-NOT: NO DIAGS
//
// RUN: %check_clang_tidy -assume-filename=%s.cpp %s -check-suffix=ANONYMOUS \
// RUN: cert-env32-c %t -- -- -DCPPMODE -DTEST_NS_NAME=''
// CHECK-NOTES-ANONYMOUS-NOT: NO DIAGS
================
Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:70
+ .bind("handler_expr"));
+ Finder->addMatcher(
+ callExpr(IsRegisterFunction, HasHandlerAsFirstArg).bind("register_call"),
----------------
aaron.ballman wrote:
> I am not at all certain whether this is plausible, but I *think* this check
> can be almost entirely implemented in the matchers rather than having to do
> manual work. I think you can bind the argument node from the call to
> `at_quick_exit()` and then use `equalsBoundNode()` to find the function calls
> within the bound `functionDecl()` node.
>
> However, if that's not workable, I think you can get rid of the
> `CalledFunctionsCollector` entirely and just use matchers directly within the
> `check()` function because by that point, you'll know exactly which AST nodes
> you want to traverse through.
I have investigated, and to do that recursive matching up to indeterminate
depth all the while keeping the already matched functions in a set is not
something I would implement with a single matcher. I could use a standalone
matcher, but as far as I can understand I would need to implement a callback
function for handling the matched results out of line for that as well, so I
think that the ASTVisitor based solution is at least as good as a standalone
ASTMatcher would be. Therefore I'd rather keep this solution.
================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cert-env32-c.c:31
+// --------------
+// C99 requires jmp_buf to be an array type.
+typedef int jmp_buf[1];
----------------
whisperity wrote:
> Which is the standard version this test file is set to analyse with? I don't
> see any `-std=` flag in the `RUN:` line.
Right now there is a run-line for handling it as C, and 2 others for handling
it as C++ with different namespaces. The test cases themselves are not
dependent on the standard used (AFAIK). The comment contains a standard
reference for C, but that is just the first time it was standardized in C so
the mention is there for traceability and not to restrict the standard used.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83717/new/
https://reviews.llvm.org/D83717
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
