This is an archive of the discontinued LLVM Phabricator instance.

run-clang-tidy: Fix infinite loop on windows
ClosedPublic

Authored by Febbe on Feb 10 2022, 1:58 PM.

Details

Summary

find_compilation_database checked only for "/" as exit point, but on Windows, this root is impossible.
Fixes #53642

Diff Detail

Event Timeline

Febbe created this revision.Feb 10 2022, 1:58 PM
Febbe requested review of this revision.Feb 10 2022, 1:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 10 2022, 1:58 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
Febbe retitled this revision from run-clang-tidy: Fix infinite loop on windows find_compilation_database checked only for "/" as exit point, but on windows, this root is impossible. Fixes #53642 to run-clang-tidy: Fix infinite loop on windows.Feb 10 2022, 1:59 PM
Febbe edited the summary of this revision. (Show Details)
Febbe added a comment.Feb 10 2022, 2:04 PM

Currently, only tested on Windows.
This should also increase performance, since the path is canonicalized only once.

Febbe added a comment.Feb 10 2022, 3:54 PM

I don't think I have commit rights, so someone of you also have to commit this.

Just a drive-by comment to say, thank you for taking the time to make this fix. It's a bug I've triggered many times. Great to see it being resolved.

JonasToth accepted this revision.EditedFeb 21 2022, 9:23 AM
JonasToth added a subscriber: JonasToth.

LGTM with a small nit.

i could commit on your behalf and check it for linux, too. But the change seems sane to me.

clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
69

please delete the whitespace at the end of the line

This revision is now accepted and ready to land.Feb 21 2022, 9:23 AM
Febbe updated this revision to Diff 411606.Feb 26 2022, 6:29 AM

Applied the feedback

Febbe added a comment.Feb 26 2022, 6:30 AM

@JonasToth yes, it would be nice, to test this and then push it for me. Also a backport to 14.0 would be good :).

Febbe added a comment.Feb 26 2022, 6:31 AM

@salman-javed-nz you're welcome, I only fixed it because I saw the bug in the trace directly. Normally, I would only fix C/C++ stuff :D.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 24 2022, 8:18 AM

Sorry for the long delay, i simply forgot.
The patch is commited! :)