Sedeniono added a comment. Heh, ok, so I wasn't that naive then to not run the tests of everything :-)
I had a look at the issue. The ClangRenameTests first do some replacements, and then call formatAndApplyReplacements() <https://github.com/llvm/llvm-project/blob/9598fe54e216d78da017f804382e9971918fff2d/clang/include/clang/Tooling/Refactoring.h#L92> with them, which also formats the lines containing the replacements. With my patch, calling `clang-format.exe -style=llvm --lines=13:13` on the following file (line 13 is the `OldAlias old_alias;`) #include "header.h" namespace x { class Old {}; } namespace ns { #define REF(alias) alias alias_var; #define ALIAS(old) \ using old##Alias = x::old; \ REF(old##Alias); ALIAS(Old); OldAlias old_alias; } triggers the assert in adjustToUnmodifiedLine() <https://github.com/llvm/llvm-project/blob/9598fe54e216d78da017f804382e9971918fff2d/clang/lib/Format/UnwrappedLineFormatter.cpp#L94>. This is what happens in the `RenameAliasTest.AliasesInMacros` test. The issue originates already from the line `#define REF(alias) alias alias_var;`: These are two `AnnotatedLine` instances at first, namely `#define REF(alias)` with `AnnotatedLine::Level` set to 0 and `alias alias_var;` with `AnnotatedLine::Level` equals to 1. They get joined eventually. Since my patch sets `A.Level = B.Level;` in `join()`, the joined line gets level 1. But the `LevelIndentTracker` is still in level 0 (i.e. `IndentForLevel` contains only 1 element). It is kind of the same problem why I added the `if (!Line.InPPDirective)` check in `nextLine()` in the patch: I think, if both AnnotatedLines are PP directives, then `join()` should not step "into" or "out of" levels. PP-directives are kind-of "temporary insertions". So, replacing in `join()` the `A.Level = B.Level;` from my patch with assert(A.InPPDirective == B.InPPDirective); if (!A.InPPDirective && !B.InPPDirective) A.Level = B.Level; seems to fix the problem. At least all Google tests (including the ClangRenameTests) pass that didn't fail without the patch. Any opinions on that idea? I have to think some more about the levels and what it means regarding `join()`. I also need to get the clang-tidy test to run locally and also add the above example as a formatter test (if possible in a reduced form). Most likely I won't be able to do this until next week, sorry. :-/ To create a new fix, do I understand the guide <https://llvm.org/docs/Phabricator.html#using-patch-summaries> correctly that I basically execute `arc diff --verbatim` and mention `Depends on D151047` somewhere in the message? Will it then appear here automatically? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151047/new/ https://reviews.llvm.org/D151047 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
