This revision was automatically updated to reflect the committed changes.
Closed by commit rL350922: [clang-tidy] new check
'readability-redundant-preprocessor' (authored by vmiklos, committed
by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D543
vmiklos added a comment.
In https://reviews.llvm.org/D54349#1301663, @aaron.ballman wrote:
> I think this check is in good shape for the initial commit. Additional
> functionality can be added incrementally.
OK, thanks. I'll lcommit this once https://reviews.llvm.org/D54450 is committed.
htt
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
I think this check is in good shape for the initial commit. Additional
functionality can be added incrementally.
Comment at: test/clang-tidy/readability-redund
vmiklos marked an inline comment as done.
vmiklos added inline comments.
Comment at: test/clang-tidy/readability-redundant-preprocessor.cpp:53
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant #if; consider
removing it [readability-redundant-preprocessor]
+#if FOO == 3 +
aaron.ballman added inline comments.
Comment at: test/clang-tidy/readability-redundant-preprocessor.cpp:53
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant #if; consider
removing it [readability-redundant-preprocessor]
+#if FOO == 3 + 1
+// CHECK-NOTES: [[@LINE-3]]:2: n
vmiklos marked 2 inline comments as done.
vmiklos added inline comments.
Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:37
+CheckMacroRedundancy(Loc, Condition, IfStack,
+ "nested redundant if; consider removing it",
+
vmiklos updated this revision to Diff 174439.
https://reviews.llvm.org/D54349
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantPreprocessorCheck.cpp
clang-tidy/readability/RedundantPreprocessorCheck.h
docs/Rele
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:37
+CheckMacroRedundancy(Loc, Condition, IfStack,
+ "nested redundant if; consider removing it",
+ "previous if was here", tr
vmiklos updated this revision to Diff 174021.
vmiklos marked 2 inline comments as done.
https://reviews.llvm.org/D54349
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantPreprocessorCheck.cpp
clang-tidy/readabilit
vmiklos marked 2 inline comments as done.
vmiklos added inline comments.
Comment at: clang-tidy/readability/ReadabilityTidyModule.cpp:84
+CheckFactories.registerCheck(
+"readability-redundant-preprocessor");
CheckFactories.registerCheck(
aaron.ba
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/ReadabilityTidyModule.cpp:84
+CheckFactories.registerCheck(
+"readability-redundant-preprocessor");
CheckFactories.registerCheck(
Please keep this list sorted alphabetically.
Szelethus added inline comments.
Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:56-59
+StringRef SourceText =
+Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange),
+ PP.getSourceManager(), PP.getLangOpts()
vmiklos updated this revision to Diff 173575.
vmiklos marked an inline comment as done.
https://reviews.llvm.org/D54349
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantPreprocessorCheck.cpp
clang-tidy/readabilit
vmiklos marked 2 inline comments as done.
vmiklos added inline comments.
Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:56-59
+StringRef SourceText =
+Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange),
+
Szelethus added inline comments.
Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:56-59
+StringRef SourceText =
+Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange),
+ PP.getSourceManager(), PP.getLangOpts()
vmiklos marked an inline comment as done.
vmiklos added inline comments.
Comment at: docs/clang-tidy/checks/readability-redundant-preprocessor.rst:34
+
+ #ifdef FOO
+ #ifndef FOO // inner ifndef is considered redundant
JonasToth wrote:
> The indendation for these
vmiklos updated this revision to Diff 173572.
https://reviews.llvm.org/D54349
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantPreprocessorCheck.cpp
clang-tidy/readability/RedundantPreprocessorCheck.h
docs/Rele
JonasToth added inline comments.
Comment at: docs/clang-tidy/checks/readability-redundant-preprocessor.rst:34
+
+ #ifdef FOO
+ #ifndef FOO // inner ifndef is considered redundant
The indendation for these examples (following as well) is not enough. Please
use 2
vmiklos added a comment.
In https://reviews.llvm.org/D54349#1294600, @vmiklos wrote:
> Let's see if that works in practice.
I've implemented this now, please take a look. (Extended test + docs as well,
as usual.) Thanks!
https://reviews.llvm.org/D54349
vmiklos updated this revision to Diff 173556.
vmiklos marked an inline comment as done.
https://reviews.llvm.org/D54349
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantPreprocessorCheck.cpp
clang-tidy/readabilit
vmiklos marked an inline comment as done.
vmiklos added a comment.
> As it stands, I feel like this check is a bit too simplistic to have much
> value, but if you covered some of these other cases, it would alleviate that
> worry for me.
I've now added support for detecting inverted conditions
vmiklos updated this revision to Diff 173554.
https://reviews.llvm.org/D54349
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantPreprocessorCheck.cpp
clang-tidy/readability/RedundantPreprocessorCheck.h
docs/Rele
aaron.ballman added a comment.
In https://reviews.llvm.org/D54349#1294325, @vmiklos wrote:
> > What do you think about code like:
> >
> > #if FOO == 4
> > #if FOO == 4
> > #endif
> > #endif
> >
> > #if defined(FOO)
> > #if defined(FOO)
> > #endif
> > #endif
> >
> > #if !d
Szelethus added inline comments.
Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:19-22
+struct Entry {
+ SourceLocation Loc;
+ std::string MacroName;
+};
vmiklos wrote:
> Szelethus wrote:
> > This is a way too general name in my opinion. Eithe
vmiklos updated this revision to Diff 173526.
vmiklos marked an inline comment as done.
https://reviews.llvm.org/D54349
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantPreprocessorCheck.cpp
clang-tidy/readabilit
vmiklos marked an inline comment as done.
vmiklos added inline comments.
Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:19-22
+struct Entry {
+ SourceLocation Loc;
+ std::string MacroName;
+};
Szelethus wrote:
> This is a way too general name
vmiklos added a comment.
> What do you think about code like:
>
> #if FOO == 4
> #if FOO == 4
> #endif
> #endif
>
> #if defined(FOO)
> #if defined(FOO)
> #endif
> #endif
>
> #if !defined(FOO)
> #if !defined(FOO)
> #endif
> #endif
I looked at supporting these, but it
vmiklos updated this revision to Diff 173524.
https://reviews.llvm.org/D54349
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantPreprocessorCheck.cpp
clang-tidy/readability/RedundantPreprocessorCheck.h
docs/Rele
Szelethus added inline comments.
Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:19-22
+struct Entry {
+ SourceLocation Loc;
+ std::string MacroName;
+};
This is a way too general name in my opinion. Either include comments that
describe it,
JonasToth added a comment.
> Done.
Please also mark the inline comments as done. Otherwise its hard to follow if
there are outstanding issues somewhere, especially if code moves around.
https://reviews.llvm.org/D54349
___
cfe-commits mailing list
aaron.ballman added a comment.
What do you think about code like:
#if FOO == 4
#if FOO == 4
#endif
#endif
#if defined(FOO)
#if defined(FOO)
#endif
#endif
#if !defined(FOO)
#if !defined(FOO)
#endif
#endif
#if defined(FOO)
#if !defined(FOO)
#endif
#endif
vmiklos updated this revision to Diff 173468.
vmiklos edited the summary of this revision.
https://reviews.llvm.org/D54349
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantPreprocessorCheck.cpp
clang-tidy/readabi
vmiklos added a comment.
> preprocessor directives? Same in documentation.
Done.
https://reviews.llvm.org/D54349
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
vmiklos added a comment.
> I think that name is not very descriptive for the user of clang-tidy. `pp`
> should be `preprocessor` or some other constellation that makes it very clear
> its about the preprocessor.
Done, renamed to readability-redundant-preprocessor.
> you are in namespace `clang
vmiklos updated this revision to Diff 173465.
vmiklos retitled this revision from "[clang-tidy] new check
'readability-redundant-pp'" to "[clang-tidy] new check
'readability-redundant-preprocessor'".
https://reviews.llvm.org/D54349
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/rea
35 matches
Mail list logo
