[PATCH] D55510: [ExprConstant] Improve memchr/memcmp for type mismatch and multibyte element types

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 10 10:05:22 PST 2018


aaron.ballman added inline comments.


================
Comment at: lib/AST/ExprConstant.cpp:6151
+        Info.Ctx.getBaseElementType(getType(Result.getLValueBase()));
+    const bool IsRawByte = BuiltinOp == Builtin::BImemchr ||
+                           BuiltinOp == Builtin::BI__builtin_memchr;
----------------
Drop the top-level `const`.


================
Comment at: lib/AST/ExprConstant.cpp:8448
+
+    const bool IsRawByte = BuiltinOp == Builtin::BImemcmp ||
+                           BuiltinOp == Builtin::BI__builtin_memcmp;
----------------
Drop top-level `const`. (Same suggestion anywhere the type is local to a function and is not a pointer or reference type.)


================
Comment at: lib/AST/ExprConstant.cpp:8461
+      }
+      const auto CharTy1Width = Info.Ctx.getTypeSize(CharTy1);
+      // Give up on comparing between elements with disparate widths.
----------------
Don't use `auto` here as the type is not spelled out in the initialization.


================
Comment at: lib/AST/ExprConstant.cpp:8465
+        return false;
+      const auto LengthPerElement = CharTy1Width / Info.Ctx.getCharWidth();
+      assert(MaxLength);
----------------
Drop `const` and use a non-deduced type.


================
Comment at: lib/AST/ExprConstant.cpp:8466
+      const auto LengthPerElement = CharTy1Width / Info.Ctx.getCharWidth();
+      assert(MaxLength);
+      for (;;) {
----------------
Can you add a string literal to this assert explaining what's gone wrong if it triggers?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55510/new/

https://reviews.llvm.org/D55510





More information about the cfe-commits mailing list