ROCm / rocm-cmake

CMake modules used within the ROCm libraries
https://rocm.docs.amd.com/projects/ROCmCMakeBuildTools/en/latest/
MIT License
62 stars 43 forks source link

Add `--config` option for clang version < 14 #126

Closed umangyadav closed 10 months ago

umangyadav commented 1 year ago

--config-file option is only available for clang v14++.

https://releases.llvm.org/14.0.0/tools/clang/tools/extra/docs/clang-tidy/index.html

For versions < 14, need to use --config

https://releases.llvm.org/13.0.0/tools/clang/tools/extra/docs/clang-tidy/index.html

umangyadav commented 1 year ago

Hold on from merging this :

For some reason --config option is not parsing .clang-tidy file correctly for MIGraphX.

pfultz2 commented 1 year ago

For some reason --config option is not parsing .clang-tidy file

Ah, the --config flag takes a string not a file, but can be left empty and clang-tidy will search for .clang-tidy file.

correctly for MIGraphX.

But we use at least clang v15 in migraphx so I dont see how this would affect it.

umangyadav commented 1 year ago

But we use at least clang v15 in migraphx so I dont see how this would affect it.

In github actions it uses clang v11.

pfultz2 commented 1 year ago

In github actions it uses clang v11.

It uses the clang-tidy from rocm. Where are you seeing it using clang v11?

umangyadav commented 1 year ago

In github actions it uses clang v11.

It uses the clang-tidy from rocm. Where are you seeing it using clang v11?

GH actions wouldn't have ROCm installed.

https://github.com/ROCmSoftwarePlatform/AMDMIGraphX/actions/runs/4690184294/jobs/8312941796?pr=1680

pfultz2 commented 1 year ago

GH actions wouldn't have ROCm installed.

https://github.com/ROCmSoftwarePlatform/AMDMIGraphX/actions/runs/4690184294/jobs/8312941796?pr=1680

We dont run clang-tidy in those jobs. The job that runs clang tidy has rocm installed in the docker container.

umangyadav commented 1 year ago

We dont run clang-tidy in those jobs. The job that runs clang tidy has rocm installed in the docker container.

Yes, It just CMake. It prints the checks options for the clang-tidy. Build process/Actions themselves are not running tidy checks unless requested by make tidy.

https://github.com/RadeonOpenCompute/rocm-cmake/blob/026eb10b74bccc91f45c20fde1d9714e55f4bf3d/share/rocm/cmake/ROCMClangTidy.cmake#L152

eidenyoshida commented 10 months ago

@umangyadav wondering if this is still an issue or if this PR can be closed? thanks

umangyadav commented 10 months ago

@umangyadav wondering if this is still an issue or if this PR can be closed? thanks

Closing this ticket for now. I can reopen if necessary.