r292562 - clang-format: fix fallback style set to "none" not always formatting

Fri Jan 20 05:20:53 PST 2017

Hi Mikael,

Just to be sure, did you build clang-format before running this test?

Possibly related, one of the tests I added was just removed for being
redundant:
https://reviews.llvm.org/D28943

On Fri, Jan 20, 2017 at 7:50 AM Mikael Holmén <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
>
> > 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/deab644d/attachment-0001.html>