<div dir="ltr"><br><div class="gmail_extra"><div class="gmail_quote">On Thu, May 9, 2013 at 10:24 PM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class=""><div class="h5">On Tue, May 7, 2013 at 11:32 AM, Alexander Kornienko <<a href="mailto:alexfh@google.com">alexfh@google.com</a>> wrote:<br>
> Author: alexfh<br>
> Date: Tue May 7 10:32:14 2013<br>
> New Revision: 181326<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=181326&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=181326&view=rev</a><br>
> Log:<br>
> Config file support for clang-format, part 1.<br>
><br>
> Summary:<br>
> Added parseConfiguration method, which reads FormatStyle from YAML<br>
> string. This supports all FormatStyle fields and an additional BasedOnStyle<br>
> field, which can be used to specify base style.<br>
><br>
> Reviewers: djasper, klimek<br>
><br>
> Reviewed By: djasper<br>
><br>
> CC: cfe-commits<br>
><br>
> Differential Revision: <a href="http://llvm-reviews.chandlerc.com/D754" target="_blank">http://llvm-reviews.chandlerc.com/D754</a><br>
><br>
> Modified:<br>
> cfe/trunk/include/clang/Format/Format.h<br>
> cfe/trunk/lib/Format/Format.cpp<br>
> cfe/trunk/unittests/Format/FormatTest.cpp<br>
><br>
> Modified: cfe/trunk/include/clang/Format/Format.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=181326&r1=181325&r2=181326&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=181326&r1=181325&r2=181326&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Format/Format.h (original)<br>
> +++ cfe/trunk/include/clang/Format/Format.h Tue May 7 10:32:14 2013<br>
> @@ -17,6 +17,7 @@<br>
><br>
> #include "clang/Frontend/FrontendAction.h"<br>
> #include "clang/Tooling/Refactoring.h"<br>
> +#include "llvm/Support/system_error.h"<br>
><br>
> namespace clang {<br>
><br>
> @@ -110,6 +111,18 @@ FormatStyle getChromiumStyle();<br>
> /// <a href="https://developer.mozilla.org/en-US/docs/Developer_Guide/Coding_Style" target="_blank">https://developer.mozilla.org/en-US/docs/Developer_Guide/Coding_Style</a>.<br>
> FormatStyle getMozillaStyle();<br>
><br>
> +/// \brief Returns a predefined style by name.<br>
> +///<br>
> +/// Currently supported names: LLVM, Google, Chromium, Mozilla. Names are<br>
> +/// compared case-insensitively.<br>
> +FormatStyle getPredefinedStyle(StringRef Name);<br>
> +<br>
> +/// \brief Parse configuration from YAML-formatted text.<br>
> +llvm::error_code parseConfiguration(StringRef Text, FormatStyle *Style);<br>
> +<br>
> +/// \brief Gets configuration in a YAML string.<br>
> +std::string configurationAsText(const FormatStyle &Style);<br>
> +<br>
> /// \brief Reformats the given \p Ranges in the token stream coming out of<br>
> /// \c Lex.<br>
> ///<br>
><br>
> Modified: cfe/trunk/lib/Format/Format.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=181326&r1=181325&r2=181326&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=181326&r1=181325&r2=181326&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Format/Format.cpp (original)<br>
> +++ cfe/trunk/lib/Format/Format.cpp Tue May 7 10:32:14 2013<br>
> @@ -28,9 +28,59 @@<br>
> #include "llvm/ADT/STLExtras.h"<br>
> #include "llvm/Support/Allocator.h"<br>
> #include "llvm/Support/Debug.h"<br>
> +#include "llvm/Support/YAMLTraits.h"<br>
> #include <queue><br>
> #include <string><br>
><br>
> +namespace llvm {<br>
> +namespace yaml {<br>
> +template <><br>
> +struct ScalarEnumerationTraits<clang::format::FormatStyle::LanguageStandard> {<br>
> + static void enumeration(IO &io,<br>
> + clang::format::FormatStyle::LanguageStandard &value) {<br>
> + io.enumCase(value, "C++03", clang::format::FormatStyle::LS_Cpp03);<br>
> + io.enumCase(value, "C++11", clang::format::FormatStyle::LS_Cpp11);<br>
> + io.enumCase(value, "Auto", clang::format::FormatStyle::LS_Auto);<br>
> + }<br>
> +};<br>
> +<br>
> +template <> struct MappingTraits<clang::format::FormatStyle> {<br>
> + static void mapping(llvm::yaml::IO &IO, clang::format::FormatStyle &Style) {<br>
> + if (!IO.outputting()) {<br>
> + StringRef BasedOnStyle;<br>
> + IO.mapOptional("BasedOnStyle", BasedOnStyle);<br>
> +<br>
> + if (!BasedOnStyle.empty())<br>
> + Style = clang::format::getPredefinedStyle(BasedOnStyle);<br>
> + }<br>
> +<br>
> + IO.mapOptional("AccessModifierOffset", Style.AccessModifierOffset);<br>
> + IO.mapOptional("AlignEscapedNewlinesLeft", Style.AlignEscapedNewlinesLeft);<br>
> + IO.mapOptional("AllowAllParametersOfDeclarationOnNextLine",<br>
> + Style.AllowAllParametersOfDeclarationOnNextLine);<br>
> + IO.mapOptional("AllowShortIfStatementsOnASingleLine",<br>
> + Style.AllowShortIfStatementsOnASingleLine);<br>
> + IO.mapOptional("BinPackParameters", Style.BinPackParameters);<br>
> + IO.mapOptional("ColumnLimit", Style.ColumnLimit);<br>
> + IO.mapOptional("ConstructorInitializerAllOnOneLineOrOnePerLine",<br>
> + Style.ConstructorInitializerAllOnOneLineOrOnePerLine);<br>
> + IO.mapOptional("DerivePointerBinding", Style.DerivePointerBinding);<br>
> + IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);<br>
> + IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);<br>
> + IO.mapOptional("ObjCSpaceBeforeProtocolList",<br>
> + Style.ObjCSpaceBeforeProtocolList);<br>
> + IO.mapOptional("PenaltyExcessCharacter", Style.PenaltyExcessCharacter);<br>
> + IO.mapOptional("PenaltyReturnTypeOnItsOwnLine",<br>
> + Style.PenaltyReturnTypeOnItsOwnLine);<br>
> + IO.mapOptional("PointerBindsToType", Style.PointerBindsToType);<br>
> + IO.mapOptional("SpacesBeforeTrailingComments",<br>
> + Style.SpacesBeforeTrailingComments);<br>
> + IO.mapOptional("Standard", Style.Standard);<br>
> + }<br>
> +};<br>
> +}<br>
> +}<br>
> +<br>
> namespace clang {<br>
> namespace format {<br>
><br>
> @@ -98,6 +148,37 @@ FormatStyle getMozillaStyle() {<br>
> return MozillaStyle;<br>
> }<br>
><br>
> +FormatStyle getPredefinedStyle(StringRef Name) {<br>
> + if (Name.equals_lower("llvm"))<br>
> + return getLLVMStyle();<br>
> + if (Name.equals_lower("chromium"))<br>
> + return getChromiumStyle();<br>
> + if (Name.equals_lower("mozilla"))<br>
> + return getMozillaStyle();<br>
> + if (Name.equals_lower("google"))<br>
> + return getGoogleStyle();<br>
> +<br>
> + llvm::errs() << "Unknown style " << Name << ", using Google style.\n";<br>
> + return getGoogleStyle();<br>
> +}<br>
> +<br>
> +llvm::error_code parseConfiguration(StringRef Text, FormatStyle *Style) {<br>
> + llvm::yaml::Input Input(Text);<br>
> + Input >> *Style;<br>
> + return Input.error();<br>
> +}<br>
> +<br>
> +std::string configurationAsText(const FormatStyle &Style) {<br>
> + std::string Text;<br>
> + llvm::raw_string_ostream Stream(Text);<br>
> + llvm::yaml::Output Output(Stream);<br>
> + // We use the same mapping method for input and output, so we need a non-const<br>
> + // reference here.<br>
> + FormatStyle NonConstStyle = Style;<br>
> + Output << NonConstStyle;<br>
> + return Text;<br>
> +}<br>
> +<br>
> // Returns the length of everything up to the first possible line break after<br>
> // the ), ], } or > matching \c Tok.<br>
> static unsigned getLengthToMatchingParen(const AnnotatedToken &Tok) {<br>
><br>
> Modified: cfe/trunk/unittests/Format/FormatTest.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=181326&r1=181325&r2=181326&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=181326&r1=181325&r2=181326&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)<br>
> +++ cfe/trunk/unittests/Format/FormatTest.cpp Tue May 7 10:32:14 2013<br>
> @@ -3909,5 +3909,113 @@ TEST_F(FormatTest, DoNotCreateUnreasonab<br>
> "}");<br>
> }<br>
><br>
> +bool operator==(const FormatStyle &L, const FormatStyle &R) {<br>
> + return L.AccessModifierOffset == R.AccessModifierOffset &&<br>
> + L.AlignEscapedNewlinesLeft == R.AlignEscapedNewlinesLeft &&<br>
> + L.AllowAllParametersOfDeclarationOnNextLine ==<br>
> + R.AllowAllParametersOfDeclarationOnNextLine &&<br>
> + L.AllowShortIfStatementsOnASingleLine ==<br>
> + R.AllowShortIfStatementsOnASingleLine &&<br>
> + L.BinPackParameters == R.BinPackParameters &&<br>
> + L.ColumnLimit == R.ColumnLimit &&<br>
> + L.ConstructorInitializerAllOnOneLineOrOnePerLine ==<br>
> + R.ConstructorInitializerAllOnOneLineOrOnePerLine &&<br>
> + L.DerivePointerBinding == R.DerivePointerBinding &&<br>
> + L.IndentCaseLabels == R.IndentCaseLabels &&<br>
> + L.MaxEmptyLinesToKeep == R.MaxEmptyLinesToKeep &&<br>
> + L.ObjCSpaceBeforeProtocolList == R.ObjCSpaceBeforeProtocolList &&<br>
> + L.PenaltyExcessCharacter == R.PenaltyExcessCharacter &&<br>
> + L.PenaltyReturnTypeOnItsOwnLine == R.PenaltyReturnTypeOnItsOwnLine &&<br>
> + L.PointerBindsToType == R.PointerBindsToType &&<br>
> + L.SpacesBeforeTrailingComments == R.SpacesBeforeTrailingComments &&<br>
> + L.Standard == R.Standard;<br>
> +}<br>
> +<br>
> +bool allStylesEqual(ArrayRef<FormatStyle> Styles) {<br>
> + for (size_t i = 1; i < Styles.size(); ++i)<br>
> + if (!(Styles[0] == Styles[i]))<br>
> + return false;<br>
> + return true;<br>
> +}<br>
> +<br>
> +TEST_F(FormatTest, GetsPredefinedStyleByName) {<br>
> + FormatStyle LLVMStyles[] = { getLLVMStyle(), getPredefinedStyle("LLVM"),<br>
> + getPredefinedStyle("llvm"),<br>
> + getPredefinedStyle("lLvM") };<br>
> + EXPECT_TRUE(allStylesEqual(LLVMStyles));<br>
> +<br>
> + FormatStyle GoogleStyles[] = { getGoogleStyle(), getPredefinedStyle("Google"),<br>
> + getPredefinedStyle("google"),<br>
> + getPredefinedStyle("gOOgle") };<br>
> + EXPECT_TRUE(allStylesEqual(GoogleStyles));<br>
> +<br>
> + FormatStyle ChromiumStyles[] = { getChromiumStyle(),<br>
> + getPredefinedStyle("Chromium"),<br>
> + getPredefinedStyle("chromium"),<br>
> + getPredefinedStyle("chROmiUM") };<br>
> + EXPECT_TRUE(allStylesEqual(ChromiumStyles));<br>
> +<br>
> + FormatStyle MozillaStyles[] = { getMozillaStyle(),<br>
> + getPredefinedStyle("Mozilla"),<br>
> + getPredefinedStyle("mozilla"),<br>
> + getPredefinedStyle("moZilla") };<br>
> + EXPECT_TRUE(allStylesEqual(MozillaStyles));<br>
> +}<br>
> +<br>
> +TEST_F(FormatTest, ParsesConfiguration) {<br>
> + FormatStyle Style = {};<br>
> +#define CHECK_PARSE(TEXT, FIELD, VALUE) \<br>
> + EXPECT_NE(VALUE, Style.FIELD); \<br>
> + EXPECT_EQ(0, parseConfiguration(TEXT, &Style).value()); \<br>
> + EXPECT_EQ(VALUE, Style.FIELD)<br>
> +<br>
> +#define CHECK_PARSE_BOOL(FIELD) \<br>
> + Style.FIELD = false; \<br>
> + EXPECT_EQ(0, parseConfiguration(#FIELD ": true", &Style).value()); \<br>
> + EXPECT_EQ(true, Style.FIELD); \<br>
> + EXPECT_EQ(0, parseConfiguration(#FIELD ": false", &Style).value()); \<br>
> + EXPECT_EQ(false, Style.FIELD);<br>
> +<br>
> + CHECK_PARSE_BOOL(AlignEscapedNewlinesLeft);<br>
> + CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);<br>
> + CHECK_PARSE_BOOL(AllowShortIfStatementsOnASingleLine);<br>
> + CHECK_PARSE_BOOL(BinPackParameters);<br>
> + CHECK_PARSE_BOOL(ConstructorInitializerAllOnOneLineOrOnePerLine);<br>
> + CHECK_PARSE_BOOL(DerivePointerBinding);<br>
> + CHECK_PARSE_BOOL(IndentCaseLabels);<br>
> + CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);<br>
> + CHECK_PARSE_BOOL(PointerBindsToType);<br>
> +<br>
> + CHECK_PARSE("AccessModifierOffset: -1234", AccessModifierOffset, -1234);<br>
> + CHECK_PARSE("ColumnLimit: 1234", ColumnLimit, 1234u);<br>
> + CHECK_PARSE("MaxEmptyLinesToKeep: 1234", MaxEmptyLinesToKeep, 1234u);<br>
> + CHECK_PARSE("PenaltyExcessCharacter: 1234", PenaltyExcessCharacter, 1234u);<br>
> + CHECK_PARSE("PenaltyReturnTypeOnItsOwnLine: 1234",<br>
> + PenaltyReturnTypeOnItsOwnLine, 1234u);<br>
> + CHECK_PARSE("SpacesBeforeTrailingComments: 1234",<br>
> + SpacesBeforeTrailingComments, 1234u);<br>
> +<br>
> + Style.Standard = FormatStyle::LS_Auto;<br>
> + CHECK_PARSE("Standard: C++03", Standard, FormatStyle::LS_Cpp03);<br>
> + CHECK_PARSE("Standard: C++11", Standard, FormatStyle::LS_Cpp11);<br>
> + CHECK_PARSE("Standard: Auto", Standard, FormatStyle::LS_Auto);<br>
> +<br>
> + Style.ColumnLimit = 123;<br>
> + FormatStyle BaseStyle = getLLVMStyle();<br>
> + CHECK_PARSE("BasedOnStyle: LLVM", ColumnLimit, BaseStyle.ColumnLimit);<br>
> + CHECK_PARSE("BasedOnStyle: LLVM\nColumnLimit: 1234", ColumnLimit, 1234u);<br>
> +<br>
> +#undef CHECK_PARSE<br>
> +#undef CHECK_PARSE_BOOL<br>
> +}<br>
> +<br>
> +TEST_F(FormatTest, ConfigurationRoundTripTest) {<br>
> + FormatStyle Style = getLLVMStyle();<br>
> + std::string YAML = configurationAsText(Style);<br>
> + FormatStyle ParsedStyle = {};<br>
> + EXPECT_EQ(0, parseConfiguration(YAML, &ParsedStyle).value());<br>
<br>
</div></div>This unit test is failing because of Input::beginMapping in<br>
YAMLTraits.cpp. It uses dyn_cast, but CurrentNode is null (and so it<br>
asserts). I don't know enough about YAML to know whether CurrentNode<br>
being null is sensible or not. </blockquote></div><br>The failure may be related to this bug in YAML I/O: <a href="http://llvm.org/bugs/show_bug.cgi?id=15927">http://llvm.org/bugs/show_bug.cgi?id=15927</a>, but this test passes for me (linux on amd64, build type - none, with assertions). Could you provide more details on your build configuration/environment? Inserting <font face="courier new, monospace">llvm::errs() << "<" << YAML << ">\n";</font> after configurationAsText(Style) call could also be helpful.</div>
</div>