[PATCH] D115604: [Support] Expand `<CFGDIR>` as the base directory in response files.

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

The name of the patch could be more precise if you change `response files` to `configuration files`.



================
Comment at: clang/docs/UsersManual.rst:926-927
+
+A potential `<CFGDIR>` use-case may be search paths in a portable (i.e. not
+installed) SDK directory for embedded development. The user may only need to
+specify a root configuration file to establish every aspect of the SDK with the
----------------
It does not describe the use case clearly. Such macro is useful in the cases when configuration file is kept together with SDK content so that location of various SDK component can be expressed using the file location. Something about that could be provided here.


================
Comment at: llvm/include/llvm/Support/CommandLine.h:2117
+                         llvm::vfs::FileSystem &FS,
+                         StringRef ExpandBasePathToken = {});
 
----------------
It seems that this parameter can be boolean. It only instructs to call `ExpandBasePaths`, which knows what to expand.


================
Comment at: llvm/include/llvm/Support/CommandLine.h:2126
+    llvm::Optional<llvm::StringRef> CurrentDir = llvm::None,
+    StringRef ExpandBasePathToken = {});
 
----------------
The same about boolean parameter.


================
Comment at: llvm/lib/Support/CommandLine.cpp:1082
+// Substitute token with the file's base path.
+static void ExpandBasePaths(StringRef BasePath, StringRef Token,
+                            StringSaver &Saver, const char *&Arg) {
----------------
It seems that the parameter `Token` is not needed, as it is always `<CFGDIR>`.


================
Comment at: llvm/lib/Support/CommandLine.cpp:1118
+                                      llvm::vfs::FileSystem &FS,
+                                      StringRef ExpandBasePathToken) {
   assert(sys::path::is_absolute(FName));
----------------
It also could be boolean.


================
Comment at: llvm/lib/Support/CommandLine.cpp:1184
+                             llvm::vfs::FileSystem &FS,
+                             StringRef ExpandBasePathToken) {
   bool AllExpanded = true;
----------------
It could be boolean.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115604/new/

https://reviews.llvm.org/D115604