[PATCH] D152623: [clang-format] Indent Verilog struct literal on new line
This revision was automatically updated to reflect the committed changes. Closed by commit rG6bf66d839f13: [clang-format] Indent Verilog struct literal on new line (authored by sstwcw). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152623/new/ https://reviews.llvm.org/D152623 Files: clang/lib/Format/ContinuationIndenter.cpp clang/unittests/Format/FormatTestVerilog.cpp Index: clang/unittests/Format/FormatTestVerilog.cpp === --- clang/unittests/Format/FormatTestVerilog.cpp +++ clang/unittests/Format/FormatTestVerilog.cpp @@ -1172,6 +1172,15 @@ verifyFormat("c = '{a : 0, b : 0.0, default : 0};", Style); verifyFormat("c = ab'{a : 0, b : 0.0};", Style); verifyFormat("c = ab'{cd : cd'{1, 1.0}, ef : ef'{2, 2.0}};", Style); + + // It should be indented correctly when the line has to break. + verifyFormat("c = //\n" + "'{default: 0};"); + Style = getDefaultStyle(); + Style.ContinuationIndentWidth = 2; + verifyFormat("c = //\n" + " '{default: 0};", + Style); } TEST_F(FormatTestVerilog, StructuredProcedure) { Index: clang/lib/Format/ContinuationIndenter.cpp === --- clang/lib/Format/ContinuationIndenter.cpp +++ clang/lib/Format/ContinuationIndenter.cpp @@ -1173,7 +1173,15 @@ } if (NextNonComment->is(TT_TemplateString) && NextNonComment->closesScope()) return State.Stack[State.Stack.size() - 2].LastSpace; + // Field labels in a nested type should be aligned to the brace. For example + // in ProtoBuf: + // optional int32 b = 2 [(foo_options) = {aaa: 123, + // :"baz"}]; + // For Verilog, a quote following a brace is treated as an identifier. And + // Both braces and colons get annotated as TT_DictLiteral. So we have to + // check. if (Current.is(tok::identifier) && Current.Next && + (!Style.isVerilog() || Current.Next->is(tok::colon)) && (Current.Next->is(TT_DictLiteral) || ((Style.Language == FormatStyle::LK_Proto || Style.Language == FormatStyle::LK_TextProto) && Index: clang/unittests/Format/FormatTestVerilog.cpp === --- clang/unittests/Format/FormatTestVerilog.cpp +++ clang/unittests/Format/FormatTestVerilog.cpp @@ -1172,6 +1172,15 @@ verifyFormat("c = '{a : 0, b : 0.0, default : 0};", Style); verifyFormat("c = ab'{a : 0, b : 0.0};", Style); verifyFormat("c = ab'{cd : cd'{1, 1.0}, ef : ef'{2, 2.0}};", Style); + + // It should be indented correctly when the line has to break. + verifyFormat("c = //\n" + "'{default: 0};"); + Style = getDefaultStyle(); + Style.ContinuationIndentWidth = 2; + verifyFormat("c = //\n" + " '{default: 0};", + Style); } TEST_F(FormatTestVerilog, StructuredProcedure) { Index: clang/lib/Format/ContinuationIndenter.cpp === --- clang/lib/Format/ContinuationIndenter.cpp +++ clang/lib/Format/ContinuationIndenter.cpp @@ -1173,7 +1173,15 @@ } if (NextNonComment->is(TT_TemplateString) && NextNonComment->closesScope()) return State.Stack[State.Stack.size() - 2].LastSpace; + // Field labels in a nested type should be aligned to the brace. For example + // in ProtoBuf: + // optional int32 b = 2 [(foo_options) = {aaa: 123, + // :"baz"}]; + // For Verilog, a quote following a brace is treated as an identifier. And + // Both braces and colons get annotated as TT_DictLiteral. So we have to + // check. if (Current.is(tok::identifier) && Current.Next && + (!Style.isVerilog() || Current.Next->is(tok::colon)) && (Current.Next->is(TT_DictLiteral) || ((Style.Language == FormatStyle::LK_Proto || Style.Language == FormatStyle::LK_TextProto) && ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D152623: [clang-format] Indent Verilog struct literal on new line
sstwcw updated this revision to Diff 533617. sstwcw added a comment. - limit change to Verilog Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152623/new/ https://reviews.llvm.org/D152623 Files: clang/lib/Format/ContinuationIndenter.cpp clang/unittests/Format/FormatTestVerilog.cpp Index: clang/unittests/Format/FormatTestVerilog.cpp === --- clang/unittests/Format/FormatTestVerilog.cpp +++ clang/unittests/Format/FormatTestVerilog.cpp @@ -1172,6 +1172,15 @@ verifyFormat("c = '{a : 0, b : 0.0, default : 0};", Style); verifyFormat("c = ab'{a : 0, b : 0.0};", Style); verifyFormat("c = ab'{cd : cd'{1, 1.0}, ef : ef'{2, 2.0}};", Style); + + // It should be indented correctly when the line has to break. + verifyFormat("c = //\n" + "'{default: 0};"); + Style = getDefaultStyle(); + Style.ContinuationIndentWidth = 2; + verifyFormat("c = //\n" + " '{default: 0};", + Style); } TEST_F(FormatTestVerilog, StructuredProcedure) { Index: clang/lib/Format/ContinuationIndenter.cpp === --- clang/lib/Format/ContinuationIndenter.cpp +++ clang/lib/Format/ContinuationIndenter.cpp @@ -1172,7 +1172,15 @@ } if (NextNonComment->is(TT_TemplateString) && NextNonComment->closesScope()) return State.Stack[State.Stack.size() - 2].LastSpace; + // Field labels in a nested type should be aligned to the brace. For example + // in ProtoBuf: + // optional int32 b = 2 [(foo_options) = {aaa: 123, + // :"baz"}]; + // For Verilog, a quote following a brace is treated as an identifier. And + // Both braces and colons get annotated as TT_DictLiteral. So we have to + // check. if (Current.is(tok::identifier) && Current.Next && + (!Style.isVerilog() || Current.Next->is(tok::colon)) && (Current.Next->is(TT_DictLiteral) || ((Style.Language == FormatStyle::LK_Proto || Style.Language == FormatStyle::LK_TextProto) && Index: clang/unittests/Format/FormatTestVerilog.cpp === --- clang/unittests/Format/FormatTestVerilog.cpp +++ clang/unittests/Format/FormatTestVerilog.cpp @@ -1172,6 +1172,15 @@ verifyFormat("c = '{a : 0, b : 0.0, default : 0};", Style); verifyFormat("c = ab'{a : 0, b : 0.0};", Style); verifyFormat("c = ab'{cd : cd'{1, 1.0}, ef : ef'{2, 2.0}};", Style); + + // It should be indented correctly when the line has to break. + verifyFormat("c = //\n" + "'{default: 0};"); + Style = getDefaultStyle(); + Style.ContinuationIndentWidth = 2; + verifyFormat("c = //\n" + " '{default: 0};", + Style); } TEST_F(FormatTestVerilog, StructuredProcedure) { Index: clang/lib/Format/ContinuationIndenter.cpp === --- clang/lib/Format/ContinuationIndenter.cpp +++ clang/lib/Format/ContinuationIndenter.cpp @@ -1172,7 +1172,15 @@ } if (NextNonComment->is(TT_TemplateString) && NextNonComment->closesScope()) return State.Stack[State.Stack.size() - 2].LastSpace; + // Field labels in a nested type should be aligned to the brace. For example + // in ProtoBuf: + // optional int32 b = 2 [(foo_options) = {aaa: 123, + // :"baz"}]; + // For Verilog, a quote following a brace is treated as an identifier. And + // Both braces and colons get annotated as TT_DictLiteral. So we have to + // check. if (Current.is(tok::identifier) && Current.Next && + (!Style.isVerilog() || Current.Next->is(tok::colon)) && (Current.Next->is(TT_DictLiteral) || ((Style.Language == FormatStyle::LK_Proto || Style.Language == FormatStyle::LK_TextProto) && ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D152623: [clang-format] Indent Verilog struct literal on new line
sstwcw added a comment. > So I assume your `'` is a 'DictLiteral`? The brace following the quote is a `DictLiteral`. The quote is a `tok::identifier` and `TT_Unknown`. > Does it have to be one? The brace is set to this type when used this way in all other languages. I don't want to make an exception. > I don't know what else maybe a `DictLiteral` in what language and am not so > comfortable with this change. I changed the patch to use the new behavior only for Verilog. The `DictLiteral` type gets set on the braces and colons in literals where the field names and values are separated by colons in all languages that have them. In addition, because the protocol buffer text format allows angular brackets as braces, the type is also set on those angular brackets for that language. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152623/new/ https://reviews.llvm.org/D152623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D152623: [clang-format] Indent Verilog struct literal on new line
HazardyKnusperkeks added a comment. So I assume your `'` is a 'DictLiteral`? Does it have to be one? I don't know what else maybe a `DictLiteral` in what language and am not so comfortable with this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152623/new/ https://reviews.llvm.org/D152623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D152623: [clang-format] Indent Verilog struct literal on new line
sstwcw created this revision. Herald added projects: All, clang, clang-format. Herald added a subscriber: cfe-commits. Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay. sstwcw requested review of this revision. Before: c = // '{default: 0}; After: c = // '{default: 0}; If the line has to be broken, the continuation part should be indented. Before this fix, it was not the case if the continuation part was a struct literal. The rule that caused the problem was added in 783bac6b. It was intended for aligning the field labels in ProtoBuf. The type `TT_DictLiteral` was only for colons back then, so the program didn't have to check whether the token was a colon when it was already type `TT_DictLiteral`. Now the type applies to more things including the braces enclosing a dictionary literal. In Verilog, struct literals start with a quote. The quote is regarded as an identifier by the program. So the rule for aligning the fields in ProtoBuf applied to this situation by mistake. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D152623 Files: clang/lib/Format/ContinuationIndenter.cpp clang/unittests/Format/FormatTestVerilog.cpp Index: clang/unittests/Format/FormatTestVerilog.cpp === --- clang/unittests/Format/FormatTestVerilog.cpp +++ clang/unittests/Format/FormatTestVerilog.cpp @@ -1172,6 +1172,15 @@ verifyFormat("c = '{a : 0, b : 0.0, default : 0};", Style); verifyFormat("c = ab'{a : 0, b : 0.0};", Style); verifyFormat("c = ab'{cd : cd'{1, 1.0}, ef : ef'{2, 2.0}};", Style); + + // It should be indented correctly when the line has to break. + verifyFormat("c = //\n" + "'{default: 0};"); + Style = getDefaultStyle(); + Style.ContinuationIndentWidth = 2; + verifyFormat("c = //\n" + " '{default: 0};", + Style); } TEST_F(FormatTestVerilog, StructuredProcedure) { Index: clang/lib/Format/ContinuationIndenter.cpp === --- clang/lib/Format/ContinuationIndenter.cpp +++ clang/lib/Format/ContinuationIndenter.cpp @@ -1172,8 +1172,12 @@ } if (NextNonComment->is(TT_TemplateString) && NextNonComment->closesScope()) return State.Stack[State.Stack.size() - 2].LastSpace; + // Field labels in a nested type should be aligned to the brace. For example + // in ProtoBuf: + // optional int32 b = 2 [(foo_options) = {aaa: 123, + // :"baz"}]; if (Current.is(tok::identifier) && Current.Next && - (Current.Next->is(TT_DictLiteral) || + ((Current.Next->is(tok::colon) && Current.Next->is(TT_DictLiteral)) || ((Style.Language == FormatStyle::LK_Proto || Style.Language == FormatStyle::LK_TextProto) && Current.Next->isOneOf(tok::less, tok::l_brace { Index: clang/unittests/Format/FormatTestVerilog.cpp === --- clang/unittests/Format/FormatTestVerilog.cpp +++ clang/unittests/Format/FormatTestVerilog.cpp @@ -1172,6 +1172,15 @@ verifyFormat("c = '{a : 0, b : 0.0, default : 0};", Style); verifyFormat("c = ab'{a : 0, b : 0.0};", Style); verifyFormat("c = ab'{cd : cd'{1, 1.0}, ef : ef'{2, 2.0}};", Style); + + // It should be indented correctly when the line has to break. + verifyFormat("c = //\n" + "'{default: 0};"); + Style = getDefaultStyle(); + Style.ContinuationIndentWidth = 2; + verifyFormat("c = //\n" + " '{default: 0};", + Style); } TEST_F(FormatTestVerilog, StructuredProcedure) { Index: clang/lib/Format/ContinuationIndenter.cpp === --- clang/lib/Format/ContinuationIndenter.cpp +++ clang/lib/Format/ContinuationIndenter.cpp @@ -1172,8 +1172,12 @@ } if (NextNonComment->is(TT_TemplateString) && NextNonComment->closesScope()) return State.Stack[State.Stack.size() - 2].LastSpace; + // Field labels in a nested type should be aligned to the brace. For example + // in ProtoBuf: + // optional int32 b = 2 [(foo_options) = {aaa: 123, + // :"baz"}]; if (Current.is(tok::identifier) && Current.Next && - (Current.Next->is(TT_DictLiteral) || + ((Current.Next->is(tok::colon) && Current.Next->is(TT_DictLiteral)) || ((Style.Language == FormatStyle::LK_Proto || Style.Language == FormatStyle::LK_TextProto) && Current.Next->isOneOf(tok::less, tok::l_brace { ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits