alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Thank you for the fix! A few comments below.
================
Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:291-299
const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath());
- if (!Entry && Warned.insert(R.getFilePath()).second) {
- errs() << "Described file '" << R.getFilePath()
- << "' doesn't exist. Ignoring...\n";
+ if (!Entry) {
+ if (Warned.insert(R.getFilePath()).second) {
+ errs() << "Described file '" << R.getFilePath()
+ << "' doesn't exist. Ignoring...\n";
+ }
continue;
----------------
I'd swap the positive and the negative branches and remove `continue`:
if (const FileEntry *Entry = ...) {
GroupedReplacements[Entry].push_back(R);
} else if (...) {
errs() << ...;
}
Same below.
================
Comment at: test/clang-apply-replacements/invalid-files.cpp:1
+// RUN: mkdir -p %T/Inputs/invalid-files
+// RUN: clang-apply-replacements %T/Inputs/invalid-files
----------------
No need for `Inputs` in the path inside the build directory.
================
Comment at: test/clang-apply-replacements/invalid-files.cpp:5-7
+// RUN: ls -1 %T/Inputs/invalid-files | FileCheck %s --check-prefix=YAML
+//
+//// YAML: {{.+\.yaml$}}
----------------
FileCheck is not needed here. Just `// RUN: ls
%T/invalid-files/invalid-files.yaml` should be enough to assert the file
existence.
https://reviews.llvm.org/D35194
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
