[PATCH] D37813: clang-format: better handle namespace macros

This revision was automatically updated to reflect the committed changes. Closed by commit rL362740: clang-format: better handle namespace macros (authored by Typz, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST

[PATCH] D37813: clang-format: better handle namespace macros

Typz updated this revision to Diff 203425. Typz added a comment. Rebase Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37813/new/ https://reviews.llvm.org/D37813 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/Format.cpp

[PATCH] D37813: clang-format: better handle namespace macros

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37813/new/ https://reviews.llvm.org/D37813 ___ cfe-commits

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. ping? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37813/new/ https://reviews.llvm.org/D37813 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D37813: clang-format: better handle namespace macros

Typz marked 2 inline comments as done. Typz added inline comments. Comment at: unittests/Format/NamespaceEndCommentsFixerTest.cpp:526 EXPECT_EQ("namespace A {\n" " int i;\n" "} // namespace A", klimek wrote: > Typz wrote: > > Should

[PATCH] D37813: clang-format: better handle namespace macros

Typz updated this revision to Diff 201936. Typz marked an inline comment as done. Typz added a comment. - Rebase - Fix NamespaceEndCommentsFixerTest to match LLVM indent style Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37813/new/

[PATCH] D37813: clang-format: better handle namespace macros

klimek added inline comments. Comment at: unittests/Format/NamespaceEndCommentsFixerTest.cpp:526 EXPECT_EQ("namespace A {\n" " int i;\n" "} // namespace A", Typz wrote: > Should I also fix these tests? > > They already existed

[PATCH] D37813: clang-format: better handle namespace macros

Typz marked 2 inline comments as done. Typz added inline comments. Comment at: unittests/Format/NamespaceEndCommentsFixerTest.cpp:526 EXPECT_EQ("namespace A {\n" " int i;\n" "} // namespace A", Should I also fix these tests? They

[PATCH] D37813: clang-format: better handle namespace macros

klimek added inline comments. Comment at: unittests/Format/NamespaceEndCommentsFixerTest.cpp:582 + EXPECT_EQ("TESTSUITE() {\n" +" int i;\n" +"} // TESTSUITE()", All of the fixNamespaceEndComments tests are indented, but standard llvm

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. > Ah, wow, I see what confused me now: > In the fixNamespaceEndComment tests you use an indent that clang-format > would not produce. Can you please fix that, otherwise I find this super > confusing :) Can you point a specific exemple (in code) ? There are many tests

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In D37813#1512317 , @Typz wrote: > The patch goal is indeed to indent the content of namespace-macro blocks like > the content of any 'regular' namespace. So it should look like the content of > a namespace, possibly depending on

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. The patch goal is indeed to indent the content of namespace-macro blocks like the content of any 'regular' namespace. So it should look like the content of a namespace, possibly depending on the choose style options. To sumarize, here is a detailed summary of the

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In D37813#1254891 , @Typz wrote: > In D37813#1254865 , @klimek wrote: > > > In D37813#1253813 , @Typz wrote: > > > > > In D37813#1184051

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. Herald added a project: clang. ping ? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37813/new/ https://reviews.llvm.org/D37813 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. ping? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37813/new/ https://reviews.llvm.org/D37813 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. In https://reviews.llvm.org/D37813#1254865, @klimek wrote: > In https://reviews.llvm.org/D37813#1253813, @Typz wrote: > > > In https://reviews.llvm.org/D37813#1184051, @klimek wrote: > > > > > The problem here is that we have different opinions on how the formatting > > >

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In https://reviews.llvm.org/D37813#1253813, @Typz wrote: > In https://reviews.llvm.org/D37813#1184051, @klimek wrote: > > > The problem here is that we have different opinions on how the formatting > > on namespace macros should behave in the first place- I think they

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. In https://reviews.llvm.org/D37813#1184051, @klimek wrote: > The problem here is that we have different opinions on how the formatting on > namespace macros should behave in the first place- I think they should behave > like namespaces, you want them to be formatted

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. The problem here is that we have different opinions on how the formatting on namespace macros should behave in the first place- I think they should behave like namespaces, you want them to be formatted differently. Given that you want full control over the formatting of

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. Ping! This has been stale for a while now. Last comment indicate this is not the right approach, but some prototyping has been done a more generic approach, with no timeline. I understand this, but in the meantime the problem is still there, with no solution at the

[PATCH] D37813: clang-format: better handle namespace macros

Typz updated this revision to Diff 158309. Typz added a comment. Herald added a subscriber: acoomans. Regeneration documentation Repository: rC Clang https://reviews.llvm.org/D37813 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/Format.cpp

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. ping? Repository: rC Clang https://reviews.llvm.org/D37813 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37813: clang-format: better handle namespace macros

Typz updated this revision to Diff 147027. Typz added a comment. Rebase Repository: rC Clang https://reviews.llvm.org/D37813 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/FormatToken.h lib/Format/FormatTokenLexer.cpp lib/Format/NamespaceEndCommentsFixer.cpp

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. In https://reviews.llvm.org/D37813#958222, @klimek wrote: > Some initial design work has been done, and Krasimir said that he's > interested. No timeline though :( @klimek , @krasimir : any progress on this new preprocessor-like way to configure macros ? Repository:

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. > Some initial design work has been done, and Krasimir said that he's > interested. No timeline though :( any update or progress maybe? https://reviews.llvm.org/D37813 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D37813: clang-format: better handle namespace macros

Typz updated this revision to Diff 132838. Typz added a comment. rebase https://reviews.llvm.org/D37813 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/FormatToken.h lib/Format/FormatTokenLexer.cpp lib/Format/NamespaceEndCommentsFixer.cpp

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In https://reviews.llvm.org/D37813#958116, @Typz wrote: > OK. > > So you mean a solution like the one discussed earlier would be the way to go? > > > I mean that we can configure macros in the format style, like "define A(X) > > class X {". I'm not 100% sure whether we

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. OK. So you mean a solution like the one discussed earlier would be the way to go? > I mean that we can configure macros in the format style, like "define A(X) > class X {". I'm not 100% sure whether we would just try to use the > Preprocessor for this, or whether we'd

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. I've talked with Daniel and we both believe this patch is not the right way to go forward for clang-format. It is adding configuration mechanisms that we need to maintain going forward, without addressing a general problem; specifically for how to handle macros, I

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. ping? https://reviews.llvm.org/D37813 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. > Ok, that's probably where our different opinions come from - I would want a > macro to be formatted to reflect how it's implemented, because otherwise I'm > going to be surprised when I look at the implementation, and I consider > surprises to be something to avoid in

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In https://reviews.llvm.org/D37813#945125, @Typz wrote: > I don't think this is really relevant for this tool: if someone changes the > implementation of the macro, then *they* must indeed if it should not be > formatted like a namespace (and keep the clang-format

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. I don't think this is really relevant for this tool: if someone changes the implementation of the macro, then *they* must indeed if it should not be formatted like a namespace (and keep the clang-format configuration unchanged), or if it should now be formatted like a

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In https://reviews.llvm.org/D37813#942137, @Typz wrote: > As far as "parsing" and formatting inside the block is concerned, this is > indeed unrelated (and would totally work if all macros where specified with > some preprocessor definitions). > > But identifying the

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. As far as "parsing" and formatting inside the block is concerned, this is indeed unrelated (and would totally work if all macros where specified with some preprocessor definitions). But identifying the 'opening' token and generating the matching 'closing' comment are

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In https://reviews.llvm.org/D37813#941987, @Typz wrote: > Definitely that would be much more expressive. But this approach is also > incomplete: in case of namespace (and possibly others?), we also need to > perform the reverse operation, e.g. to "generate" a macro call

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. Definitely that would be much more expressive. But this approach is also incomplete: in case of namespace (and possibly others?), we also need to perform the reverse operation, e.g. to "generate" a macro call for rewriting the closing comment. On top of this, I think

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In https://reviews.llvm.org/D37813#930065, @Typz wrote: > ping? Argh, very sorry for the delay in response. In https://reviews.llvm.org/D37813#905257, @Typz wrote: > In https://reviews.llvm.org/D37813#876227, @klimek wrote: > > > I think instead of introducing more

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. ping? https://reviews.llvm.org/D37813 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. ping? https://reviews.llvm.org/D37813 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37813: clang-format: better handle namespace macros

Typz updated this revision to Diff 120085. Typz added a comment. rebase https://reviews.llvm.org/D37813 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/FormatToken.h lib/Format/FormatTokenLexer.cpp lib/Format/NamespaceEndCommentsFixer.cpp

[PATCH] D37813: clang-format: better handle namespace macros

Typz added a comment. In https://reviews.llvm.org/D37813#876227, @klimek wrote: > I think instead of introducing more and more special cases of macros we might > want to handle, we should instead allow specifying macro productions globally. what do you mean? Do you mean to group all the macro

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. I think instead of introducing more and more special cases of macros we might want to handle, we should instead allow specifying macro productions globally. https://reviews.llvm.org/D37813 ___ cfe-commits mailing list

[PATCH] D37813: clang-format: better handle namespace macros

Typz updated this revision to Diff 115802. Typz added a comment. rebase https://reviews.llvm.org/D37813 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/FormatToken.h lib/Format/FormatTokenLexer.cpp lib/Format/NamespaceEndCommentsFixer.cpp

[PATCH] D37813: clang-format: better handle namespace macros

Typz created this revision. Other macros are used to declare namespaces, and should thus be handled similarly. This is the case for crpcut's TESTSUITE macro, or for unittest-cpp's SUITE macro: TESTSUITE(Foo) { TEST(MyFirstTest) { assert(0); } } // TESTSUITE(Foo) This patch deals