[PATCH] D153228: [clang-format] Fixed bad performance with enabled qualifier fixer.

[Sedenion via Phabricator via cfe-commits]

Sedeniono marked an inline comment as done.
Sedeniono added inline comments.


================
Comment at: clang/lib/Format/Format.cpp:3571-3585
+  // Don't make replacements that replace nothing. This can affect e.g. the
+  // output of clang-format with the --output-replacements-xml option.
+  tooling::Replacements NonNoOpFixes;
+  for (const tooling::Replacement &Fix : Fixes) {
+    StringRef OriginalCode = Code.substr(Fix.getOffset(), Fix.getLength());
+    if (!OriginalCode.equals(Fix.getReplacementText())) {
+      auto Err = NonNoOpFixes.add(Fix);
----------------
owenpan wrote:
> You only need this for the `QualifierAlignment` passes as others (e.g. `IntegerLiteralSeparatorFixer`) already skip no-op replacements.
Yes, in principle I need to do this only after all QualifierAlignment passes ran. The key point is that it needs to be done only after **all** QualifierAlignment passes ran. In other words, I cannot move this to `LeftRightQualifierAlignmentFixer::analyze()`; it would do nothing there. The non-no-op code exists so that if e.g. `const volatile` becomes `volatile const` in one pass and then again `const volatile` in another pass, we end up with no replacements.

I also cannot simply put all the QualifierAlignment passes into a simple dedicated loop that directly runs them after one another; the part with `applyAllReplacements()` etc (line 3554 till 3566 above) would be missing between the passes. I could, of course, extract lines 3554 till 3566 into a new function and call it in both places. Should I do that?

Or do you mean that I should wrap the NonNoOpFixes code in an `if (Style.QualifierAlignment != FormatStyle::QAS_Leave)`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153228/new/

https://reviews.llvm.org/D153228