[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-19 Thread Owen Pan via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG92bccf5d3d21: [clang-format] Don't use PPIndentWidth inside multi-line macros (authored by goldstein.w.n, committed by owenpan). Repository: rG LL

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-19 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3938942 , @owenpan wrote: > In D137181#3938913 , @goldstein.w.n > wrote: > >> Anything left for me todo (can't imagine I have push access). > > See here

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-19 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n updated this revision to Diff 476732. goldstein.w.n added a comment. 1. Updating D137181 : [clang-format] Don't use 'PPIndentWidth' inside multi-line macros # 2. Enter a brief description of the changes included in this update. 3. The first line i

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-18 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. In D137181#3938913 , @goldstein.w.n wrote: > Anything left for me todo (can't imagine I have push access). See here . We will need your name and email. Repository:

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-18 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n marked an inline comment as done. goldstein.w.n added a comment. In D137181#3938904 , @owenpan wrote: > LGTM Awesome! Thanks for sticking with it through all these revisions :) Anything left for me todo (can't imagine I have push access).

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-18 Thread Owen Pan via Phabricator via cfe-commits
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/D137181/new/ https://reviews.llvm.org/D137181 _

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-18 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n marked 2 inline comments as done. goldstein.w.n added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:838 + // If this changes PPLevel needs to be used for get correct indentation. + assert(!Line.InMacroBody && !InPPDirective); return Line

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-18 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n marked an inline comment as done. goldstein.w.n added a comment. In D137181#3938771 , @owenpan wrote: > In D137181#3935951 , @owenpan wrote: > >> In D137181#3935856

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-18 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n updated this revision to Diff 476654. goldstein.w.n added a comment. 1. Updating D137181 : [clang-format] Don't use 'PPIndentWidth' inside multi-line macros # 2. Enter a brief description of the changes included in this update. 3. The first line i

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-18 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. In D137181#3935951 , @owenpan wrote: > In D137181#3935856 , @goldstein.w.n > wrote: > >> I could remove either the `PPDIS_BeforeHash` or `PPDIS_AfterHash` though as >> the they really no

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-18 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3935951 , @owenpan wrote: > In D137181#3935856 , @goldstein.w.n > wrote: > >> In D137181#3935752 , @owenpan >> wrote: >> >>> Pl

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-18 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n updated this revision to Diff 476528. goldstein.w.n added a comment. 1. Updating D137181 : [clang-format] Don't use 'PPIndentWidth' inside multi-line macros # 2. Enter a brief description of the changes included in this update. 3. The first line i

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-17 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. In D137181#3935856 , @goldstein.w.n wrote: > In D137181#3935752 , @owenpan wrote: > >> Please mark review comments as done if you have addressed them. Can you also >> clean up the test c

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-17 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3935752 , @owenpan wrote: > Please mark review comments as done if you have addressed them. Can you also > clean up the test cases, removing overlapping/redundant ones, making sure a > test case doesn't end with

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-17 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:69-75 + if (Line.InMacroBody) { +Indent = (Line.PPLevel + 1) * PPIndentWidth; +Indent += (Line.Level - Line.PPLevel - 1) * Style.IndentWidth; +Indent += Style

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-17 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n updated this revision to Diff 476337. goldstein.w.n added a comment. 1. Updating D137181 : [clang-format] Don't use 'PPIndentWidth' inside multi-line macros # 2. Enter a brief description of the changes included in this update. 3. The first line i

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-17 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:838 + // If this changes PPLevel needs to be used for get correct indentation. + assert(!(Line.InMacroBody && InPPDirective)); return Line.Level * Style.IndentWidth + Length <= ColumnLimit; -

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-17 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. Please mark review comments as done if you have addressed them. Can you also clean up the test cases, removing overlapping/redundant ones, making sure a test case doesn't end with a newline (e.g., line 5380), etc? Comment at: clang/lib/Format/Unwrappe

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-16 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3926848 , @owenpan wrote: > In D137181#3924002 , @sstwcw wrote: > >> Can you make `TokenAnnotator::printDebugInfo` print `PPLevel`? > > @goldstein.w.n can you add it as fo

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-16 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n updated this revision to Diff 476013. goldstein.w.n marked 2 inline comments as done. goldstein.w.n added a comment. 1. Updating D137181 : [clang-format] Don't use 'PPIndentWidth' inside multi-line macros # 2. Enter a brief description of the chan

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-15 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. In D137181#3924002 , @sstwcw wrote: > Can you make `TokenAnnotator::printDebugInfo` print `PPLevel`? @goldstein.w.n can you add it as follows? $ git diff TokenAnnotator.cpp diff --git a/clang/lib/Format/TokenAnnotator.cpp b

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-13 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n marked 2 inline comments as done. goldstein.w.n added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:69 (Style.PPIndentWidth >= 0) ? Style.PPIndentWidth : Style.IndentWidth; - Indent = Line.Level * IndentWidth + AdditionalIn

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-13 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3923921 , @owenpan wrote: > I think we are close to the finishing line. Can you revisit the change to the > formatter and clean it up? For example, casting `PPLevel` to `unsigned` is > redundant now. IMO you can

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-13 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n updated this revision to Diff 475050. goldstein.w.n added a comment. 1. Updating D137181 : [clang-format] Don't use 'PPIndentWidth' inside multi-line macros # 2. Enter a brief description of the changes included in this update. 3. The first line i

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-13 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. I think we are close to the finishing line. Can you revisit the change to the formatter and clean it up? For example, casting `PPLevel` to `unsigned` is redundant now. IMO you can simply the change too. Comment at: clang/lib/Format/UnwrappedLineFormat

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-13 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3923434 , @owenpan wrote: > In D137181#3918715 , @goldstein.w.n > wrote: > >> In D137181#3918673 , @owenpan >> wrote: >> >>> Be

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-12 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. In D137181#3918715 , @goldstein.w.n wrote: > In D137181#3918673 , @owenpan wrote: > >> Below is how I defined `PPLevel`: >> >> $ git diff UnwrappedLineParser.h >> diff --git a/clang/l

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-09 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3918673 , @owenpan wrote: > In D137181#3918117 , @goldstein.w.n > wrote: > maybe you did something different? >>> >>> Here is what I did: >>> >>> $ git diff Unwrap

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-09 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n updated this revision to Diff 474447. goldstein.w.n added a comment. 1. Updating D137181 : [clang-format] Don't use 'PPIndentWidth' inside multi-line macros # 2. Enter a brief description of the changes included in this update. 3. The first line i

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-09 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. In D137181#3918117 , @goldstein.w.n wrote: >>> maybe you did something different? >> >> Here is what I did: >> >> $ git diff UnwrappedLineParser.cpp >> diff --git a/clang/lib/Format/UnwrappedLineParser.cpp >> b/clang/lib/For

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-09 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3916605 , @owenpan wrote: > In D137181#3916558 , @goldstein.w.n > wrote: > >> In D137181#3916547 , @owenpan >> wrote: >> >>> Ye

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-09 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n updated this revision to Diff 474359. goldstein.w.n added a comment. 1. Updating D137181 : [clang-format] Don't use 'PPIndentWidth' inside multi-line macros # 2. Enter a brief description of the changes included in this update. 3. The first line i

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-08 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. In D137181#3916558 , @goldstein.w.n wrote: > In D137181#3916547 , @owenpan wrote: > >> Yes, if there is a header guard. Otherwise, set `PPLevel` to `PPBranchLevel >> + 1`. > > That fails

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-08 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.h:66 + /// #endifPPLevel still at : 0 + int PPLevel; + goldstein.w.n wrote: > owenpan wrote: > > You need to initialize `PPLevel` in the constructor. FWIW I'd use >

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-08 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3916547 , @owenpan wrote: > In D137181#3916521 , @goldstein.w.n > wrote: > >> I think we can remove the places I set `InitialPPLevel` and `OldPPLevel` but >> anything el

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-08 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:112 : Line(Line), TokenSource(TokenSource), ResetToken(ResetToken), -PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource), -Token(nullptr), PreviousToken(nullptr)

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-08 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. In D137181#3916521 , @goldstein.w.n wrote: > I think we can remove the places I set `InitialPPLevel` and `OldPPLevel` but > anything else I remove causes at least one test to fail. > > What did you do? Just set `PPLevel = PPBran

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-08 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3916505 , @owenpan wrote: > In D137181#3916488 , @goldstein.w.n > wrote: > >> In D137181#3916474 , @owenpan >> wrote: >> >>> Yo

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-08 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. In D137181#3916488 , @goldstein.w.n wrote: > In D137181#3916474 , @owenpan wrote: > >> You can still simply the changes to UnwrappedLineParser.cpp a lot. In fact, >> instead of adjusting

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-08 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3916474 , @owenpan wrote: > You can still simply the changes to UnwrappedLineParser.cpp a lot. In fact, > instead of adjusting `PPLevel` at various places, you only need to set it > once when `InMacroBody` is se

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-08 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:92-94 + if (Line.Level >= IndentForLevel.size()) +IndentForLevel.resize(Line.Level + 1, UnknownIndent ? -1 : Indent); +} owenpan wrote: > Please run git-clang-format

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-08 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. You can still simply the changes to UnwrappedLineParser.cpp a lot. In fact, instead of adjusting `PPLevel` at various places, you only need to set it once when `InMacroBody` is set. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:92-94 + if (

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-08 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.h:47 + /// The preprocessor indent level of the \c UnwrappedLine. + int PPLevel; sstwcw wrote: > Please elaborate what preprocessor indent level means. Done. Repository:

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-08 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n updated this revision to Diff 474079. goldstein.w.n added a comment. 1. Updating D137181 : [clang-format] Don't use 'PPIndentWidth' inside multi-line macros # 2. Enter a brief description of the changes included in this update. 3. The first line i

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-08 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3914292 , @owenpan wrote: > In D137181#3914100 , @goldstein.w.n > wrote: > >> Err I'm not sure that's right. I thought we where going for the C-code >> should start at t

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-08 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n updated this revision to Diff 473917. goldstein.w.n added a comment. 1. Updating D137181 : [clang-format] Don't use 'PPIndentWidth' inside multi-line macros # 2. Enter a brief description of the changes included in this update. 3. The first line i

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-08 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3914245 , @owenpan wrote: > In D137181#3914083 , @goldstein.w.n > wrote: > >> In D137181#3914066 , @owenpan >> wrote: >> >>> @g

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-08 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n updated this revision to Diff 473906. goldstein.w.n added a comment. 1. Updating D137181 : [clang-format] Don't use 'PPIndentWidth' inside multi-line macros # 2. Enter a brief description of the changes included in this update. 3. The first line i

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-08 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. In D137181#3914100 , @goldstein.w.n wrote: > Err I'm not sure that's right. I thought we where going for the C-code should > start at the column that the 'd' in define is. > So it would be: > > #define X \ >switch (x

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-08 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. In D137181#3914083 , @goldstein.w.n wrote: > In D137181#3914066 , @owenpan wrote: > >> @goldstein.w.n do you need to modify MacroCallReconstructor.cpp, Macros.h, >> and MacroCallReconstr

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-07 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3911602 , @owenpan wrote: > In D137181#3911005 , @sstwcw wrote: > >> Here is one problem: >> >> clang-format -style='{IndentPPDirectives: BeforeHash, PPIndentWidth: 1,

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-07 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n updated this revision to Diff 473875. goldstein.w.n added a comment. 1. Updating D137181 : [clang-format] Don't use 'PPIndentWidth' inside multi-line macros # 2. Enter a brief description of the changes included in this update. 3. The first line i

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-07 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3914066 , @owenpan wrote: > @goldstein.w.n do you need to modify MacroCallReconstructor.cpp, Macros.h, > and MacroCallReconstructorTest.cpp? Leaving them out wouldn't break any > existing tests. > > Adding `PPBr

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-07 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3911602 , @owenpan wrote: > In D137181#3911005 , @sstwcw wrote: > >> Here is one problem: >> >> clang-format -style='{IndentPPDirectives: BeforeHash, PPIndentWidth: 1,

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-07 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. @goldstein.w.n do you need to modify MacroCallReconstructor.cpp, Macros.h, and MacroCallReconstructorTest.cpp? Leaving them out wouldn't break any existing tests. Adding `PPBranchLevel` (or `PPLevel` in your case) to `UnwrappedLine` and `AnnotatedLine` worked for me. I

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-07 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. In D137181#3911005 , @sstwcw wrote: > Here is one problem: > > clang-format -style='{IndentPPDirectives: BeforeHash, PPIndentWidth: 1, > IndentWidth: 4, IndentCaseLabels: true}' > > actual: > #define X

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-07 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. In D137181#3910800 , @goldstein.w.n wrote: > In D137181#3910799 , @owenpan wrote: > >> IMO we should find a simpler way to indent bodies of macro definitions that >> are nested more than

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-06 Thread sstwcw via Phabricator via cfe-commits
sstwcw added a comment. Here is one problem: clang-format -style='{IndentPPDirectives: BeforeHash, PPIndentWidth: 1, IndentWidth: 4, IndentCaseLabels: true}' actual: #define X \ switch (x) {

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-06 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3910799 , @owenpan wrote: > IMO we should find a simpler way to indent bodies of macro definitions that > are nested more than one level deep. I prefer we handle that in another patch > and only support the exam

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-06 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. IMO we should find a simpler way to indent bodies of macro definitions that are nested more than one level deep. I prefer we handle that in another patch and only support the examples in the summary for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-06 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3910566 , @owenpan wrote: > In D137181#3910404 , @goldstein.w.n > wrote: > >> Doesn't that add an arbitrary +1 to the begining of the indentation? >> Shouldn't it be: >>

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-06 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n updated this revision to Diff 473489. goldstein.w.n added a comment. 1. Updating D137181 : [clang-format] Don't use 'PPIndentWidth' inside multi-line macros # 2. Enter a brief description of the changes included in this update. 3. The first line i

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-06 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. In D137181#3910404 , @goldstein.w.n wrote: > Doesn't that add an arbitrary +1 to the begining of the indentation? > Shouldn't it be: > > // IndentPPDirectives: AfterHash > #ifdef foo > # define bar() \\ > if (A) { \

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-05 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3904383 , @owenpan wrote: > With `ColumnLimit: 16` and `IndentPPDirectives: BeforeHash`, the format > should be: > > #ifdef foo >#define bar() \ >if (A) { \ >B(); \ >}

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-03 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:5056-5059 + "if (A) { \\\n" + "B(); \\\n" + "}\\\n" + "C();\n" goldstein.w.n

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-03 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:5056-5059 + "if (A) { \\\n" + "B(); \\\n" + "}\\\n" + "C();\n" owenpan wrote

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-02 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:5056-5059 + "if (A) { \\\n" + "B(); \\\n" + "}\\\n" + "C();\n" goldstein.w.n wrote

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-02 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. With `ColumnLimit: 16` and `IndentPPDirectives: BeforeHash`, the format should be: #ifdef foo #define bar() \ if (A) { \ B(); \ } \ C(); #endif With `IndentPPDirectives: AfterHash`, it should look like: #ifdef foo #

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-02 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:5056-5059 + "if (A) { \\\n" + "B(); \\\n" + "}\\\n" + "C();\n" owenpan wrote

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-02 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:5056-5059 + "if (A) { \\\n" + "B(); \\\n" + "}\\\n" + "C();\n" I just noticed that

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-02 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n marked an inline comment as done. goldstein.w.n added a comment. In D137181#3899874 , @MyDeveloperDay wrote: > This needs a unit test in clang/unittest/Format Fixed in V2 I think but not sure as the summary doesn't seem to have changed.

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-02 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n updated this revision to Diff 472759. goldstein.w.n added a comment. 1. Updating D137181 : [clang-format] Don't use 'PPIndentWidth' inside multi-line macros # 2. Enter a brief description of the changes included in this update. 3. The first line i

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-02 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:5047 + style.IndentPPDirectives = FormatStyle::PPDIS_None; + verifyFormat("#ifdef foo\n" + "#define bar() \\\n" goldstein.w.n wrote: > owenpan wrote: > > Do you need

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-02 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added a comment. In D137181#3903161 , @owenpan wrote: > Can you fix the format of the summary? I see the misindentation of the After/Before hash examples. Will fix those for V2. Anything else? Repository: rG LLVM Github Monorepo CHANG

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-02 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:5024 style); + verifyFormat("#if 1\n" owenpan wrote: > Don't add an empty line here. Will drop for V2. Comment at: clang/unittests/Format/

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-02 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. Can you fix the format of the summary? Comment at: clang/unittests/Format/FormatTest.cpp:5024 style); + verifyFormat("#if 1\n" Don't add an empty line here. Comment at: clang/unittests/Format/Format

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-01 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n updated this revision to Diff 472397. goldstein.w.n added a comment. 1. Updating D137181 : [clang-format] Don't use 'PPIndentWidth' inside multi-line macros # 2. Enter a brief description of the changes included in this update. 3. The first line i

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This needs a unit test in clang/unittest/Format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137181/new/ https://reviews.llvm.org/D137181 ___ cfe-commits mailing list cfe

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-01 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n created this revision. Herald added a project: All. goldstein.w.n requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. I.e with: PPIndentWidth = 1 IndentWidth = 4 ``` #ifdef foo #define bar() if (A) { B(); } C(); #end