[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