[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

2020-11-19 Thread Nathan James via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG617e8e5ee3bb: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches… (authored by njames93).

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

2020-11-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return-pp-no-crash.cpp:1 +// RUN: clang-tidy %s -checks=-*,readability-else-after-return -- -std=c++11 +

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

2020-11-18 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 306143. njames93 added a comment. Added test cases to ensure clang-tidy doesn't crash. Expanded auto out. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91485/new/ https://reviews.llvm.org/D91485 Files:

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

2020-11-18 Thread Stephen Kelly via Phabricator via cfe-commits
steveire requested changes to this revision. steveire added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp:34 + return; +auto = Collections[SM.getFileID(Loc)]; +

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

2020-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from the testing request. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp:313 +#endif +}

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

2020-11-17 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 305898. njames93 added a comment. Whoops missed one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91485/new/ https://reviews.llvm.org/D91485 Files:

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

2020-11-17 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 305897. njames93 marked an inline comment as done. njames93 added a comment. Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91485/new/ https://reviews.llvm.org/D91485 Files:

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

2020-11-17 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 8 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp:313 +#endif +} aaron.ballman wrote: > We should probably add some tests for more pathological

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

2020-11-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp:118 static void removeElseAndBrackets(DiagnosticBuilder , ASTContext , - const Stmt *Else, SourceLocation ElseLoc) { +

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

2020-11-15 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 305373. njames93 added a comment. Fix assertions failing with included files. Ran this over the entire clang directory, which has a lot of macro (ab)use, and no crashes or otherwise unexpected behaviour. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

2020-11-14 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. njames93 requested review of this revision. Consider this code: if (Cond) { #ifdef X_SUPPORTED X(); #else return;