[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.
This revision was automatically updated to reflect the committed changes. Closed by commit rL343892: Emit CK_NoOp casts in C mode, not just C++. (authored by jyknight, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D52918?vs=168477&id=168542#toc Repository: rL LLVM https://reviews.llvm.org/D52918 Files: cfe/trunk/lib/AST/ExprConstant.cpp cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/test/Sema/c-casts.c Index: cfe/trunk/lib/AST/ExprConstant.cpp === --- cfe/trunk/lib/AST/ExprConstant.cpp +++ cfe/trunk/lib/AST/ExprConstant.cpp @@ -5864,11 +5864,7 @@ // permitted in constant expressions in C++11. Bitcasts from cv void* are // also static_casts, but we disallow them as a resolution to DR1312. if (!E->getType()->isVoidPointerType()) { - // If we changed anything other than cvr-qualifiers, we can't use this - // value for constant folding. FIXME: Qualification conversions should - // always be CK_NoOp, but we get this wrong in C. - if (!Info.Ctx.hasCvrSimilarType(E->getType(), E->getSubExpr()->getType())) -Result.Designator.setInvalid(); + Result.Designator.setInvalid(); if (SubExpr->getType()->isVoidPointerType()) CCEDiag(E, diag::note_constexpr_invalid_cast) << 3 << SubExpr->getType(); Index: cfe/trunk/lib/Sema/SemaExpr.cpp === --- cfe/trunk/lib/Sema/SemaExpr.cpp +++ cfe/trunk/lib/Sema/SemaExpr.cpp @@ -5862,6 +5862,8 @@ LangAS DestAS = DestTy->getPointeeType().getAddressSpace(); if (SrcAS != DestAS) return CK_AddressSpaceConversion; + if (Context.hasCvrSimilarType(SrcTy, DestTy)) +return CK_NoOp; return CK_BitCast; } case Type::STK_BlockPointer: @@ -7762,7 +7764,12 @@ if (isa(RHSType)) { LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace(); LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace(); - Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast; + if (AddrSpaceL != AddrSpaceR) +Kind = CK_AddressSpaceConversion; + else if (Context.hasCvrSimilarType(RHSType, LHSType)) +Kind = CK_NoOp; + else +Kind = CK_BitCast; return checkPointerTypesForAssignment(*this, LHSType, RHSType); } Index: cfe/trunk/test/Sema/c-casts.c === --- cfe/trunk/test/Sema/c-casts.c +++ cfe/trunk/test/Sema/c-casts.c @@ -0,0 +1,25 @@ +// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s + +// The cast construction code both for implicit and c-style casts is very +// different in C vs C++. This file is intended to test the C behavior. + +// TODO: add tests covering the rest of the code in +// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast + +// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer +void cast_cvr_pointer(char volatile * __restrict * const * p) { + char*** x; + // CHECK: ImplicitCastExpr {{.*}} 'char ***' + x = p; + // CHECK: CStyleCastExpr {{.*}} 'char ***' + x = (char***)p; +} + +// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type +void cast_pointer_type(char *p) { + void *x; + // CHECK: ImplicitCastExpr {{.*}} 'void *' + x = p; + // CHECK: CStyleCastExpr {{.*}} 'void *' + x = (void*)p; +} Index: cfe/trunk/lib/AST/ExprConstant.cpp === --- cfe/trunk/lib/AST/ExprConstant.cpp +++ cfe/trunk/lib/AST/ExprConstant.cpp @@ -5864,11 +5864,7 @@ // permitted in constant expressions in C++11. Bitcasts from cv void* are // also static_casts, but we disallow them as a resolution to DR1312. if (!E->getType()->isVoidPointerType()) { - // If we changed anything other than cvr-qualifiers, we can't use this - // value for constant folding. FIXME: Qualification conversions should - // always be CK_NoOp, but we get this wrong in C. - if (!Info.Ctx.hasCvrSimilarType(E->getType(), E->getSubExpr()->getType())) -Result.Designator.setInvalid(); + Result.Designator.setInvalid(); if (SubExpr->getType()->isVoidPointerType()) CCEDiag(E, diag::note_constexpr_invalid_cast) << 3 << SubExpr->getType(); Index: cfe/trunk/lib/Sema/SemaExpr.cpp === --- cfe/trunk/lib/Sema/SemaExpr.cpp +++ cfe/trunk/lib/Sema/SemaExpr.cpp @@ -5862,6 +5862,8 @@ LangAS DestAS = DestTy->getPointeeType().getAddressSpace(); if (SrcAS != DestAS) return CK_AddressSpaceConversion; + if (Context.hasCvrSimilarType(SrcTy, DestTy)) +return CK_NoOp; return CK_BitCast; } case Type::STK_BlockPointer: @@ -7762,7 +7764,12 @@ if (isa(RHSType)) { LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace()
[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.
This revision was automatically updated to reflect the committed changes. Closed by commit rC343892: Emit CK_NoOp casts in C mode, not just C++. (authored by jyknight, committed by ). Changed prior to commit: https://reviews.llvm.org/D52918?vs=168477&id=168541#toc Repository: rL LLVM https://reviews.llvm.org/D52918 Files: lib/AST/ExprConstant.cpp lib/Sema/SemaExpr.cpp test/Sema/c-casts.c Index: test/Sema/c-casts.c === --- test/Sema/c-casts.c +++ test/Sema/c-casts.c @@ -0,0 +1,25 @@ +// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s + +// The cast construction code both for implicit and c-style casts is very +// different in C vs C++. This file is intended to test the C behavior. + +// TODO: add tests covering the rest of the code in +// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast + +// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer +void cast_cvr_pointer(char volatile * __restrict * const * p) { + char*** x; + // CHECK: ImplicitCastExpr {{.*}} 'char ***' + x = p; + // CHECK: CStyleCastExpr {{.*}} 'char ***' + x = (char***)p; +} + +// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type +void cast_pointer_type(char *p) { + void *x; + // CHECK: ImplicitCastExpr {{.*}} 'void *' + x = p; + // CHECK: CStyleCastExpr {{.*}} 'void *' + x = (void*)p; +} Index: lib/AST/ExprConstant.cpp === --- lib/AST/ExprConstant.cpp +++ lib/AST/ExprConstant.cpp @@ -5864,11 +5864,7 @@ // permitted in constant expressions in C++11. Bitcasts from cv void* are // also static_casts, but we disallow them as a resolution to DR1312. if (!E->getType()->isVoidPointerType()) { - // If we changed anything other than cvr-qualifiers, we can't use this - // value for constant folding. FIXME: Qualification conversions should - // always be CK_NoOp, but we get this wrong in C. - if (!Info.Ctx.hasCvrSimilarType(E->getType(), E->getSubExpr()->getType())) -Result.Designator.setInvalid(); + Result.Designator.setInvalid(); if (SubExpr->getType()->isVoidPointerType()) CCEDiag(E, diag::note_constexpr_invalid_cast) << 3 << SubExpr->getType(); Index: lib/Sema/SemaExpr.cpp === --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -5862,6 +5862,8 @@ LangAS DestAS = DestTy->getPointeeType().getAddressSpace(); if (SrcAS != DestAS) return CK_AddressSpaceConversion; + if (Context.hasCvrSimilarType(SrcTy, DestTy)) +return CK_NoOp; return CK_BitCast; } case Type::STK_BlockPointer: @@ -7762,7 +7764,12 @@ if (isa(RHSType)) { LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace(); LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace(); - Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast; + if (AddrSpaceL != AddrSpaceR) +Kind = CK_AddressSpaceConversion; + else if (Context.hasCvrSimilarType(RHSType, LHSType)) +Kind = CK_NoOp; + else +Kind = CK_BitCast; return checkPointerTypesForAssignment(*this, LHSType, RHSType); } Index: test/Sema/c-casts.c === --- test/Sema/c-casts.c +++ test/Sema/c-casts.c @@ -0,0 +1,25 @@ +// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s + +// The cast construction code both for implicit and c-style casts is very +// different in C vs C++. This file is intended to test the C behavior. + +// TODO: add tests covering the rest of the code in +// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast + +// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer +void cast_cvr_pointer(char volatile * __restrict * const * p) { + char*** x; + // CHECK: ImplicitCastExpr {{.*}} 'char ***' + x = p; + // CHECK: CStyleCastExpr {{.*}} 'char ***' + x = (char***)p; +} + +// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type +void cast_pointer_type(char *p) { + void *x; + // CHECK: ImplicitCastExpr {{.*}} 'void *' + x = p; + // CHECK: CStyleCastExpr {{.*}} 'void *' + x = (void*)p; +} Index: lib/AST/ExprConstant.cpp === --- lib/AST/ExprConstant.cpp +++ lib/AST/ExprConstant.cpp @@ -5864,11 +5864,7 @@ // permitted in constant expressions in C++11. Bitcasts from cv void* are // also static_casts, but we disallow them as a resolution to DR1312. if (!E->getType()->isVoidPointerType()) { - // If we changed anything other than cvr-qualifiers, we can't use this - // value for constant folding. FIXME: Qualification conversions should - // always be CK_NoOp, but we get this wrong in C. - if (!Info.Ctx.hasCvrSimilarType(E->getType(), E->getSubExpr()->getType())) -Result.Designator.setInvalid(); + Resu
[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you for adding the test and the TODO! https://reviews.llvm.org/D52918 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.
jyknight updated this revision to Diff 168477. jyknight added a comment. Added test. https://reviews.llvm.org/D52918 Files: clang/lib/AST/ExprConstant.cpp clang/lib/Sema/SemaExpr.cpp clang/test/Sema/c-casts.c Index: clang/test/Sema/c-casts.c === --- /dev/null +++ clang/test/Sema/c-casts.c @@ -0,0 +1,25 @@ +// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s + +// The cast construction code both for implicit and c-style casts is very +// different in C vs C++. This file is intended to test the C behavior. + +// TODO: add tests covering the rest of the code in +// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast + +// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer +void cast_cvr_pointer(char volatile * __restrict * const * p) { + char*** x; + // CHECK: ImplicitCastExpr {{.*}} 'char ***' + x = p; + // CHECK: CStyleCastExpr {{.*}} 'char ***' + x = (char***)p; +} + +// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type +void cast_pointer_type(char *p) { + void *x; + // CHECK: ImplicitCastExpr {{.*}} 'void *' + x = p; + // CHECK: CStyleCastExpr {{.*}} 'void *' + x = (void*)p; +} Index: clang/lib/Sema/SemaExpr.cpp === --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -5862,6 +5862,8 @@ LangAS DestAS = DestTy->getPointeeType().getAddressSpace(); if (SrcAS != DestAS) return CK_AddressSpaceConversion; + if (Context.hasCvrSimilarType(SrcTy, DestTy)) +return CK_NoOp; return CK_BitCast; } case Type::STK_BlockPointer: @@ -7762,7 +7764,12 @@ if (isa(RHSType)) { LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace(); LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace(); - Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast; + if (AddrSpaceL != AddrSpaceR) +Kind = CK_AddressSpaceConversion; + else if (Context.hasCvrSimilarType(RHSType, LHSType)) +Kind = CK_NoOp; + else +Kind = CK_BitCast; return checkPointerTypesForAssignment(*this, LHSType, RHSType); } Index: clang/lib/AST/ExprConstant.cpp === --- clang/lib/AST/ExprConstant.cpp +++ clang/lib/AST/ExprConstant.cpp @@ -5861,11 +5861,7 @@ // permitted in constant expressions in C++11. Bitcasts from cv void* are // also static_casts, but we disallow them as a resolution to DR1312. if (!E->getType()->isVoidPointerType()) { - // If we changed anything other than cvr-qualifiers, we can't use this - // value for constant folding. FIXME: Qualification conversions should - // always be CK_NoOp, but we get this wrong in C. - if (!Info.Ctx.hasCvrSimilarType(E->getType(), E->getSubExpr()->getType())) -Result.Designator.setInvalid(); + Result.Designator.setInvalid(); if (SubExpr->getType()->isVoidPointerType()) CCEDiag(E, diag::note_constexpr_invalid_cast) << 3 << SubExpr->getType(); Index: clang/test/Sema/c-casts.c === --- /dev/null +++ clang/test/Sema/c-casts.c @@ -0,0 +1,25 @@ +// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s + +// The cast construction code both for implicit and c-style casts is very +// different in C vs C++. This file is intended to test the C behavior. + +// TODO: add tests covering the rest of the code in +// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast + +// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer +void cast_cvr_pointer(char volatile * __restrict * const * p) { + char*** x; + // CHECK: ImplicitCastExpr {{.*}} 'char ***' + x = p; + // CHECK: CStyleCastExpr {{.*}} 'char ***' + x = (char***)p; +} + +// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type +void cast_pointer_type(char *p) { + void *x; + // CHECK: ImplicitCastExpr {{.*}} 'void *' + x = p; + // CHECK: CStyleCastExpr {{.*}} 'void *' + x = (void*)p; +} Index: clang/lib/Sema/SemaExpr.cpp === --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -5862,6 +5862,8 @@ LangAS DestAS = DestTy->getPointeeType().getAddressSpace(); if (SrcAS != DestAS) return CK_AddressSpaceConversion; + if (Context.hasCvrSimilarType(SrcTy, DestTy)) +return CK_NoOp; return CK_BitCast; } case Type::STK_BlockPointer: @@ -7762,7 +7764,12 @@ if (isa(RHSType)) { LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace(); LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace(); - Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast; + if (AddrSpaceL != AddrSpaceR) +Kind = CK_AddressSpaceConversion; + else if (Context.hasCvrSimilarType(RHSTyp
[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.
jyknight added a comment. In https://reviews.llvm.org/D52918#1256420, @aaron.ballman wrote: > Patch is missing tests -- perhaps you could dump the AST and check the > casting notation from the output? It would appear that which casts get emitted in C mode is almost completely untested. I added tests just for this change in a new file, and left a TODO to fill it out for the remainder of those functions. https://reviews.llvm.org/D52918 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.
aaron.ballman added a comment. Patch is missing tests -- perhaps you could dump the AST and check the casting notation from the output? https://reviews.llvm.org/D52918 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.
jyknight created this revision. jyknight added a reviewer: rsmith. Previously, it had been using CK_BitCast even for casts that only change const/restrict/volatile. Now it will use CK_Noop where appropriate. This is an alternate solution to r336746. https://reviews.llvm.org/D52918 Files: clang/lib/AST/ExprConstant.cpp clang/lib/Sema/SemaExpr.cpp Index: clang/lib/Sema/SemaExpr.cpp === --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -5846,7 +5846,9 @@ // pointers. Everything else should be possible. QualType SrcTy = Src.get()->getType(); - if (Context.hasSameUnqualifiedType(SrcTy, DestTy)) + // We can use a no-op cast if the types are the same, or only adding/removing + // const, volatile, or restricted qualifiers. + if (Context.hasCvrSimilarType(SrcTy, DestTy)) return CK_NoOp; switch (Type::ScalarTypeKind SrcKind = SrcTy->getScalarTypeKind()) { @@ -7762,7 +7764,12 @@ if (isa(RHSType)) { LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace(); LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace(); - Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast; + if (AddrSpaceL != AddrSpaceR) +Kind = CK_AddressSpaceConversion; + else if (Context.hasCvrSimilarType(RHSType, LHSType)) +Kind = CK_NoOp; + else +Kind = CK_BitCast; return checkPointerTypesForAssignment(*this, LHSType, RHSType); } Index: clang/lib/AST/ExprConstant.cpp === --- clang/lib/AST/ExprConstant.cpp +++ clang/lib/AST/ExprConstant.cpp @@ -5861,11 +5861,7 @@ // permitted in constant expressions in C++11. Bitcasts from cv void* are // also static_casts, but we disallow them as a resolution to DR1312. if (!E->getType()->isVoidPointerType()) { - // If we changed anything other than cvr-qualifiers, we can't use this - // value for constant folding. FIXME: Qualification conversions should - // always be CK_NoOp, but we get this wrong in C. - if (!Info.Ctx.hasCvrSimilarType(E->getType(), E->getSubExpr()->getType())) -Result.Designator.setInvalid(); + Result.Designator.setInvalid(); if (SubExpr->getType()->isVoidPointerType()) CCEDiag(E, diag::note_constexpr_invalid_cast) << 3 << SubExpr->getType(); Index: clang/lib/Sema/SemaExpr.cpp === --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -5846,7 +5846,9 @@ // pointers. Everything else should be possible. QualType SrcTy = Src.get()->getType(); - if (Context.hasSameUnqualifiedType(SrcTy, DestTy)) + // We can use a no-op cast if the types are the same, or only adding/removing + // const, volatile, or restricted qualifiers. + if (Context.hasCvrSimilarType(SrcTy, DestTy)) return CK_NoOp; switch (Type::ScalarTypeKind SrcKind = SrcTy->getScalarTypeKind()) { @@ -7762,7 +7764,12 @@ if (isa(RHSType)) { LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace(); LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace(); - Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast; + if (AddrSpaceL != AddrSpaceR) +Kind = CK_AddressSpaceConversion; + else if (Context.hasCvrSimilarType(RHSType, LHSType)) +Kind = CK_NoOp; + else +Kind = CK_BitCast; return checkPointerTypesForAssignment(*this, LHSType, RHSType); } Index: clang/lib/AST/ExprConstant.cpp === --- clang/lib/AST/ExprConstant.cpp +++ clang/lib/AST/ExprConstant.cpp @@ -5861,11 +5861,7 @@ // permitted in constant expressions in C++11. Bitcasts from cv void* are // also static_casts, but we disallow them as a resolution to DR1312. if (!E->getType()->isVoidPointerType()) { - // If we changed anything other than cvr-qualifiers, we can't use this - // value for constant folding. FIXME: Qualification conversions should - // always be CK_NoOp, but we get this wrong in C. - if (!Info.Ctx.hasCvrSimilarType(E->getType(), E->getSubExpr()->getType())) -Result.Designator.setInvalid(); + Result.Designator.setInvalid(); if (SubExpr->getType()->isVoidPointerType()) CCEDiag(E, diag::note_constexpr_invalid_cast) << 3 << SubExpr->getType(); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits