r292562 - clang-format: fix fallback style set to "none" not always formatting
Antonio Maiorano via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 20 08:18:29 PST 2017
Hello Mikael,
Did you try again with the removal of the redundant test (D28943)?
On Fri, 20 Jan 2017 at 08:34 Mikael Holmén <mikael.holmen at ericsson.com>
wrote:
> Hi,
>
> On 01/20/2017 02:20 PM, Antonio Maiorano wrote:
> > Hi Mikael,
> >
> > Just to be sure, did you build clang-format before running this test?
>
> Yes I did. I compiled a clean trunk, ran the test, got the failure.
>
> >
> > Possibly related, one of the tests I added was just removed for being
> > redundant:
> > https://reviews.llvm.org/D28943
>
> Aha. I can't say I understand what's going on there, I don't even know
> how to change the default fallback style or what it is :)
>
> Regards,
>
> Mikael
>
> >
> >
> > On Fri, Jan 20, 2017 at 7:50 AM Mikael Holmén
> > <mikael.holmen at ericsson.com <mailto:mikael.holmen at ericsson.com>> wrote:
> >
> > Hi Antonio,
> >
> >
> >
> > The test case
> >
> >
> >
> > Clang :: Format/style-on-command-line.cpp
> >
> >
> >
> > fails for me since this commit.
> >
> >
> >
> > test/Format/style-on-command-line.cpp:40:13: error: expected string
> not
> >
> > found in input
> >
> > // CHECK10: {{^ int\* i;$}}
> >
> > ^
> >
> > <stdin>:1:1: note: scanning from here
> >
> > // RUN: clang-format -style="{BasedOnStyle: Google, IndentWidth: 8}"
> > %s |
> >
> > ^
> >
> > <stdin>:61:16: note: possible intended match here
> >
> > // CHECK10: {{^ int\* i;$}}
> >
> > ^
> >
> >
> >
> > I suppose it works for you... Any idea what the problem can be?
> >
> >
> >
> > Regards,
> >
> > Mikael
> >
> >
> > On 01/20/2017 02:22 AM, Antonio Maiorano via cfe-commits wrote:
> >
> > > Author: amaiorano
> >
> > > Date: Thu Jan 19 19:22:42 2017
> >
> > > New Revision: 292562
> >
> > >
> >
> > > URL: http://llvm.org/viewvc/llvm-project?rev=292562&view=rev
> >
> > > Log:
> >
> > > clang-format: fix fallback style set to "none" not always
> formatting
> >
> > >
> >
> > > This fixes clang-format not formatting if fallback-style is
> > explicitly set to
> >
> > > "none", and either a config file is found or YAML is passed in
> > without a
> >
> > > "BasedOnStyle". With this change, passing "none" in these cases
> > will have no
> >
> > > affect, and LLVM style will be used as the base style.
> >
> > >
> >
> > > Differential Revision: https://reviews.llvm.org/D28844
> >
> > >
> >
> > > Modified:
> >
> > > cfe/trunk/lib/Format/Format.cpp
> >
> > > cfe/trunk/test/Format/style-on-command-line.cpp
> >
> > > cfe/trunk/unittests/Format/FormatTest.cpp
> >
> > >
> >
> > > Modified: cfe/trunk/lib/Format/Format.cpp
> >
> > > URL:
> >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=292562&r1=292561&r2=292562&view=diff
> >
> > >
> >
> ==============================================================================
> >
> > > --- cfe/trunk/lib/Format/Format.cpp (original)
> >
> > > +++ cfe/trunk/lib/Format/Format.cpp Thu Jan 19 19:22:42 2017
> >
> > > @@ -1888,8 +1888,8 @@ static FormatStyle::LanguageKind getLang
> >
> > > }
> >
> > >
> >
> > > llvm::Expected<FormatStyle> getStyle(StringRef StyleName,
> > StringRef FileName,
> >
> > > - StringRef FallbackStyle,
> > StringRef Code,
> >
> > > - vfs::FileSystem *FS) {
> >
> > > + StringRef FallbackStyleName,
> >
> > > + StringRef Code,
> > vfs::FileSystem *FS) {
> >
> > > if (!FS) {
> >
> > > FS = vfs::getRealFileSystem().get();
> >
> > > }
> >
> > > @@ -1903,9 +1903,9 @@ llvm::Expected<FormatStyle> getStyle(Str
> >
> > > (Code.contains("\n- (") || Code.contains("\n+ (")))
> >
> > > Style.Language = FormatStyle::LK_ObjC;
> >
> > >
> >
> > > - // FIXME: If FallbackStyle is explicitly "none", format is
> > disabled.
> >
> > > - if (!getPredefinedStyle(FallbackStyle, Style.Language, &Style))
> >
> > > - return make_string_error("Invalid fallback style \"" +
> > FallbackStyle.str());
> >
> > > + FormatStyle FallbackStyle = getNoStyle();
> >
> > > + if (!getPredefinedStyle(FallbackStyleName, Style.Language,
> > &FallbackStyle))
> >
> > > + return make_string_error("Invalid fallback style \"" +
> > FallbackStyleName);
> >
> > >
> >
> > > if (StyleName.startswith("{")) {
> >
> > > // Parse YAML/JSON style from the command line.
> >
> > > @@ -1977,7 +1977,7 @@ llvm::Expected<FormatStyle> getStyle(Str
> >
> > > return make_string_error("Configuration file(s) do(es) not
> > support " +
> >
> > > getLanguageName(Style.Language) + ":
> " +
> >
> > > UnsuitableConfigFiles);
> >
> > > - return Style;
> >
> > > + return FallbackStyle;
> >
> > > }
> >
> > >
> >
> > > } // namespace format
> >
> > >
> >
> > > Modified: cfe/trunk/test/Format/style-on-command-line.cpp
> >
> > > URL:
> >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Format/style-on-command-line.cpp?rev=292562&r1=292561&r2=292562&view=diff
> >
> > >
> >
> ==============================================================================
> >
> > > --- cfe/trunk/test/Format/style-on-command-line.cpp (original)
> >
> > > +++ cfe/trunk/test/Format/style-on-command-line.cpp Thu Jan 19
> > 19:22:42 2017
> >
> > > @@ -11,6 +11,21 @@
> >
> > > // RUN: clang-format -style=file -assume-filename=%T/foo.cpp < %s
> > | FileCheck -strict-whitespace -check-prefix=CHECK7 %s
> >
> > > // RUN: clang-format -style="{BasedOnStyle: LLVM,
> > PointerBindsToType: true}" %s | FileCheck -strict-whitespace
> > -check-prefix=CHECK8 %s
> >
> > > // RUN: clang-format -style="{BasedOnStyle: WebKit,
> > PointerBindsToType: false}" %s | FileCheck -strict-whitespace
> > -check-prefix=CHECK9 %s
> >
> > > +
> >
> > > +// Fallback style tests
> >
> > > +// RUN: rm %T/_clang-format
> >
> > > +// Test no config file found, WebKit fallback style is applied
> >
> > > +// RUN: clang-format -style=file -fallback-style=WebKit
> > -assume-filename=%T/foo.cpp < %s 2>&1 | FileCheck -strict-whitespace
> > -check-prefix=CHECK10 %s
> >
> > > +// Test no config file and no fallback style, LLVM style is
> applied
> >
> > > +// RUN: clang-format -style=file -assume-filename=%T/foo.cpp < %s
> > 2>&1 | FileCheck -strict-whitespace -check-prefix=CHECK11 %s
> >
> > > +// Test no config file and fallback style "none", no formatting
> > is applied
> >
> > > +// RUN: clang-format -style=file -fallback-style=none
> > -assume-filename=%T/foo.cpp < %s 2>&1 | FileCheck -strict-whitespace
> > -check-prefix=CHECK12 %s
> >
> > > +// Test config file with no based style, and fallback style
> > "none", formatting is applied
> >
> > > +// RUN: printf "IndentWidth: 6\n" > %T/_clang-format
> >
> > > +// RUN: clang-format -style=file -fallback-style=none
> > -assume-filename=%T/foo.cpp < %s 2>&1 | FileCheck -strict-whitespace
> > -check-prefix=CHECK13 %s
> >
> > > +// Test yaml with no based style, and fallback style "none", LLVM
> > formatting applied
> >
> > > +// RUN: clang-format -style="{IndentWidth: 7}"
> > -fallback-style=none %s | FileCheck -strict-whitespace
> > -check-prefix=CHECK14 %s
> >
> > > +
> >
> > > void f() {
> >
> > > // CHECK1: {{^ int\* i;$}}
> >
> > > // CHECK2: {{^ int \*i;$}}
> >
> > > @@ -22,6 +37,11 @@ void f() {
> >
> > > // CHECK7: {{^ int\* i;$}}
> >
> > > // CHECK8: {{^ int\* i;$}}
> >
> > > // CHECK9: {{^ int \*i;$}}
> >
> > > +// CHECK10: {{^ int\* i;$}}
> >
> > > +// CHECK11: {{^ int \*i;$}}
> >
> > > +// CHECK12: {{^int\*i;$}}
> >
> > > +// CHECK13: {{^ int \*i;$}}
> >
> > > +// CHECK14: {{^ int \*i;$}}
> >
> > > int*i;
> >
> > > int j;
> >
> > > }
> >
> > >
> >
> > > Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> >
> > > URL:
> >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=292562&r1=292561&r2=292562&view=diff
> >
> > >
> >
> ==============================================================================
> >
> > > --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> >
> > > +++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Jan 19 19:22:42
> 2017
> >
> > > @@ -10978,13 +10978,31 @@ TEST(FormatStyle, GetStyleOfFile) {
> >
> > > ASSERT_TRUE((bool)Style1);
> >
> > > ASSERT_EQ(*Style1, getLLVMStyle());
> >
> > >
> >
> > > - // Test 2: fallback to default.
> >
> > > + // Test 2.1: fallback to default.
> >
> > > ASSERT_TRUE(
> >
> > > FS.addFile("/b/test.cpp", 0,
> > llvm::MemoryBuffer::getMemBuffer("int i;")));
> >
> > > auto Style2 = getStyle("file", "/b/test.cpp", "Mozilla", "",
> &FS);
> >
> > > ASSERT_TRUE((bool)Style2);
> >
> > > ASSERT_EQ(*Style2, getMozillaStyle());
> >
> > >
> >
> > > + // Test 2.2: no format on 'none' fallback style.
> >
> > > + Style2 = getStyle("file", "/b/test.cpp", "none", "", &FS);
> >
> > > + ASSERT_TRUE((bool)Style2);
> >
> > > + ASSERT_EQ(*Style2, getNoStyle());
> >
> > > +
> >
> > > + // Test 2.3: format if config is found with no based style
> > while fallback is
> >
> > > + // 'none'.
> >
> > > + ASSERT_TRUE(FS.addFile("/b/.clang-format", 0,
> >
> > > +
> > llvm::MemoryBuffer::getMemBuffer("IndentWidth: 2")));
> >
> > > + Style2 = getStyle("file", "/b/test.cpp", "none", "", &FS);
> >
> > > + ASSERT_TRUE((bool)Style2);
> >
> > > + ASSERT_EQ(*Style2, getLLVMStyle());
> >
> > > +
> >
> > > + // Test 2.4: format if yaml with no based style, while fallback
> > is 'none'.
> >
> > > + Style2 = getStyle("{}", "a.h", "none", "", &FS);
> >
> > > + ASSERT_TRUE((bool)Style2);
> >
> > > + ASSERT_EQ(*Style2, getLLVMStyle());
> >
> > > +
> >
> > > // Test 3: format file in parent directory.
> >
> > > ASSERT_TRUE(
> >
> > > FS.addFile("/c/.clang-format", 0,
> >
> > >
> >
> > >
> >
> > > _______________________________________________
> >
> > > cfe-commits mailing list
> >
> > > cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
> >
> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
> > >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170120/d7642bfa/attachment-0001.html>
More information about the cfe-commits
mailing list