Generalization tool #805
Generalization tool #805
Conversation
|
ok good, I like that this is removing code from Solver.cpp |
| #include "llvm/Support/KnownBits.h" | ||
|
|
||
| #include "souper/Infer/Preconditions.h" | ||
|
|
There was a problem hiding this comment.
does this blank line among souper includes serve a purpose?
| Solver *S, ParsedReplacement Input) { | ||
|
|
||
| if (DebugLevel > 1) { | ||
| llvm::errs() << "Attempting to generalize by removing leaf.\n"; |
|
|
||
| std::set<Inst *> Visited; | ||
| while (!Stack.empty()) { | ||
| auto Current = Stack.back(); |
There was a problem hiding this comment.
indentation error.
if you're having trouble with basic code formatting, why not get into the habit of running clang-format on new files before contributing them?
There was a problem hiding this comment.
Good idea, I'll do them cmake tweaking required for running ninja format or make format.
There was a problem hiding this comment.
for now, please only do this on new files that are being added to the repo. doing this for existing files would be fun, but it would make it effectively impossible to ever merge any existing branch. we can only do this at a time where we're certain that no non-trivial branches exist. at least one exists right now, and probably more.
| @@ -0,0 +1,22 @@ | |||
| ; REQUIRES: solver, synthesis | |||
There was a problem hiding this comment.
since this test is just looking for a bunch of LGTMs out of a two-step process, it's not really possible to see what behavior it's actually testing. please modify it to only run your new tool, and then check that its output is correct without invoking a second tool, so we can look at the test and understand what's going on. also do this to the other test.
There was a problem hiding this comment.
OK, sounds good.
No description provided.