Re: r345211 - [SemaCXX] Unconfuse Clang when std::align_val_t is unscoped in C++03

2018-10-24 Thread Richard Smith via cfe-commits
On Wed, 24 Oct 2018 at 15:40, Eric Fiselier via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: ericwf
> Date: Wed Oct 24 15:38:49 2018
> New Revision: 345211
>
> URL: http://llvm.org/viewvc/llvm-project?rev=345211=rev
> Log:
> [SemaCXX] Unconfuse Clang when std::align_val_t is unscoped in C++03
>
> Summary:
> When -faligned-allocation is specified in C++03 libc++ defines
> std::align_val_t as an unscoped enumeration type (because Clang didn't
> provide scoped enumerations as an extension until 8.0).
> Unfortunately Clang confuses the `align_val_t` overloads of delete with
> the sized deallocation overloads which aren't enabled. This caused Clang to
> call the aligned deallocation function as if it were the sized deallocation
> overload.
>
> For example: https://godbolt.org/z/xXJELh
>
> This patch fixes the confusion.
>
> Reviewers: rsmith, EricWF
>
> Reviewed By: EricWF
>
> Subscribers: cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D53508
>
> Added:
> cfe/trunk/test/SemaCXX/cxx03-aligned-allocation-unscoped-enum.cpp
> Modified:
> cfe/trunk/lib/Sema/SemaExprCXX.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=345211=345210=345211=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Oct 24 15:38:49 2018
> @@ -1515,8 +1515,9 @@ namespace {
>if (FD->getNumParams() == NumBaseParams + 2)
>  HasAlignValT = HasSizeT = true;
>else if (FD->getNumParams() == NumBaseParams + 1) {
> -HasSizeT =
> FD->getParamDecl(NumBaseParams)->getType()->isIntegerType();
> -HasAlignValT = !HasSizeT;
> +QualType ParamTy = FD->getParamDecl(NumBaseParams)->getType();
> +HasAlignValT = ParamTy->isAlignValT();
> +HasSizeT = !HasAlignValT && ParamTy->isIntegerType();
>

The isIntegerType() check here is redundant, and should probably be an
assert. If we think we have a usual deallocation function but the parameter
is neither align_val_t nor an integral type, something has gone wrong.


>}
>
>// In CUDA, determine how much we'd like / dislike to call this.
>
> Added: cfe/trunk/test/SemaCXX/cxx03-aligned-allocation-unscoped-enum.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx03-aligned-allocation-unscoped-enum.cpp?rev=345211=auto
>
> ==
> --- cfe/trunk/test/SemaCXX/cxx03-aligned-allocation-unscoped-enum.cpp
> (added)
> +++ cfe/trunk/test/SemaCXX/cxx03-aligned-allocation-unscoped-enum.cpp Wed
> Oct 24 15:38:49 2018
> @@ -0,0 +1,21 @@
> +// RUN: %clang_cc1 -std=c++03 %s -faligned-allocation \
> +// RUN:   -emit-llvm -o - | FileCheck %s
> +
> +// Ensure Clang doesn't confuse std::align_val_t with the sized
> deallocation
> +// parameter when the enum type is unscoped. Libc++ does this in C++03 in
> order
> +// to support aligned allocation in that dialect.
> +
> +using size_t = __decltype(sizeof(0));
> +
> +namespace std {
> +enum align_val_t { zero = size_t(0),
> +   max = size_t(-1) };
> +}
> +
> +// CHECK-LABEL: define void @_Z1fPi(
> +void f(int *p) {
> +  // CHECK-NOT: call void @_ZdlPvSt11align_val_t(
> +  // CHECK: call void @_ZdlPv(
> +  // CHECK: ret void
> +  delete p;
> +}
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r345211 - [SemaCXX] Unconfuse Clang when std::align_val_t is unscoped in C++03

2018-10-24 Thread Eric Fiselier via cfe-commits
Author: ericwf
Date: Wed Oct 24 15:38:49 2018
New Revision: 345211

URL: http://llvm.org/viewvc/llvm-project?rev=345211=rev
Log:
[SemaCXX] Unconfuse Clang when std::align_val_t is unscoped in C++03

Summary:
When -faligned-allocation is specified in C++03 libc++ defines std::align_val_t 
as an unscoped enumeration type (because Clang didn't provide scoped 
enumerations as an extension until 8.0).
Unfortunately Clang confuses the `align_val_t` overloads of delete with the 
sized deallocation overloads which aren't enabled. This caused Clang to call 
the aligned deallocation function as if it were the sized deallocation overload.

For example: https://godbolt.org/z/xXJELh

This patch fixes the confusion.

Reviewers: rsmith, EricWF

Reviewed By: EricWF

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D53508

Added:
cfe/trunk/test/SemaCXX/cxx03-aligned-allocation-unscoped-enum.cpp
Modified:
cfe/trunk/lib/Sema/SemaExprCXX.cpp

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=345211=345210=345211=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Oct 24 15:38:49 2018
@@ -1515,8 +1515,9 @@ namespace {
   if (FD->getNumParams() == NumBaseParams + 2)
 HasAlignValT = HasSizeT = true;
   else if (FD->getNumParams() == NumBaseParams + 1) {
-HasSizeT = FD->getParamDecl(NumBaseParams)->getType()->isIntegerType();
-HasAlignValT = !HasSizeT;
+QualType ParamTy = FD->getParamDecl(NumBaseParams)->getType();
+HasAlignValT = ParamTy->isAlignValT();
+HasSizeT = !HasAlignValT && ParamTy->isIntegerType();
   }
 
   // In CUDA, determine how much we'd like / dislike to call this.

Added: cfe/trunk/test/SemaCXX/cxx03-aligned-allocation-unscoped-enum.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx03-aligned-allocation-unscoped-enum.cpp?rev=345211=auto
==
--- cfe/trunk/test/SemaCXX/cxx03-aligned-allocation-unscoped-enum.cpp (added)
+++ cfe/trunk/test/SemaCXX/cxx03-aligned-allocation-unscoped-enum.cpp Wed Oct 
24 15:38:49 2018
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -std=c++03 %s -faligned-allocation \
+// RUN:   -emit-llvm -o - | FileCheck %s
+
+// Ensure Clang doesn't confuse std::align_val_t with the sized deallocation
+// parameter when the enum type is unscoped. Libc++ does this in C++03 in order
+// to support aligned allocation in that dialect.
+
+using size_t = __decltype(sizeof(0));
+
+namespace std {
+enum align_val_t { zero = size_t(0),
+   max = size_t(-1) };
+}
+
+// CHECK-LABEL: define void @_Z1fPi(
+void f(int *p) {
+  // CHECK-NOT: call void @_ZdlPvSt11align_val_t(
+  // CHECK: call void @_ZdlPv(
+  // CHECK: ret void
+  delete p;
+}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits