[PATCH] D23861: [LLVM] Fix some Clang-tidy modernize-use-using and Include What You Use warnings; other minor fixes

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 24 18:24:13 PDT 2016


Most of the changes in that patch look fine to me.

However IMO using newlines or not works well to indicate steps of an algorithm that belong together or put things apart. I like that fact being a conscious choice by the programmer rather than some mechanical rule that says you must have a newline in front of a loop. So for similar changes in the future I'd prefer it if this part of the formatting is left untouched.

- Matthias

> On Aug 24, 2016, at 6:17 PM, Eugene Zelenko via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Hi, Matthias!
> 
> I just made spacing consistent across file.
> 
> Eugene.
> 
> On Wed, Aug 24, 2016 at 6:14 PM, Matthias Braun <matze at braunis.de> wrote:
>> MatzeB added a subscriber: MatzeB.
>> 
>> ================
>> Comment at: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp:2001-2003
>> @@ -1945,4 +2000,5 @@
>>   // Read all the records for this value table.
>>   SmallString<128> ValueName;
>> -  while (1) {
>> +
>> +  while (true) {
>>     BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
>> ----------------
>> What's the deal with these gratuitous newlines in front of loops? I like putting declarations immediately in front of loops for temporary variables only used inside the loop or values that are computed in the loop...
>> 
>> 
>> Repository:
>>  rL LLVM
>> 
>> https://reviews.llvm.org/D23861
>> 
>> 
>> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list