[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

This revision was automatically updated to reflect the committed changes. Closed by commit rG52296f5ed88b: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check (authored by PiotrZSL). Changed prior to commit: https://reviews.llvm.org/D145617?vs=508321=508408#toc Repository:

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145617/new/ https://reviews.llvm.org/D145617

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

carlosgalvezp added a comment. In D145617#4221369 , @PiotrZSL wrote: > @carlosgalvezp Do you want me to shorten documentation ? Or we leave it like > it is ? It's fine for me either way! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

PiotrZSL updated this revision to Diff 508321. PiotrZSL added a comment. Fix doc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145617/new/ https://reviews.llvm.org/D145617 Files:

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

PiotrZSL updated this revision to Diff 508320. PiotrZSL added a comment. Doc update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145617/new/ https://reviews.llvm.org/D145617 Files:

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:139 + + Check flags always enabled or disabled code blocks in preprocessor ``#if`` + conditions, such as ``#if 0`` and ``#if 1`` etc. Please omit `Check`.

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

PiotrZSL updated this revision to Diff 508302. PiotrZSL added a comment. Rebase + Shorten documentation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145617/new/ https://reviews.llvm.org/D145617 Files:

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

PiotrZSL added a comment. @carlosgalvezp Do you want me to shorten documentation ? Or we leave it like it is ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145617/new/ https://reviews.llvm.org/D145617

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

carlosgalvezp requested changes to this revision. carlosgalvezp added a comment. This revision now requires changes to proceed. I noticed the pre-merge checks are red, however, would be good to get them fixed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM thank you! Really useful check! I'm not very expert in the PP callbacks so I can't give much feedback, feel free to wait for other more expert reviewers if you want.

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

PiotrZSL updated this revision to Diff 506310. PiotrZSL added a comment. Change name of isStatic* to isImmutable* Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145617/new/ https://reviews.llvm.org/D145617 Files:

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp:41 + + bool isStatic(SourceManager , const LangOptions , +SourceRange ConditionRange) { "static" is an

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

PiotrZSL added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability/avoid-unconditional-preprocessor-if.cpp:67-69 +#if 10 > DDD +// some code +#endif carlosgalvezp wrote: > Add case for if 10 < DDD ? > > Also, add test case for

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

PiotrZSL updated this revision to Diff 506304. PiotrZSL marked 3 inline comments as done. PiotrZSL added a comment. Removed some whitespace & added tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145617/new/ https://reviews.llvm.org/D145617

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

carlosgalvezp added a comment. Looks good, minor comments! Comment at: clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.h:16 + +/// Check flags always enabled or disabled code blocks in preprocessor `#if` +/// conditions, such as `#if 0` and

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

PiotrZSL added a comment. There are 186 findings from this check in llvm repository, no false-positives. Example: llvm/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp:2071:2: warning: preprocessor condition is always 'true', consider removing condition but leaving its contents

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

PiotrZSL updated this revision to Diff 503616. PiotrZSL edited the summary of this revision. PiotrZSL added a comment. Review comments fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145617/new/ https://reviews.llvm.org/D145617 Files:

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp:29 + return; +auto = PP.getSourceManager(); +if (!isStatic(SM, PP.getLangOpts(), ConditionRange)) Please don't use

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

PiotrZSL updated this revision to Diff 503529. PiotrZSL added a comment. Micro fixes in documentation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145617/new/ https://reviews.llvm.org/D145617 Files:

[PATCH] D145617: [clang-tidy] Add readability-avoid-unconditional-preprocessor-if check

PiotrZSL created this revision. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a reviewer: njames93. Herald added a project: All. PiotrZSL requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Check flags always