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

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 3 18:30:34 PDT 2016


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 @@
  */
-LLVMBool LLVMPrintModuleToFile(LLVMModuleRef M, const char *Filename,
+bool LLVMPrintModuleToFile(LLVMModuleRef M, const char *Filename,
                                char **ErrorMessage);
----------------
Prazek wrote:
> deadalnix wrote:
> > Please keep LLVMBool in the C API. ABI change are a NO NO.
> As I said in my comment, I won't commit most of the changes. This review is only to show what check has found.
>From what I see in this patch, changing the function type to bool not a useful fix, and certainly a dangerous one, since 1. ABI changes, 2. The check can't see all the users of the function.

================
Comment at: lib/AST/Decl.cpp:173
@@ -172,4 +172,3 @@
 /// and if so, is it an explicit specialization?
-template <class T> static typename
-std::enable_if<!std::is_base_of<RedeclarableTemplateDecl, T>::value, bool>::type
+template <class T> static bool
 isExplicitMemberSpecialization(const T *D) {
----------------
Apparently, the check should filter out template instantiations.

================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:1934
@@ -1933,3 +1933,3 @@
   auto Int32Ty =
-      CGF.getContext().getIntTypeForBitwidth(/*DestWidth*/ 32, /*Signed*/ true);
+      CGF.getContext().getIntTypeForBitwidth(/*DestWidth*/ 32, /*Signed*/ 1);
   auto ThreadIDTemp = CGF.CreateMemTemp(Int32Ty, /*Name*/ ".threadid_temp.");
----------------
Looks like the parameter type should be changed to bool instead.

================
Comment at: lib/CodeGen/SplitKit.h:298
@@ -297,3 +297,3 @@
 
-  typedef PointerIntPair<VNInfo*, 1> ValueForcePair;
+  typedef PointerIntPair<VNInfo*, 1, bool> ValueForcePair;
   typedef DenseMap<std::pair<unsigned, unsigned>, ValueForcePair> ValueMap;
----------------
Is it an automated fix?

================
Comment at: lib/Target/ARM/ARMConstantIslandPass.cpp:1643
@@ -1642,3 +1642,3 @@
 bool ARMConstantIslands::removeUnusedCPEntries() {
-  unsigned MadeChange = false;
+  unsigned MadeChange = 0;
   for (unsigned i = 0, e = CPEntries.size(); i != e; ++i) {
----------------
In this case, changing the type would be better.

================
Comment at: utils/TableGen/CTagsEmitter.cpp:36
@@ -35,3 +35,3 @@
       : Id(&Name), Loc(Location) {}
-  int operator<(const Tag &B) const { return *Id < *B.Id; }
+  bool operator<(const Tag &B) const { return *Id < *B.Id; }
   void emit(raw_ostream &OS) const {
----------------
One of the few cases where changing the function return type makes sense.


http://reviews.llvm.org/D19105





More information about the cfe-commits mailing list