[PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

Prazek abandoned this revision. Prazek added inline comments. Comment at: lib/CodeGen/SplitKit.h:298 - typedef PointerIntPair ValueForcePair; + typedef PointerIntPair ValueForcePair; typedef DenseMap,

Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

alexfh added a comment. Most of the fixits are still useless, but I didn't notice any false positives (i.e. all warnings would be useful to some degree). So I suggest disabling the fixes completely at least for now. Comment at: include/llvm-c/Core.h:604 @@ -603,3 +603,3 @@

Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

deadalnix requested changes to this revision. This revision now requires changes to proceed. Comment at: include/llvm-c/Core.h:604 @@ -603,3 +603,3 @@ */ -LLVMBool LLVMPrintModuleToFile(LLVMModuleRef M, const char *Filename, +bool LLVMPrintModuleToFile(LLVMModuleRef M, const

Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

Prazek added inline comments. Comment at: include/llvm-c/Core.h:604 @@ -603,3 +603,3 @@ */ -LLVMBool LLVMPrintModuleToFile(LLVMModuleRef M, const char *Filename, +bool LLVMPrintModuleToFile(LLVMModuleRef M, const char *Filename, char

Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

alexfh added a comment. In http://reviews.llvm.org/D19105#427046, @Prazek wrote: > In http://reviews.llvm.org/D19105#426903, @alexfh wrote: > > > returning a bool from a function that is declared to return a typedef to an > > integral type that contains `bool` in its name (e.g. `LLVMBool`), and

Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

Prazek added inline comments. Comment at: lib/Target/Hexagon/MCTargetDesc/HexagonMCShuffler.cpp:194 @@ -193,3 +193,3 @@ - if (doneShuffling == false) { + if (doneShuffling == 0) { HexagonMCShuffler MCS(MCII, STI, MCB); alexfh wrote: > This is wrong. Of

Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

Prazek added a comment. In http://reviews.llvm.org/D19105#426903, @alexfh wrote: > returning a bool from a function that is declared to return a typedef to an > integral type that contains `bool` in its name (e.g. `LLVMBool`), and maybe > some other cases. Isn't it LLVMBool issue? I won't

Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. The bottom line is that the fixit in its current form is not particularly useful in most cases. We should try to detect some cases where the replacement is unlikely to be useful,

Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

alexfh added a comment. Too much noise here as well. Particularly, `someBool == true` should not be changed to `someBool == 1`. Please fix the check and update the results. Comment at: clang-tidy/readability/ImplicitBoolCastCheck.cpp:253 @@ -252,3 +252,3 @@

Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

Prazek added a comment. In http://reviews.llvm.org/D19105#422702, @Quuxplusone wrote: > It seems like this proposed diagnostic and fixit, statistically speaking, is > *never* correct. > In the cases where there is a code issue to be corrected, the diagnosable > issue really seems to involve

Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

jfb added inline comments. Comment at: lib/Transforms/IPO/MergeFunctions.cpp:147 @@ -146,3 +146,3 @@ struct Config : ValueMapConfig { -enum { FollowRAUW = false }; +enum { FollowRAUW = 0 }; }; This change should go to ValueMap, I think

Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

Quuxplusone added a subscriber: Quuxplusone. Quuxplusone added a comment. It seems like this proposed diagnostic and fixit, statistically speaking, is *never* correct. In the cases where there is a code issue to be corrected, the diagnosable issue really seems to involve dataflow analysis:

Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

Prazek updated this revision to Diff 56250. Prazek added a comment. Herald added a reviewer: tstellarAMD. Herald added subscribers: jfb, mzolotukhin, dsanders, arsenm, MatzeB. It seems that is doing it's work right now. I will have to change some places to post it to llvm like: - changing

Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

alexfh added a comment. Ignoring 1-bit wide bitfields seems like a reasonable thing to do for the check. Most of the changes I see here are making the code less idiomatic, I'd say. http://reviews.llvm.org/D19105 ___ cfe-commits mailing list

Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

thakis added a subscriber: thakis. thakis added a comment. > I want to replace all unsigned that are 1 bits with bool. MSVC only packs bitfields of the same type together, so doing that change would make clang use much more memory on Windows. http://reviews.llvm.org/D19105