-
Notifications
You must be signed in to change notification settings - Fork 86
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed multilevel pointer conversion tidy check errors and re-enable t… #3615
base: develop
Are you sure you want to change the base?
Conversation
test/operation.cpp
Outdated
EXPECT(test::throws([&] { migraphx::any_cast<not_operation&>(op1); })); | ||
migraphx::operation op2 = simple_operation{2}; | ||
EXPECT(migraphx::any_cast<simple_operation>(op2).data == 2); | ||
EXPECT(migraphx::any_cast<not_operation*>(&op2) == nullptr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't be modifying the test for this. Those checks are there to ensure that these resolve correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the tests back
@@ -134,7 +134,7 @@ void kernel::launch(hipStream_t stream, | |||
hipEvent_t stop) const | |||
{ | |||
assert(impl != nullptr); | |||
void* kernargs = args.data(); | |||
void* kernargs = reinterpret_cast<void*>(args.data()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense here.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3615 +/- ##
========================================
Coverage 92.18% 92.18%
========================================
Files 513 513
Lines 21576 21576
========================================
Hits 19889 19889
Misses 1687 1687 ☔ View full report in Codecov by Sentry. |
2e34b44
to
1a2b5bf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Handle the format/tidy checks that are failing.
seeing this in the tidy check now.
57880 warnings generated.
Warning: multilevel pointer conversion from 'const char **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion,-warnings-as-errors]
172 | dnnl_primitive_desc_query(desc, dnnl_query_impl_info_str, 0, &str);
| ^
CMake Error at tidy-target-migraphx_cpu-pooling_cpp.cmake:83 (message):
Clang tidy failed.
make[3]: *** [src/targets/cpu/CMakeFiles/tidy-target-migraphx_cpu-pooling_cpp.dir/build.make:71: src/targets/cpu/CMakeFiles/tidy-target-migraphx_cpu-pooling_cpp] Error 1
make[3]: Target 'src/targets/cpu/CMakeFiles/tidy-target-migraphx_cpu-pooling_cpp.dir/build' not remade because of errors.
make[2]: *** [CMakeFiles/Makefile2:18026: src/targets/cpu/CMakeFiles/tidy-target-migraphx_cpu-pooling_cpp.dir/all] Error 2
[ 20%] clang-tidy: Running clang-tidy on target compile_miopen.cpp...
59009 warnings generated.
Warning: multilevel pointer conversion from 'const char **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion,-warnings-as-errors]
172 | dnnl_primitive_desc_query(desc, dnnl_query_impl_info_str, 0, &str);
|
Format you can use with a git hook for formatting. check in tools/format.py
8176666
to
27330aa
Compare
118a810
to
3b724aa
Compare
df38b0e
to
c729117
Compare
This build is not recommended to merge 🔴 |
🔴bert_large_uncased_fp16: FAILED: MIGraphX is not within tolerance - check verbose output |
Fix #3472, fix the multilevel pointer conversion tidy check, and re-enable the check. I had to remove a couple of not_operation checks to pass the tidy check.