[PATCH] D23765: Fix for clang PR 29087

Serge Pavlov via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 21 04:27:11 PDT 2016


sepavloff added inline comments.


================
Comment at: lib/Sema/SemaExprCXX.cpp:4231
+        const CXXConstructorDecl *Constructor = nullptr;
+        if (const ConstructorUsingShadowDecl *CSD =
+            dyn_cast<ConstructorUsingShadowDecl>(ND)) {
----------------
Use `auto` here. Type of `CSD` is clear from `dyn_cast`.


================
Comment at: lib/Sema/SemaExprCXX.cpp:4233
+            dyn_cast<ConstructorUsingShadowDecl>(ND)) {
+          assert(isa<CXXConstructorDecl>(CSD->getTargetDecl()));
+          Constructor = cast<CXXConstructorDecl>(CSD->getTargetDecl());
----------------
This `assert` is excessive. The subsequent `cast` makes this check.


================
Comment at: lib/Sema/SemaExprCXX.cpp:4239-4240
+            continue;
+        }
+        else
+          Constructor = cast<CXXConstructorDecl>(ND);
----------------
Put `else` on the same line as `}`.


================
Comment at: lib/Sema/SemaExprCXX.cpp:4281
+        const CXXConstructorDecl *Constructor = nullptr;
+        if (const ConstructorUsingShadowDecl *CSD =
+            dyn_cast<ConstructorUsingShadowDecl>(ND)) {
----------------
Use `auto` here.


================
Comment at: lib/Sema/SemaExprCXX.cpp:4283
+            dyn_cast<ConstructorUsingShadowDecl>(ND)) {
+          assert(isa<CXXConstructorDecl>(CSD->getTargetDecl()));
+          Constructor = cast<CXXConstructorDecl>(CSD->getTargetDecl());
----------------
The `assert` is excessive.


================
Comment at: lib/Sema/SemaExprCXX.cpp:4289-4290
+            continue;
+        }
+        else
+          Constructor = cast<CXXConstructorDecl>(ND);
----------------
Put `else` on the same line as `}`.


https://reviews.llvm.org/D23765





More information about the cfe-commits mailing list