[FileCheck] Add missing includes in header

[grimar]

llvm/lib/Support/FileCheck.cpp

21

It's unsorted. Should probably be:

#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringSet.h"

(In LLD we always sort headers, I am not sure this rule applies to whole LLVM, but I think it is).

But first of all I am unsure we want to include these headers after "llvm/Support/FileCheck.h" where
they are already included. Do we really want it? Why?

[thopre]

Address review comment

[thopre]

llvm/lib/Support/FileCheck.cpp

21

It's debatable. My thoughts were to include it if there's any use of a type that doesn't come from implementing an interface declared in FileCheck.h. So if interfaces change to no longer use StringRef for instance, headers inclusion only change in FileCheck.h and FileCheck.cpp still compiles (after changing the prototype of course) despite there still being some StringRef.

But I did wonder for a while whether FileCheck.cpp needed some includes as well and am happy to not change the include there.

[jhenderson]

llvm/lib/Support/FileCheck.cpp

22

Have you read the relevant part of the coding standards? llvm/Support/FileCheck.h should be first as the file's own header. Additionally, clang-format sorts things appropriately, and the coding standards say you don't need to include headers if they're included by a referenced header.

[thopre]

• Include FileCheck.h first
• Remove includes in FileCheck.cpp of headers already included in FileCheck.h

[jhenderson]

LGTM, I think.

[grimar]

Looks good to me too.

llvm/include/llvm/Support/FileCheck.h

 //==-- llvm/Support/FileCheck.h ---------------------------*- C++ -*-==//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 /// \file This file has some utilities to use FileCheck as an API
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_SUPPORT_FILECHECK_H
 #define LLVM_SUPPORT_FILECHECK_H
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/SourceMgr.h"
-#include <vector>
 #include <map>
+#include <string>
+#include <vector>
 
 namespace llvm {
 
 /// Contains info about various FileCheck options.
 struct FileCheckRequest {
   std::vector<std::string> CheckPrefixes;
   bool NoCanonicalizeWhiteSpace = false;
   std::vector<std::string> ImplicitCheckNot;

llvm/lib/Support/FileCheck.cpp

 //===- FileCheck.cpp - Check that File's Contents match what is expected --===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // FileCheck does a line-by line check of a file that validates whether it
 // contains the expected content.  This is useful for regression tests etc.
 //
 // This file implements most of the API that will be used by the FileCheck utility
 // as well as various unittests.
 //===----------------------------------------------------------------------===//
 
-#include "llvm/Support/FileCheck.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FileCheck.h"
 #include "llvm/Support/FormatVariadic.h"
 #include <cstdint>
 #include <list>
 #include <map>
 #include <tuple>
 #include <utility>
 
 using namespace llvm;