This is an archive of the discontinued LLVM Phabricator instance.

[Clangd] Ensure children are always RootStmt in ExtractFunction (Fixes #153)
ClosedPublic

Authored by SureYeaah on Sep 28 2019, 8:06 AM.

Details

Summary

We weren't always checking if children are RootStmts in ExtractFunction.

For void f([[int a]]);, the ParmVarDecl appeared as a RootStmt since
we didn't perform the check and ended up being casted to a (null) Stmt.

Event Timeline

SureYeaah created this revision.Sep 28 2019, 8:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 28 2019, 8:06 AM
SureYeaah edited the summary of this revision. (Show Details)Sep 28 2019, 8:08 AM
SureYeaah updated this revision to Diff 222293.Sep 28 2019, 8:10 AM

Added llmv_unreachable

SureYeaah updated this revision to Diff 222499.Sep 30 2019, 2:19 PM

Moved Unittest

kadircet added inline comments.Oct 1 2019, 3:02 AM
clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
110

can you also update the comment to mention

returns null if any child of the parent is not a root statement

111

can we restructure this into:

if (!CommonAnc) return nullptr;
const Node *Parent = nullptr;
switch(...) {
...Fill In Parent...
default:
llvm_unreachable(..)
}
return llvm::all_of(..)
SureYeaah updated this revision to Diff 222585.Oct 1 2019, 3:57 AM
SureYeaah marked 2 inline comments as done.

Address review comments

This revision was not accepted when it landed; it landed in state Needs Review.Oct 2 2019, 6:49 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptOct 2 2019, 6:49 AM