Repository: carbon-language/carbon-lang
File: toolchain/check/import_cpp.cpp (C++)

Code before:

#include "clang/Frontend/TextDiagnosticPrinter.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/raw_ostream.h"
#include "toolchain/check/context.h"
#include "toolchain/check/diagnostic_helpers.h"
#include "toolchain/diagnostics/diagnostic.h"
#include "toolchain/diagnostics/format_providers.h"

namespace Carbon::Check {
namespace {

auto GenerateCppIncludesHeaderFilename(llvm::StringRef importing_file_path)
    -> std::string {
  return llvm::Twine(importing_file_path)
      .concat(".generated.cpp_imports.h")
      .str();
}

auto GenerateCppIncludesHeaderCode(
    llvm::ArrayRef<std::pair<llvm::StringRef, SemIRLoc>> imports)
    -> std::string {
  constexpr llvm::StringLiteral IncludePrefix(R"(#include ")");
  constexpr llvm::StringLiteral IncludeSuffix(R"("
)");

  std::string code;
  for (const auto& [path, _] : imports) {
    code.append(
        llvm::Twine(IncludePrefix).concat(path).concat(IncludeSuffix).str());
  }
  return code;
}

}  // namespace

auto ImportCppFiles(
    Context& context, llvm::StringRef importing_file_path,
    llvm::ArrayRef<std::pair<llvm::StringRef, SemIRLoc>> imports,
    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs) -> void {
  size_t num_imports = imports.size();
  if (num_imports == 0) {
    return;
  }

  // TODO: Use all import locations by referring each Clang diagnostic to the
  // relevant import.
  SemIRLoc loc = imports.back().second;


Review comment (security):
I'd find these easier to read as escaped characters instead of raw string literals; `")"` hides the closing `"` for me, for example, and the newline breaks the indentation of code. That said, had you looked at inlining the constant values?

I think we often do things like this with streams, such as:

```suggestion
  std::string code;
  llvm::raw_string_ostream code_str(code);
  for (const auto& [path, _] : imports) {
    code_str << "#include \"" << path << "\"\n";
  }
  return code;
```

(ed: dropping `Twine` suggestion that doesn't work)

Code after:

#include "clang/Frontend/TextDiagnosticPrinter.h"
#include "clang/Tooling/Tooling.h"
#include "common/raw_string_ostream.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/raw_ostream.h"
#include "toolchain/check/context.h"
#include "toolchain/check/diagnostic_helpers.h"
#include "toolchain/diagnostics/diagnostic.h"
#include "toolchain/diagnostics/format_providers.h"

namespace Carbon::Check {

// Generates C++ file contents to #include all requested imports.
static auto GenerateCppIncludesHeaderCode(
    llvm::ArrayRef<std::pair<llvm::StringRef, SemIRLoc>> imports)
    -> std::string {
  std::string code;
  llvm::raw_string_ostream code_stream(code);
  for (const auto& [path, _] : imports) {
    code_stream << "#include \"";
    code_stream.write_escaped(path);
    code_stream << "\"\n";
  }
  return code;
}

auto ImportCppFiles(
    Context& context, llvm::StringRef importing_file_path,
    llvm::ArrayRef<std::pair<llvm::StringRef, SemIRLoc>> imports,
    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs) -> void {
  size_t num_imports = imports.size();
  if (num_imports == 0) {
    return;
  }

  // TODO: Use all import locations by referring each Clang diagnostic to the
  // relevant import.
  SemIRLoc loc = imports.back().second;

  std::string diagnostics_str;
  llvm::raw_string_ostream diagnostics_stream(diagnostics_str);

  llvm::IntrusiveRefCntPtr<clang::DiagnosticOptions> diagnostic_options(
      new clang::DiagnosticOptions());
  clang::TextDiagnosticPrinter diagnostics_consumer(diagnostics_stream,
                                                    diagnostic_options.get());
  // TODO: Share compilation flags with ClangRunner.
  auto ast = clang::tooling::buildASTFromCodeWithArgs(
      GenerateCppIncludesHeaderCode(imports), {},