Page MenuHomePhabricator

FileCheck Enhancement - Including files
AbandonedPublic

Authored by eklepilkina on Jul 14 2016, 3:04 AM.

Details

Summary

This patch adds new directive CHECK-INCLUDE - directive to include other file with checks to another.

Diff Detail

Event Timeline

eklepilkina retitled this revision from to FileCheck Enhancement - Including files.
eklepilkina updated this object.
eklepilkina changed the edit policy from "All Users" to "Administrators".
eklepilkina added a subscriber: vsk.
eklepilkina updated this object.Jul 14 2016, 3:16 AM
vsk added a comment.Jul 15 2016, 3:06 PM

Looks super useful :). I have some initial comments -- please clang-format and upload a diff with more context and I'll take a closer look.

utils/FileCheck/FileCheck.cpp
38

This seems a bit arbitrary. Why not use a DFS? E.g;

containsIncludeCycle(MainFile, IncludeFile) {
  if (Includes[IncludeFile].contains(MainFile)) return true
  for (Include in Includes[IncludeFile]) if (containsIncludeCycle(Include, MainFile)) return true
  return false
}
990

const std::string &Directory to avoid a copy.

991

Use llvm::sys::path::append. Also note that in this line, you're taking a reference to a stack temporary (the result of the std::string concatenation should be destroyed after this line). This looks like a use-after-free.

1003

Is there some way to avoid moving the code into this else branch? If not that's OK. Otherwise, it would really help to keep unmodified code out of this diff (makes it easier to inspect the diff).

eklepilkina marked 2 inline comments as done.Jul 18 2016, 5:04 AM
eklepilkina added inline comments.
utils/FileCheck/FileCheck.cpp
38

May be I didn't understand something, but as I understood next situation will fail. Am I wrong?

1->2;
1->3;
3->3;
2->4;
2->5;
4->1;
5->2;

1003

I understood that it'll be easier to merge, but I don't see correct way to avoid branching.

eklepilkina abandoned this revision.Jul 18 2016, 10:47 PM