[PATCH] D121757: [clang-format] Take out common code for parsing blocks NFC

This revision was automatically updated to reflect the committed changes. Closed by commit rG43c146c96d8e: [clang-format] Take out common code for parsing blocks NFC (authored by sstwcw). Changed prior to commit: https://reviews.llvm.org/D121757?vs=420653&id=426277#toc Repository: rG LLVM Gi

[PATCH] D121757: [clang-format] Take out common code for parsing blocks NFC

sstwcw added a comment. Would you have a look at the parent revision namely D121756 ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121757/new/ https://reviews.llvm.org/D121757 ___

[PATCH] D121757: [clang-format] Take out common code for parsing blocks NFC

owenpan accepted this revision. owenpan added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121757/new/ https://reviews.llvm.org/D121757 _

[PATCH] D121757: [clang-format] Take out common code for parsing blocks NFC

sstwcw updated this revision to Diff 420653. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121757/new/ https://reviews.llvm.org/D121757 Files: clang/lib/Format/UnwrappedLineParser.cpp clang/lib/Format/UnwrappedLineParser.h Index: clang/lib/For

[PATCH] D121757: [clang-format] Take out common code for parsing blocks NFC

sstwcw updated this revision to Diff 420652. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121757/new/ https://reviews.llvm.org/D121757 Files: clang/lib/Format/FormatToken.h clang/lib/Format/TokenAnnotator.cpp clang/lib/Format/UnwrappedLinePar

[PATCH] D121757: [clang-format] Take out common code for parsing blocks NFC

owenpan added a comment. In D121757#3415402 , @sstwcw wrote: > I ran check-clang after formatting the entire code base with the new version. > It turned out it did break some tests. It seems to be because it messed up > these comments. > > diff --gi

[PATCH] D121757: [clang-format] Take out common code for parsing blocks NFC

sstwcw added a comment. I ran check-clang after formatting the entire code base with the new version. It turned out it did break some tests. It seems to be because it messed up these comments. diff --git a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.fallthrough/p1.cpp b/clang/test/CXX/dcl.dcl

[PATCH] D121757: [clang-format] Take out common code for parsing blocks NFC

sstwcw updated this revision to Diff 419009. sstwcw retitled this revision from "[clang-format] Take out common code for parsing blocks" to "[clang-format] Take out common code for parsing blocks NFC". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

owenpan added a comment. In D121757#3407539 , @owenpan wrote: > In D121757#3406814 , @sstwcw wrote: > >> I tried formatting the files in `clang-formatted-files.txt`. Besides the >> files in the list that get chan

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

owenpan added a comment. In D121757#3406814 , @sstwcw wrote: > I tried formatting the files in `clang-formatted-files.txt`. Besides the > files in the list that get changed when formatted with the program built from > `main`, none gets changed when I fo

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2707-2708 + if (FormatTok->is(tok::l_brace)) { +CompoundStatementIndenter Indenter(this, Style, Line->Level); +FormatToken *LeftBrace = FormatTok; +parseBlock(); You

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

sstwcw marked 9 inline comments as done. sstwcw added a comment. I tried formatting the files in `clang-formatted-files.txt`. Besides the files in the list that get changed when formatted with the program built from `main`, none gets changed when I format them with the program built from this pa

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

sstwcw updated this revision to Diff 418061. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121757/new/ https://reviews.llvm.org/D121757 Files: clang/lib/Format/UnwrappedLineParser.cpp clang/lib/Format/UnwrappedLineParser.h Index: clang/lib/For

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

owenpan added a comment. In D121757#3389092 , @sstwcw wrote: > This patch is only intended to reduce the number of times the functionality > gets implemented separately. Any change in behavior would be unintended. > And we also use the `parseIndentedB

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

MyDeveloperDay added a comment. In D121757#3389218 , @sstwcw wrote: > For the new stuff I have the option of still adding the function > `parseIndentedBlock` but only using it in new code. Please be more blunt > about whether I should close this revisi

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

sstwcw added a comment. For the new stuff I have the option of still adding the function `parseIndentedBlock` but only using it in new code. Please be more blunt about whether I should close this revision and do it that way. I guess I might have misunderstood you before from how you reacted w

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

MyDeveloperDay added a comment. Just an observation, I have this in my Day Job all the time.. 1. Developer A develops a piece of code 2. Over the years the developers in our group learn that code and become familiar with it 3. Developer B arrives in our group, telling us we are doing it all wron

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

sstwcw marked an inline comment as done. sstwcw added a comment. This patch is only intended to reduce the number of times the functionality gets implemented separately. Any change in behavior would be unintended. And we also use the `parseIndentedBlock` in Verilog stuff, so it's not just two

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

owenpan requested changes to this revision. owenpan added a comment. This revision now requires changes to proceed. Because this patch would impact inserting/removing braces, we must test it against a large codebase. Before I landed `RemoveBracesLLVM` and `InsertBraces`, I had tested them with `

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

sstwcw marked an inline comment as done. sstwcw added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2851 -addUnwrappedLine(); -++Line->Level; -parseStructuralElement(); HazardyKnusperkeks wrote: > This is completely missing. Di

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

MyDeveloperDay added a comment. Can you add a summary as to why you are making this change? the code doesn't look the same before/after so whats changing and why? is there no unit tests that perhaps need to be added? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

HazardyKnusperkeks added a comment. Looks basically okay. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2851 -addUnwrappedLine(); -++Line->Level; -parseStructuralElement(); This is completely missing. Didn't it affect anything?

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

sstwcw created this revision. sstwcw added a reviewer: clang-format. sstwcw added a project: clang-format. Herald added a project: All. sstwcw requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://re