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.