[clang-tools-extra] [clangd] Extract Function: add hoisting support (PR #75533)

Julian Schmidt via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 14 13:36:25 PST 2023


5chmidti wrote:

This is a revival of https://reviews.llvm.org/D138499.
There was no previous review on phabricator.

Open questions:
- I think that `<tuple>`/`<utility>` should be included if a `tuple` or `pair` is used, but couldn't figure out a clean way to include the headers. It looks like the way to go would be through `IncludeCleaner`s `insert` function, but for that I would need to construct the `IncludeInserter`. I don't think I can get the `FormatStyle` or `BuildDir` from inside a tweak. Does anyone have an idea, or is this a non-issue?
- The tweak is unavailable if a tuple or pair is required for the hoisting, but no auto return-type deduction is available. This is the case because I implemented the return type of the hoisted variables to use `tuple` or `pair` if two or more variables are returned. To keep the return type short, I set the return type in these cases to `auto`. Should the return type be fully spelled out? Should it be spelled out always, or should there be a config option (seems a bit overkill)?
- I'm not a fan of the `const&` `HoistSet` member that I used. Should I change this to a pointer (it's always non-null)? The `HoistSet` gets created during `prepare`, and used in `apply`.


https://github.com/llvm/llvm-project/pull/75533


More information about the cfe-commits mailing list