Details
Diff Detail
Event Timeline
The structure of this patch looks sensible. Some comments.
| lib/Transforms/Scalar/ConstantHoisting.cpp | ||
|---|---|---|
| 180 | hmm, is this clang-formatt'ed? | |
| 196 | Nice that you got rid of this weird setup() function. | |
| 290 | same (re clang-format), I could be wrong tho | |
| 500 | I prefer the idiom | |
| 534 | else after return | |
Thanks, Sean and Davide!
| lib/Transforms/Scalar/ConstantHoisting.cpp | ||
|---|---|---|
| 180 | Yes, it is. Looks like the usual format goes out of the 80-char bounds, and I guess this is the fallback. | |
| 290 | This one is a bit weirder, since the usual formatting fits (exactly), but this is what clang-format outputs. Possibly a clang-format bug... | |
| 500 | I don't really care either way, and we have both in the codebase. Since you felt strongly enough about it to comment, I don't mind changing. :-) | |
| 534 | Right, thanks! | |
Nice that you got rid of this weird setup() function.