Update tools to use new getStyle API

[amaiorano]

Summary

See https://reviews.llvm.org/D28081 for the changes to getStyle

This change will be committed right after D28081.

[amaiorano]

I made these changes, and they build, but I didn't really test them. Are there unit tests for these tools that I can run?

[malcolm.parsons]

A mailing list cannot review.

[hokein]

In D28315#635865, @amaiorano wrote:

I made these changes, and they build, but I didn't really test them. Are there unit tests for these tools that I can run?

If you use ninja build, you can run ninja check-clang-tools to run all tests.

[amaiorano]

In D28315#636670, @hokein wrote:

In D28315#635865, @amaiorano wrote:

I made these changes, and they build, but I didn't really test them. Are there unit tests for these tools that I can run?

If you use ninja build, you can run ninja check-clang-tools to run all tests.

Is there a way to run tests without ninja? I'm on Windows. If not, I'll use my Linux VM.

[hokein]

In D28315#636821, @amaiorano wrote:

Is there a way to run tests without ninja? I'm on Windows. If not, I'll use my Linux VM.

It is not related to the build system. There should be a check-clang-tools project, you can run tests by building it. For example, if you are using make, just run make check-clang-tools; If you are using Visual Studio, build the check-clang-tools project.

[amaiorano]

In D28315#636888, @hokein wrote:

In D28315#636821, @amaiorano wrote:

Is there a way to run tests without ninja? I'm on Windows. If not, I'll use my Linux VM.

It is not related to the build system. There should be a check-clang-tools project, you can run tests by building it. For example, if you are using make, just run make check-clang-tools; If you are using Visual Studio, build the check-clang-tools project.

Yes, thanks for that. Unfortunately, the tests don't all pass when run using Visual Studio. I installed the requisite GNU utils, but get 25 failures. Will investigate a bit more, otherwise will use Linux.

Edit: to clarify, the tests fail without my changes.

[amaiorano]

@ioeric Okay, check-clang-tools passes with my changes.

FYI, tests were originally failing for me (without my changes) because I use Git, and it's configured to convert line endings to Windows (crlf) on checkout, but some of the tools tests specify extract offsets into source files, which end up being incorrect if you've got an extra byte per line ;) I'll open a ticket/discussion about it separately.

So if you give me the go ahead, I'll submit this change _after_ submitting D28081.

[ioeric]

I ran ninja check-all with D28081 and D28315 (in Linux), and some lit tests failed, namely:

Clang :: Format/style-on-command-line.cpp
Clang Tools :: clang-apply-replacements/basic.cpp
Clang Tools :: clang-apply-replacements/conflict.cpp
Clang Tools :: clang-apply-replacements/crlf.cpp
Clang Tools :: clang-apply-replacements/format.cpp
Clang Tools :: clang-rename/ClassReplacements.cpp

Error message I am seeing: Expected<T> must be checked before access or destruction.

Could you double check? Thanks!

change-namespace/ChangeNamespace.cpp

892

I'd still like to apply replacements that are not cleaned up. Could you add the following line before continue, thanks! :)

FileToReplacements[FilePath] = Replaces;

[amaiorano]

In D28315#639559, @ioeric wrote:

I ran ninja check-all with D28081 and D28315 (in Linux), and some lit tests failed, namely:

Clang :: Format/style-on-command-line.cpp
Clang Tools :: clang-apply-replacements/basic.cpp
Clang Tools :: clang-apply-replacements/conflict.cpp
Clang Tools :: clang-apply-replacements/crlf.cpp
Clang Tools :: clang-apply-replacements/format.cpp
Clang Tools :: clang-rename/ClassReplacements.cpp

Error message I am seeing: Expected<T> must be checked before access or destruction.

Could you double check? Thanks!

Will definitely double check. Strange that I didn't get these errors on Windows. I did notice in the output that it said one test was disabled. I'll take a closer look asap.

change-namespace/ChangeNamespace.cpp

892

Will do.

[amaiorano]

In D28315#639559, @ioeric wrote:

I ran ninja check-all with D28081 and D28315 (in Linux), and some lit tests failed, namely:

Clang :: Format/style-on-command-line.cpp

This was the test that is explicitly disabled on Windows. I enabled it locally and modified the test to match the modified output and expected non-zero result (patch forth-coming once I understand what's up with the failed tests you mention below...)

Clang Tools :: clang-apply-replacements/basic.cpp
Clang Tools :: clang-apply-replacements/conflict.cpp
Clang Tools :: clang-apply-replacements/crlf.cpp
Clang Tools :: clang-apply-replacements/format.cpp
Clang Tools :: clang-rename/ClassReplacements.cpp

Error message I am seeing: `Expected<T> must be checked before access or destruction.`

Could you double check? Thanks!

These tests pass for me. For example, when I run llvm-lit explicitly on clang-apply-replacements/basic.cpp, I get:

C:\code\llvm-build-msvc\Debug\bin>llvm-lit.py -a -v c:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements\basic.cpp
-- Testing: 1 tests, 1 threads --
PASS: Clang Tools :: clang-apply-replacements/basic.cpp (1 of 1)
Script:
--
mkdir -p C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic
grep -Ev "// *[A-Z-]+:" C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/basic.h > C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic/basic.h
sed "s#\$(path)#C:/code/llvm-build-msvc/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/basic#" C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/file1.yaml > C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic/file1.yaml
sed "s#\$(path)#C:/code/llvm-build-msvc/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/basic#" C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/file2.yaml > C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic/file2.yaml
clang-apply-replacements C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic
FileCheck -input-file=C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic/basic.h C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/basic.h
ls -1 C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic | FileCheck C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements\basic.cpp --check-prefix=YAML
grep -Ev "// *[A-Z-]+:" C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/basic.h > C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic/basic.h
clang-apply-replacements -remove-change-desc-files C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic
ls -1 C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic | FileCheck C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements\basic.cpp --check-prefix=NO_YAML
--
Exit Code: 0

Command Output (stdout):
--
$ "mkdir" "-p" "C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic"
$ "grep" "-Ev" "// *[A-Z-]+:" "C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/basic.h"
$ "sed" "s#\$(path)#C:/code/llvm-build-msvc/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/basic#" "C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/file1.yaml"
$ "sed" "s#\$(path)#C:/code/llvm-build-msvc/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/basic#" "C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/file2.yaml"
$ "clang-apply-replacements" "C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic"
$ "FileCheck" "-input-file=C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic/basic.h" "C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/basic.h"
$ "ls" "-1" "C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic"
$ "FileCheck" "C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements\basic.cpp" "--check-prefix=YAML"
$ "grep" "-Ev" "// *[A-Z-]+:" "C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/basic.h"
$ "clang-apply-replacements" "-remove-change-desc-files" "C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic"
$ "ls" "-1" "C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic"
$ "FileCheck" "C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements\basic.cpp" "--check-prefix=NO_YAML"

--

********************
Testing Time: 0.22s
  Expected Passes    : 1

Would you mind running this same command on Linux (with -a -v) and seeing what's different? I don't see why this test would run differently on Windows.

Having said that, I believe I can see one place where I could get Expected<T> must be checked before access or destruction.: in ClangApplyReplacementsMain.cpp, I modified the code to:

llvm::Expected<format::FormatStyle> FormatStyle = format::getNoStyle();
if (DoFormat) {
  FormatStyle = format::getStyle(FormatStyleOpt, FormatStyleConfig, "LLVM");
  if (!FormatStyle) {
    llvm::errs() << llvm::toString(FormatStyle.takeError()) << "\n";
    return 1;
  }
}

If DoFormat is false, then FormatStyle will not be checked. I can fix this, but I would love to get this actual error out of the test myself.

[amaiorano]

In D28315#641032, @amaiorano wrote:

In D28315#639559, @ioeric wrote:

I ran ninja check-all with D28081 and D28315 (in Linux), and some lit tests failed, namely:

Clang :: Format/style-on-command-line.cpp

This was the test that is explicitly disabled on Windows. I enabled it locally and modified the test to match the modified output and expected non-zero result (patch forth-coming once I understand what's up with the failed tests you mention below...)

Clang Tools :: clang-apply-replacements/basic.cpp
Clang Tools :: clang-apply-replacements/conflict.cpp
Clang Tools :: clang-apply-replacements/crlf.cpp
Clang Tools :: clang-apply-replacements/format.cpp
Clang Tools :: clang-rename/ClassReplacements.cpp

Error message I am seeing: `Expected<T> must be checked before access or destruction.`

Could you double check? Thanks!

These tests pass for me. For example, when I run llvm-lit explicitly on clang-apply-replacements/basic.cpp, I get:

C:\code\llvm-build-msvc\Debug\bin>llvm-lit.py -a -v c:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements\basic.cpp
-- Testing: 1 tests, 1 threads --
PASS: Clang Tools :: clang-apply-replacements/basic.cpp (1 of 1)
Script:
--
mkdir -p C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic
grep -Ev "// *[A-Z-]+:" C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/basic.h > C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic/basic.h
sed "s#\$(path)#C:/code/llvm-build-msvc/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/basic#" C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/file1.yaml > C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic/file1.yaml
sed "s#\$(path)#C:/code/llvm-build-msvc/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/basic#" C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/file2.yaml > C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic/file2.yaml
clang-apply-replacements C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic
FileCheck -input-file=C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic/basic.h C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/basic.h
ls -1 C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic | FileCheck C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements\basic.cpp --check-prefix=YAML
grep -Ev "// *[A-Z-]+:" C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/basic.h > C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic/basic.h
clang-apply-replacements -remove-change-desc-files C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic
ls -1 C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic | FileCheck C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements\basic.cpp --check-prefix=NO_YAML
--
Exit Code: 0

Command Output (stdout):
--
$ "mkdir" "-p" "C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic"
$ "grep" "-Ev" "// *[A-Z-]+:" "C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/basic.h"
$ "sed" "s#\$(path)#C:/code/llvm-build-msvc/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/basic#" "C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/file1.yaml"
$ "sed" "s#\$(path)#C:/code/llvm-build-msvc/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/basic#" "C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/file2.yaml"
$ "clang-apply-replacements" "C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic"
$ "FileCheck" "-input-file=C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic/basic.h" "C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/basic.h"
$ "ls" "-1" "C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic"
$ "FileCheck" "C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements\basic.cpp" "--check-prefix=YAML"
$ "grep" "-Ev" "// *[A-Z-]+:" "C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements/Inputs/basic/basic.h"
$ "clang-apply-replacements" "-remove-change-desc-files" "C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic"
$ "ls" "-1" "C:\code\llvm-build-msvc\tools\clang\tools\extra\test\clang-apply-replacements\Output/Inputs/basic"
$ "FileCheck" "C:\code\llvm\tools\clang\tools\extra\test\clang-apply-replacements\basic.cpp" "--check-prefix=NO_YAML"

--

********************
Testing Time: 0.22s
  Expected Passes    : 1

Would you mind running this same command on Linux (with -a -v) and seeing what's different? I don't see why this test would run differently on Windows.

Having said that, I believe I can see one place where I could get Expected<T> must be checked before access or destruction.: in ClangApplyReplacementsMain.cpp, I modified the code to:

llvm::Expected<format::FormatStyle> FormatStyle = format::getNoStyle();
if (DoFormat) {
  FormatStyle = format::getStyle(FormatStyleOpt, FormatStyleConfig, "LLVM");
  if (!FormatStyle) {
    llvm::errs() << llvm::toString(FormatStyle.takeError()) << "\n";
    return 1;
  }
}

If DoFormat is false, then FormatStyle will not be checked. I can fix this, but I would love to get this actual error out of the test myself.

Okay, I figured it out. I needed to set CMake vars DLLVM_ENABLE_ASSERTIONS=ON and DLLVM_ABI_BREAKING_CHECKS=WITH_ASSERTS so that LLVM_ENABLE_ABI_BREAKING_CHECKS would be defined to 1, thus enabling the Expected<T> error you mentioned. I am now getting these same assertions. I will fix these and update the patch.

[amaiorano]

Fixed code in ClangApplyReplacementsMain.cpp so that we only handle the Expected<Style> when DoFormat is true. Note that I'm using the unidiomatic FormatStyleOrError variable name here, but that's because we only want to set the FormatStyle variable in the parent scope when DoFormat is true.

[amaiorano]

change-namespace/ChangeNamespace.cpp

892

Ah darn, just realized that I forgot to make this change you asked for @ioeric. Let me know if everything else is okay, and I'll be sure to add this in before submitting.

[amaiorano]

change-namespace/ChangeNamespace.cpp

892

@ioeric Was looking at this code, and I'm wondering about the change you wanted me to make here. You said that you'd still like to apply replacements that are not cleaned up, so shouldn't that also be the case for when format::cleanupAroundReplacements fails (just below)? In other words, shouldn't it be:

  // Clean up old namespaces if there is nothing in it after moving.
  auto CleanReplacements =
      format::cleanupAroundReplacements(Code, Replaces, *Style);
  if (!CleanReplacements) {
    llvm::errs() << llvm::toString(CleanReplacements.takeError()) << "\n";
    FileToReplacements[FilePath] = Replaces;
    continue;
  }
  FileToReplacements[FilePath] = *CleanReplacements;
}

If so, I propose instead that we add

FileToReplacements[FilePath] = Replaces;

before the call to format::getStyle so that if either of the format-related calls below it (getStyle or cleanupAroundReplacements) fail, we get the unclean replacements.

[ioeric]

Let me know when broken tests are fixed and this patch (and the corresponding patch) is ready again for review. Also let me know if you need any help.

change-namespace/ChangeNamespace.cpp

892

After taking a closer look, I realize that we don't actually need this change since Replaces is a reference to FileToReplacements[FilePath].

[amaiorano]

Rebased changes on latest. No functional changes in this diff since last.

All tests pass (on Windows).

[amaiorano]

In D28315#647103, @ioeric wrote:

Let me know when broken tests are fixed and this patch (and the corresponding patch) is ready again for review. Also let me know if you need any help.

I updated the two patches after rebasing on latest (no conflicts). These should be ready to go. If you don't mind testing the two again on Linux, that would be greatly appreciated. Otherwise, on Windows the tests pass.

[ioeric]

In D28315#647154, @amaiorano wrote:

In D28315#647103, @ioeric wrote:

Let me know when broken tests are fixed and this patch (and the corresponding patch) is ready again for review. Also let me know if you need any help.

I updated the two patches after rebasing on latest (no conflicts). These should be ready to go. If you don't mind testing the two again on Linux, that would be greatly appreciated. Otherwise, on Windows the tests pass.

All tests passed on Linux. Are these patches still depending on D28419? Or the previous issue has been somehow resolved on Windows?

[amaiorano]

In D28315#647212, @ioeric wrote:

In D28315#647154, @amaiorano wrote:

In D28315#647103, @ioeric wrote:

Let me know when broken tests are fixed and this patch (and the corresponding patch) is ready again for review. Also let me know if you need any help.

I updated the two patches after rebasing on latest (no conflicts). These should be ready to go. If you don't mind testing the two again on Linux, that would be greatly appreciated. Otherwise, on Windows the tests pass.

All tests passed on Linux. Are these patches still depending on D28419? Or the previous issue has been somehow resolved on Windows?

Thank you for testing it on Linux. No, there is no dependency on D28419 insofar as getting these 2 patches in. D28419 is about making sure tests work on Windows if you use Git and set core.autocrlf; however, I suspect the Windows build machines are using svn.

[ioeric]

Lgtm. Thanks!

change-namespace/ChangeNamespace.cpp

for (const auto &FwdDeclInsertion : FwdDeclInsertions) {
           Replaces.getShiftedCodePosition(FwdDeclInsertion.InsertionOffset);
       tooling::Replacement Insertion(FilePath, NewInsertionOffset, 0,
                                      FwdDeclInsertion.ForwardDeclText);
       addOrMergeReplacement(Insertion, &NewReplacements);
     }
     // Add replacements referring to the changed code to existing replacements,
     // which refers to the original code.
     Replaces = Replaces.merge(NewReplacements);
-    format::FormatStyle Style =
-        format::getStyle("file", FilePath, FallbackStyle);
+    auto Style = format::getStyle("file", FilePath, FallbackStyle);
+    if (!Style) {
+      llvm::errs() << llvm::toString(Style.takeError()) << "\n";
+      continue;
+    }
     // Clean up old namespaces if there is nothing in it after moving.
     auto CleanReplacements =
-        format::cleanupAroundReplacements(Code, Replaces, Style);
+        format::cleanupAroundReplacements(Code, Replaces, *Style);
     if (!CleanReplacements) {
       llvm::errs() << llvm::toString(CleanReplacements.takeError()) << "\n";
       continue;
     }
     FileToReplacements[FilePath] = *CleanReplacements;
   }
 
   // Make sure we don't generate replacements for files that do not match
   // FilePattern.
   for (auto &Entry : FileToReplacements)
     if (!FilePatternRE.match(Entry.first))
       Entry.second.clear();
 }
 
 } // namespace change_namespace
 } // namespace clang

clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp

int main(int argc, char **argv) {
   cl::SetVersionPrinter(&printVersion);
   cl::ParseCommandLineOptions(argc, argv);
 
   IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts(new DiagnosticOptions());
   DiagnosticsEngine Diagnostics(
       IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs()), DiagOpts.get());
 
   // Determine a formatting style from options.
-  format::FormatStyle FormatStyle;
-  if (DoFormat)
+  llvm::Expected<format::FormatStyle> FormatStyle = format::getNoStyle();
+  if (DoFormat) {
     FormatStyle = format::getStyle(FormatStyleOpt, FormatStyleConfig, "LLVM");
+    if (!FormatStyle) {
+      llvm::errs() << llvm::toString(FormatStyle.takeError()) << "\n";
+      return 1;
+    }
+  }
 
   TUReplacements TURs;
   TUReplacementFiles TUFiles;
 
   std::error_code ErrorCode =
       collectReplacementsFromDirectory(Directory, TURs, TUFiles, Diagnostics);
 
   TUDiagnostics TUDs;
if (!applyReplacements(FileAndReplacements.second, NewFileData,
                            Diagnostics)) {
       errs() << "Failed to apply replacements to " << FileName << "\n";
       continue;
     }
 
     // Apply formatting if requested.
     if (DoFormat &&
         !applyFormatting(FileAndReplacements.second, NewFileData, NewFileData,
-                         FormatStyle, Diagnostics)) {
+                         *FormatStyle, Diagnostics)) {
       errs() << "Failed to apply reformatting replacements for " << FileName
              << "\n";
       continue;
     }
 
     // Write new file to disk
     std::error_code EC;
     llvm::raw_fd_ostream FileStream(FileName, EC, llvm::sys::fs::F_None);

clang-move/ClangMove.cpp

if (Context->Spec.OldDependOnNew &&
       if (Err)
         llvm::errs() << llvm::toString(std::move(Err)) << "\n";
     }
 
     auto SI = FilePathToFileID.find(FilePath);
     // Ignore replacements for new.h/cc.
     if (SI == FilePathToFileID.end()) continue;
     llvm::StringRef Code = SM.getBufferData(SI->second);
-    format::FormatStyle Style =
-        format::getStyle("file", FilePath, Context->FallbackStyle);
+    auto Style = format::getStyle("file", FilePath, Context->FallbackStyle);
+    if (!Style) {
+      llvm::errs() << llvm::toString(Style.takeError()) << "\n";
+      continue;
+    }
     auto CleanReplacements = format::cleanupAroundReplacements(
-        Code, Context->FileToReplacements[FilePath], Style);
+        Code, Context->FileToReplacements[FilePath], *Style);
 
     if (!CleanReplacements) {
       llvm::errs() << llvm::toString(CleanReplacements.takeError()) << "\n";
       continue;
     }
     Context->FileToReplacements[FilePath] = *CleanReplacements;
   }
 }

clang-tidy/ClangTidy.cpp

if (ApplyFixes && TotalFixes > 0) {
             SourceMgr.getFileManager().getBufferForFile(File);
         if (!Buffer) {
           llvm::errs() << "Can't get buffer for file " << File << ": "
                        << Buffer.getError().message() << "\n";
           // FIXME: Maybe don't apply fixes for other files as well.
           continue;
         }
         StringRef Code = Buffer.get()->getBuffer();
-        format::FormatStyle Style = format::getStyle("file", File, FormatStyle);
+        auto Style = format::getStyle("file", File, FormatStyle);
+        if (!Style) {
+          llvm::errs() << llvm::toString(Style.takeError()) << "\n";
+          continue;
+        }
         llvm::Expected<Replacements> CleanReplacements =
             format::cleanupAroundReplacements(Code, FileAndReplacements.second,
-                                              Style);
+                                              *Style);
         if (!CleanReplacements) {
           llvm::errs() << llvm::toString(CleanReplacements.takeError()) << "\n";
           continue;
         }
         if (!tooling::applyAllReplacements(CleanReplacements.get(), Rewrite)) {
           llvm::errs() << "Can't apply replacements for file " << File << "\n";
         }
       }

include-fixer/tool/ClangIncludeFixer.cpp

if (!InsertHeader.empty()) {
     //
     // Check whether all elements in HeaderInfos have the same qualified name.
     bool IsUniqueQualifiedName = std::equal(
         HeaderInfos.begin() + 1, HeaderInfos.end(), HeaderInfos.begin(),
         [](const IncludeFixerContext::HeaderInfo &LHS,
            const IncludeFixerContext::HeaderInfo &RHS) {
           return LHS.QualifiedName == RHS.QualifiedName;
         });
-    format::FormatStyle InsertStyle =
-        format::getStyle("file", Context.getFilePath(), Style);
+    auto InsertStyle = format::getStyle("file", Context.getFilePath(), Style);
+    if (!InsertStyle) {
+      llvm::errs() << llvm::toString(InsertStyle.takeError()) << "\n";
+      return 1;
+    }
     auto Replacements = clang::include_fixer::createIncludeFixerReplacements(
-        Code->getBuffer(), Context, InsertStyle,
+        Code->getBuffer(), Context, *InsertStyle,
         /*AddQualifiers=*/IsUniqueQualifiedName);
     if (!Replacements) {
       errs() << "Failed to create replacements: "
              << llvm::toString(Replacements.takeError()) << "\n";
       return 1;
     }
 
     auto ChangedCode =
if (OutputHeaders) {
     // FIXME: Print contexts of all processing files instead of the first one.
     writeToJson(llvm::outs(), Contexts.front());
     return 0;
   }
 
   std::vector<tooling::Replacements> FixerReplacements;
   for (const auto &Context : Contexts) {
     StringRef FilePath = Context.getFilePath();
-    format::FormatStyle InsertStyle = format::getStyle("file", FilePath, Style);
+    auto InsertStyle = format::getStyle("file", FilePath, Style);
+    if (!InsertStyle) {
+      llvm::errs() << llvm::toString(InsertStyle.takeError()) << "\n";
+      return 1;
+    }
     auto Buffer = llvm::MemoryBuffer::getFile(FilePath);
     if (!Buffer) {
       errs() << "Couldn't open file: " + FilePath.str() + ": "
              << Buffer.getError().message() + "\n";
       return 1;
     }
 
     auto Replacements = clang::include_fixer::createIncludeFixerReplacements(
-        Buffer.get()->getBuffer(), Context, InsertStyle);
+        Buffer.get()->getBuffer(), Context, *InsertStyle);
     if (!Replacements) {
       errs() << "Failed to create replacement: "
              << llvm::toString(Replacements.takeError()) << "\n";
       return 1;
     }
     FixerReplacements.push_back(*Replacements);
   }