[PATCH] D24933: Enable configuration files in clang

Serge Pavlov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 26 06:26:55 PDT 2017


sepavloff marked 4 inline comments as done.
sepavloff added inline comments.


================
Comment at: lib/Driver/Driver.cpp:637
+  }
+  FilePath.clear();
+  return false;
----------------
hfinkel wrote:
> Why do we do clear FilePath here?
The intent was to avoid copying file name buffer. It however makes code less readable. Used temporary buffer instead.


================
Comment at: lib/Driver/Driver.cpp:657
+                          StringRef FileName) {
+  FilePath.clear();
+  if (searchDirectoriesForFile(FilePath, FileName, Dirs))
----------------
hfinkel wrote:
> Is this necessary? I'd rather have a contract for these functions be that they always clear the FilePath if necessary, and then the caller will not need to worry about it.
Agree. Changed accordingly.


================
Comment at: lib/Driver/Driver.cpp:661
+
+  // If not found, try searching the directory where executable resides.
+  FilePath.clear();
----------------
hfinkel wrote:
> hfinkel wrote:
> > Why not just add this directory to the end of the list of directories?
> (by which I mean, why not add BinDirectory to the list of directories instead of having special code for it?)
Now BinDirectory is added to the list of directories.


https://reviews.llvm.org/D24933





More information about the cfe-commits mailing list