sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:433
+ // inside Extraction Zone.
+ void incrementLoopSwitchCounters(Stmt *S) {
+ if (CurrentLocation != ZoneRelative::Inside)
----------------
rather than writing this twice, take an `int Increment` parameter?
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:480
+ if (CurrentLocation == ZoneRelative::Inside &&
+ !(CurNumberOfLoops + CurNumberOfSwitch))
+ Info.BrokenControlFlow = true;
----------------
`(NumberOfLoops > 0 || NumberOfSwitch = 0)`
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:497
+ // stack inside Extraction Zone. Used to check for broken control flow.
+ unsigned CurNumberOfLoops = 0;
+ unsigned CurNumberOfSwitch = 0;
----------------
NestedLoops would be clearer I think
================
Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:523
EXPECT_THAT(apply(" for([[int i = 0;]];);"), HasSubstr("extracted"));
+ // We should be able to extract break/continue with a parent loop/switch.
+ EXPECT_THAT(apply(" [[for(;;) if(1) break;]] "), HasSubstr("extracted"));
----------------
please add these to a new test case e.g. `TEST_F(ExtractFunctionTest,
ControlFlow)` to avoid each getting too long
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66732/new/
https://reviews.llvm.org/D66732
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
