nridge marked an inline comment as done.
nridge added a comment.
The updated patch addresses the infinite recursion issue by bailing on
dependent bases for now, as Sam suggested. I will implement the more
comprehensive suggested fix ("bail out once we see instantiations of the same
template decl twice in a parent chain") in a follow-up patch. I did add all
three testcases that have come up during discussion.
I believe all review comments to date are addressed now.
================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:365
Callback<tooling::Replacements> CB) {
- auto Action = [Sel](decltype(CB) CB, std::string File,
- std::string TweakID,
- Expected<InputsAndAST> InpAST) {
+ auto Action = [Sel](decltype(CB) CB, std::string File, std::string TweakID,
+ Expected<InputsAndAST> InpAST) {
----------------
kadircet wrote:
> nridge wrote:
> > kadircet wrote:
> > > Can you revert this change?
> > I tried, but clang-format automatically makes the change again.
> >
> > Is there some particular clang-format configuration I should apply, so it
> > produces the same results? I don't currently have any configuration, i.e. I
> > think it's just reading the .clang-format file in the monorepo root.
> That's what we use as well(and your formatted version is correct), but
> unfortunately sometimes people send out changes with wrong formatting and
> they are hard to notice during review. Generally we try not to touch those
> lines in irrelevant patches to keep blame clean.
>
> If you are running clang-format directly you can instead try
> clang-format-diff to format only changed lines. But not that crucial.
I'm not running clang-format directly, VSCode's clang-format extension is doing
so automatically upon every file-save. I have not found an option to configure
it to format only changed lines.
Would it help if I moved the formatting corrections to their own patch (and
made this patch depend on that)?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56370/new/
https://reviews.llvm.org/D56370
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
