Repository: carbon-language/carbon-lang
File: toolchain/base/install_paths.h (C/C++)

Code before:

#include <filesystem>

#include "common/error.h"
#include "common/filesystem.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Twine.h"
#include "toolchain/base/llvm_tools.h"

namespace Carbon {

// Locates the toolchain installation and provides paths to various components.
//
// The Carbon toolchain expects to be installed into a tree rooted at `root_`.
// This root contains the marker file and the busy box binary.
//
// In a Unix-like filesystem environment, the root is typically located as
// `<some prefix>/lib/carbon`, with symlinks in the other parts of the FHS-based
// layout back to entries below this tree. However, this class and the toolchain
// itself should only directly use things below the installation root to support
// non-FHS usage.
//
// When locating an install, we verify it with `CheckMarkerFile`. When errors
// occur, `SetError` makes `error()` available for diagnostics and clears the
// install prefix (leaving things minimally functional).
//
// The factory methods locate the install prefix based on their use-case:
//
//   - `MakeExeRelative` for command line tools in an install.
//   - `MakeForBazelRunfiles` for locating through Bazel's runfile tree.
//   - `Make` for an explicit path, for example in tests.
//
// An instance of this class provides methods that query for specific paths
// within the install. Note that we want to abstract away any platform
// differences in the installation layout. When a specific part of the install
// is needed, a dedicated accessor should be added that computes the path for
// that component.
//
// TODO: Need to check the installation structure of LLVM on Windows and figure
// out what Carbon's should be within a Windows prefix and how much of the
// structure we can share with the Unix-y layout of the prefix.
//
// TODO: InstallPaths is typically called from places using a VFS (both tests
// and the Driver), but does not use a VFS itself. It currently only supports
// using the real filesystem, but should probably support a VFS.
class InstallPaths {
 public:
  // Provide the current executable's path to detect the correct installation
  // root. This assumes the toolchain to be in its installed layout.
  //

Review comment (suggestion):
The comment on `root_` suggests that the root is cleared, not the prefix.

Is the empty path interpreted as a relative path to the CWD? If so, I think it'd be helpful to say that.

Code after:

#include <filesystem>

#include "common/error.h"
#include "common/filesystem.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Twine.h"
#include "toolchain/base/llvm_tools.h"

namespace Carbon {

// Locates the toolchain installation and provides paths to various components.
//
// The Carbon toolchain expects to be installed into a tree rooted at `root_`.
// This root contains the marker file and the busy box binary.
//
// In a Unix-like filesystem environment, the root is typically located as
// `<some prefix>/lib/carbon`, with symlinks in the other parts of the FHS-based
// layout back to entries below this tree. However, this class and the toolchain
// itself should only directly use things below the installation root to support
// non-FHS usage.
//
// When locating an install, we verify it with `CheckMarkerFile`. When errors
// occur, `SetError` makes `error()` available for diagnostics and clears the
// install root, leaving things minimally functional but with the installation
// root of the current working directory.
//
// The factory methods locate the install root based on their use-case:
//
//   - `MakeExeRelative` for command line tools in an install.
//   - `MakeForBazelRunfiles` for locating through Bazel's runfile tree.
//   - `Make` for an explicit path, for example in tests.
//
// An instance of this class provides methods that query for specific paths
// within the install. Note that we want to abstract away any platform
// differences in the installation layout. When a specific part of the install
// is needed, a dedicated accessor should be added that computes the path for
// that component.
//
// TODO: Need to check the installation structure of LLVM on Windows and figure
// out what Carbon's should be within a Windows prefix and how much of the
// structure we can share with the Unix-y layout of the prefix.
//
// TODO: InstallPaths is typically called from places using a VFS (both tests
// and the Driver), but does not use a VFS itself. It currently only supports
// using the real filesystem, but should probably support a VFS.
class InstallPaths {
 public:
  // Provide the current executable's path to detect the correct installation
  // root. This assumes the toolchain to be in its installed layout.