[PATCH] D114837: format: Remove redundant calls to guessIsObjC to speed up clang-format on unknown file types
davidvc1 added a comment. Herald added a project: All. @curdeius , the PR description mentions the reason: when you pipe input from stdin, clang-format has to run through the "is it objective C?" codepath, and this is the codepath with the bug. On the other hand, when you pass a .cpp file as a file path input, it doesn't need to figure out whether the code is objective C. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114837/new/ https://reviews.llvm.org/D114837 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D114837: format: Remove redundant calls to guessIsObjC to speed up clang-format on unknown file types
davidvc1 added a comment. + @rsmith from CODE_OWNERS.txt: Hi Richard, could you please suggest a reviewer for this change? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114837/new/ https://reviews.llvm.org/D114837 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D114837: format: Remove redundant calls to guessIsObjC to speed up clang-format on unknown file types
davidvc1 added a comment. Hi @curdeius, I'm new to the project and I'm adding you as a reviewer because I noticed you reviewed a number of recent changes in this directory. Are you a good reviewer for this change? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114837/new/ https://reviews.llvm.org/D114837 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D114837: format: Remove redundant calls to guessIsObjC to speed up clang-format on unknown file types
davidvc1 created this revision.
davidvc1 added a reviewer: curdeius.
davidvc1 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Running `clang-format` on the following input
int lambdas() {
return [&] {
return [&] {
return [&] {
return [&] {
return [&] {
return [&] {
return [&] { return 3; } ();
} (); } (); } (); } (); } (); } (); }
will finish instantly if you pass clang-format a .cpp input with this content,
but hang for tens of seconds if you pass the same via stdin.
Adding some debug statements showed that guessIsObjC was getting called
tens of millions of times in a manner that scales very rapidly with the amount
of nesting (if `clang-format` just takes a few seconds with that input passed
on stdin, try adding a couple more levels of nesting).
This change moves the recursive guessIsObjC call one level of nesting out of
an inner loop whose iterations don't affect the input to the recursive call.
This
resolves the performance issue.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D114837
Files:
clang/lib/Format/Format.cpp
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2365,9 +2365,9 @@
<< getTokenTypeName(FormatTok->getType()) << "\n");
return true;
}
-if (guessIsObjC(SourceManager, Line->Children, Keywords))
- return true;
}
+ if (guessIsObjC(SourceManager, Line->Children, Keywords))
+return true;
}
return false;
}
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2365,9 +2365,9 @@
<< getTokenTypeName(FormatTok->getType()) << "\n");
return true;
}
-if (guessIsObjC(SourceManager, Line->Children, Keywords))
- return true;
}
+ if (guessIsObjC(SourceManager, Line->Children, Keywords))
+return true;
}
return false;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D89920: Export TemplateArgumentMatcher so clients defining custom matchers don't need to use the internal namespace
davidvancleve added a comment. Hi klimek, any more action needed on my part to land this? This is my first LLVM change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89920/new/ https://reviews.llvm.org/D89920 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D89920: Export TemplateArgumentMatcher so clients defining custom matchers don't need to use the internal namespace
davidvancleve added a comment. +klimek: I added you from glancing at CODE_OWNERS.txt; are you a good reviewer? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89920/new/ https://reviews.llvm.org/D89920 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D89920: Export TemplateArgumentMatcher so clients defining custom matchers don't need to use the internal namespace
davidvancleve created this revision. davidvancleve added a reviewer: LLVM. Herald added a project: clang. Herald added a subscriber: cfe-commits. davidvancleve requested review of this revision. This change adds another export, `using TemplateArgumentMatcher = internal::Matcher;`, to the collection of exports that put instantiations of the `clang::ast_matchers::internal::Matcher` into the `clang::ast_matchers` namespace. This makes it possible to define custom TemplateArgument matchers without reaching into the `internal` namespace. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D89920 Files: clang/include/clang/ASTMatchers/ASTMatchers.h Index: clang/include/clang/ASTMatchers/ASTMatchers.h === --- clang/include/clang/ASTMatchers/ASTMatchers.h +++ clang/include/clang/ASTMatchers/ASTMatchers.h @@ -145,6 +145,7 @@ using NestedNameSpecifierMatcher = internal::Matcher; using NestedNameSpecifierLocMatcher = internal::Matcher; using CXXCtorInitializerMatcher = internal::Matcher; +using TemplateArgumentMatcher = internal::Matcher; using TemplateArgumentLocMatcher = internal::Matcher; /// @} Index: clang/include/clang/ASTMatchers/ASTMatchers.h === --- clang/include/clang/ASTMatchers/ASTMatchers.h +++ clang/include/clang/ASTMatchers/ASTMatchers.h @@ -145,6 +145,7 @@ using NestedNameSpecifierMatcher = internal::Matcher; using NestedNameSpecifierLocMatcher = internal::Matcher; using CXXCtorInitializerMatcher = internal::Matcher; +using TemplateArgumentMatcher = internal::Matcher; using TemplateArgumentLocMatcher = internal::Matcher; /// @} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83099: [clangd] Store index in '.cache/clangd/index' instead of '.clangd/index'
davidvancleve added a comment. Seems like it's working as expected now! Thanks again. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83099/new/ https://reviews.llvm.org/D83099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83099: [clangd] Store index in '.cache/clangd/index' instead of '.clangd/index'
davidvancleve added a comment. Super! Once we pull in that version (unsure of the latency; this is my first time reporting an issue with clangd), I'll be sure to come back and confirm that the fix is working for me. Thank you again for the quick turnaround! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83099/new/ https://reviews.llvm.org/D83099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83099: [clangd] Store index in '.cache/clangd/index' instead of '.clangd/index'
davidvancleve added a comment. Yes, these two are exactly the case! > - you'd be using an editor/plugin that sends compile commands over LSP (such > as YCM + ycm_extra_conf). What are you using? > - there should be relatively few *.idx files inside the extra directories > (the ones not near your compilation database), corresponding to files you've > had open rather than the whole project Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83099/new/ https://reviews.llvm.org/D83099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83099: [clangd] Store index in '.cache/clangd/index' instead of '.clangd/index'
davidvancleve added a comment. Thanks for the quick response! Nope, no compile_commands.json files, and they didn't previously contain .clangd files. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83099/new/ https://reviews.llvm.org/D83099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83099: [clangd] Store index in '.cache/clangd/index' instead of '.clangd/index'
davidvancleve added a comment. Hey, quick question about this change: I'm seeing .cache directories appear off of subdirectories too, not just my project root: Untracked files: 31 #» .cache/ 32 #» content/browser/frame_host/.cache/ 33 #» content/public/browser/.cache/ 34 #» content/test/.cache/ 35 #» net/http/.cache/ 36 #» services/network/.cache/ 37 #» services/network/trust_tokens/.cache/ Is this working as intended? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83099/new/ https://reviews.llvm.org/D83099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
