[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

carlosgalvezp added a comment. Please note, `HeaderFileExtensions` is becoming a global option in this patch to avoid duplication, which will land in the next 2 days. https://reviews.llvm.org/D141000 Therefore you'll need to update the patch based on that. CHANGES SINCE LAST ACTION

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

KyleFromKitware added a comment. In D142123#4071099 , @njames93 wrote: > Can you just elaborate on what the registry is used for? Is the plan to > support potentially dynamically loading `HeaderGuardStyle` classes. Yes. > If thats the case, Then I'd

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

njames93 added a comment. Also this goes for all your PRs, can you please upload them in future with full context `git diff -U99` should do it. Makes revewing changes easier and removes those `Context not available` message on phab CHANGES SINCE LAST ACTION

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

njames93 added a comment. Can you just elaborate on what the registry is used for? Is the plan to support potentially dynamically loading `HeaderGuardStyle` classes. If thats the case, Then I'd argue we don't need the `HeaderGuardBase` check class, We can just create a `HeaderGuardCheck` class

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

aaron.ballman added a comment. In D142123#4066805 , @KyleFromKitware wrote: > The refactoring that I've done in D142121 > is most of the way to being able to implement my above proposal. I just need > to shift a few

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

KyleFromKitware added a comment. Basically, anything that extends `HeaderGuardCheck` (or `HeaderGuardBase` from D142121 ) is really just a fancy configuration option, rather than a separate check, and should be treated as such. My proposal is this: 1. Create

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

lebedev.ri added a comment. In D142123#4066460 , @aaron.ballman wrote: > In D142123#4066447 , @lebedev.ri > wrote: > >> In D142123#4066351 , @njames93 >> wrote: >> >>>

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

KyleFromKitware added a comment. In D142123#4066706 , @KyleFromKitware wrote: > The only way to avoid this is if we overhaul the existing `HeaderGuardCheck` > and make its naming convention configurable by a configuration option as > opposed to having

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

KyleFromKitware added a comment. In D142123#4066676 , @aaron.ballman wrote: > It doesn't have to -- for example, we can name the guard based on the path to > the header. e.g., `foo/include/bar/baz.h` could use `FOO_INCLUDE_BAR_BAZ_H` > as the header

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

aaron.ballman added a comment. In D142123#4066564 , @KyleFromKitware wrote: > In terms of a "generic check", there is already the `HeaderGuardCheck`, but > that has to be extended to be specific to the project's naming convention. > And I don't think

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

KyleFromKitware added a comment. In terms of a "generic check", there is already the `HeaderGuardCheck`, but that has to be extended to be specific to the project's naming convention. And I don't think there can be a generic check that converts `#pragma once` to a header guard - when it adds

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

KyleFromKitware added a comment. In D142123#4066460 , @aaron.ballman wrote: > In D142123#4066447 , @lebedev.ri > wrote: > >> In D142123#4066351 , @njames93 >> wrote:

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

aaron.ballman added a comment. In D142123#4066447 , @lebedev.ri wrote: > In D142123#4066351 , @njames93 > wrote: > >> Given the fact its non-standard, it has caveats and peoples eagerness to >> blindly enable

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

aaron.ballman added a comment. In D142123#4066023 , @KyleFromKitware wrote: > We have this check in a custom clang-tidy module that runs on CMake's CI > system. Other people >

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

lebedev.ri added a comment. In D142123#4066351 , @njames93 wrote: > Given the fact its non-standard, it has caveats and peoples eagerness to > blindly enable all checks (or all checks from a module). > I feel this check would likely cause more harm

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

KyleFromKitware added a comment. In D142123#4066351 , @njames93 wrote: > and peoples eagerness to blindly enable all checks (or all checks from a > module) Perhaps there's some way we can have it disabled by default and not enabled unless they

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

njames93 added a comment. Given the fact its non-standard, it has caveats and peoples eagerness to blindly enable all checks (or all checks from a module). I feel this check would likely cause more harm than good. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142123/new/

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

KyleFromKitware added a comment. We have this check in a custom clang-tidy module that runs on CMake's CI system. Other people have also sought out such a check, and I

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

aaron.ballman added a comment. Can you explain a bit about why you want to promote use of `#pragma once`? It's not been standardized specifically because of how many known issues it causes, so it strikes me as odd to claim it's a modernization of a code base to use it. To me, I'd almost rather

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

KyleFromKitware updated this revision to Diff 490534. KyleFromKitware added a comment. Added documentation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142123/new/ https://reviews.llvm.org/D142123 Files: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

KyleFromKitware created this revision. KyleFromKitware added a reviewer: clang-tools-extra. KyleFromKitware created this object with edit policy "Only User: KyleFromKitware (Kyle Edwards)". KyleFromKitware added a project: clang-tools-extra. Herald added subscribers: carlosgalvezp, xazax.hun.