FabioRS added a comment. In D122698#3418018 <https://reviews.llvm.org/D122698#3418018>, @sammccall wrote:
> Thanks! I'll look at the changes in the morning, just wanted to mention one > thing > > In D122698#3417946 <https://reviews.llvm.org/D122698#3417946>, @FabioRS wrote: > >> I can not run the test the consteval, so it is not in the diff. >> >> TestTU failed to build (suppress with /*error-ok*/): >> [1:4-1:13] unknown type name 'consteval'; did you mean 'constexpr'?, >> fixes: {change 'consteval' to 'constexpr' {1:4-1:13 => "constexpr"}} > > It's a c++20 feature, so you'll need to add something like > `ExtraArgs.push_back("-std=c++20");`. > > Though if there's no real difference between handling constexpr vs consteval, > up to you whether it's worth a separate test. Thank you! I think it is interesting to have the test to keep the consteval branch covered. I think maybe there will be more things to change, so I can send the test case for consteval tomorrow with the others changes. ================ Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:455 + return std::string(llvm::formatv("{0}{1} {2}({3}){4}", renderAttributes(), + printType(ReturnType, *EnclosingFuncContext), + Name, renderParametersForDefinition(), ---------------- sammccall wrote: > For out-of-line defined methods, this is the wrong DC, which may lead to the > type being incorrectly qualified. The DC should rather be the enclosing class. > > Similarly, renderParametersForDefinition uses EnclosingFuncContext too - > instead it should take the EnclosingFuncContext as a parameter. (And be > renamed `renderParametersForDeclaration`, since it's no longer just for > definitions) It is only for out-of-line methods? I put this verification in the getEnclosing() of the new diff. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122698/new/ https://reviews.llvm.org/D122698 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
