Skip to content
This repository has been archived by the owner on Apr 23, 2020. It is now read-only.

Commit

Permalink
DR583, DR1512: Implement a rewrite to C++'s 'composite pointer type' …
Browse files Browse the repository at this point in the history 
…rules.

This has two significant effects:

1) Direct relational comparisons between null pointer constants (0 and nullopt)
   and pointers are now ill-formed. This was always the case for C, and it
   appears that C++ only ever permitted by accident. For instance, cases like
     nullptr < &a
   are now rejected.

2) Comparisons and conditional operators between differently-cv-qualified
   pointer types now work, and produce a composite type that both source
   pointer types can convert to (when possible). For instance, comparison
   between 'int **' and 'const int **' is now valid, and uses an intermediate
   type of 'const int *const *'.

Clang previously supported #2 as an extension.

We do not accept the cases in #1 as an extension. I've tested a fair amount of
code to check that this doesn't break it, but if it turns out that someone is
relying on this, we can easily add it back as an extension.

This is a re-commit of r284800.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@284890 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
@zygoloid
zygoloid committed Oct 21, 2016
1 parent a663b0a commit 4b6ad14
Show file tree
Hide file tree
Showing 29 changed files with 574 additions and 216 deletions.
8 changes: 2 additions & 6 deletions include/clang/Basic/DiagnosticSemaKinds.td
View file
Expand Up @@ -5536,6 +5536,8 @@ def ext_typecheck_ordered_comparison_of_pointer_integer : ExtWarn<
"ordered comparison between pointer and integer (%0 and %1)">;
def ext_typecheck_ordered_comparison_of_pointer_and_zero : Extension<
"ordered comparison between pointer and zero (%0 and %1) is an extension">;
def err_typecheck_ordered_comparison_of_pointer_and_zero : Error<
"ordered comparison between pointer and zero (%0 and %1)">;
def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
"ordered comparison of function pointers (%0 and %1)">;
def ext_typecheck_comparison_of_fptr_to_void : Extension<
Expand All @@ -5556,9 +5558,6 @@ def err_cond_voidptr_arc : Error <
"in ARC mode">;
def err_typecheck_comparison_of_distinct_pointers : Error<
"comparison of distinct pointer types%diff{ ($ and $)|}0,1">;
def ext_typecheck_comparison_of_distinct_pointers_nonstandard : ExtWarn<
"comparison of distinct pointer types (%0 and %1) uses non-standard "
"composite pointer type %2">, InGroup<CompareDistinctPointerType>;
def err_typecheck_op_on_nonoverlapping_address_space_pointers : Error<
"%select{comparison between %diff{ ($ and $)|}0,1"
"|arithmetic operation with operands of type %diff{ ($ and $)|}0,1"
Expand Down Expand Up @@ -6837,9 +6836,6 @@ def err_typecheck_expect_scalar_operand : Error<
"operand of type %0 where arithmetic or pointer type is required">;
def err_typecheck_cond_incompatible_operands : Error<
"incompatible operand types%diff{ ($ and $)|}0,1">;
def ext_typecheck_cond_incompatible_operands_nonstandard : ExtWarn<
"incompatible operand types%diff{ ($ and $)|}0,1 use non-standard composite "
"pointer type %2">;
def err_cast_selector_expr : Error<
"cannot type cast @selector expression">;
def ext_typecheck_cond_incompatible_pointers : ExtWarn<
Expand Down
6 changes: 2 additions & 4 deletions include/clang/Sema/Sema.h
View file
Expand Up @@ -8941,15 +8941,13 @@ class Sema {
ExprResult &cond, ExprResult &lhs, ExprResult &rhs,
ExprValueKind &VK, ExprObjectKind &OK, SourceLocation questionLoc);
QualType FindCompositePointerType(SourceLocation Loc, Expr *&E1, Expr *&E2,
bool *NonStandardCompositeType = nullptr,
bool ConvertArgs = true);
QualType FindCompositePointerType(SourceLocation Loc,
ExprResult &E1, ExprResult &E2,
bool *NonStandardCompositeType = nullptr,
bool ConvertArgs = true) {
Expr *E1Tmp = E1.get(), *E2Tmp = E2.get();
QualType Composite = FindCompositePointerType(
Loc, E1Tmp, E2Tmp, NonStandardCompositeType, ConvertArgs);
QualType Composite =
FindCompositePointerType(Loc, E1Tmp, E2Tmp, ConvertArgs);
E1 = E1Tmp;
E2 = E2Tmp;
return Composite;
Expand Down
193 changes: 111 additions & 82 deletions lib/Sema/SemaExpr.cpp
View file
Expand Up @@ -8929,35 +8929,21 @@ static bool convertPointersToCompositeType(Sema &S, SourceLocation Loc,
// C++ [expr.eq]p1 uses the same notion for (in)equality
// comparisons of pointers.

// C++ [expr.eq]p2:
// In addition, pointers to members can be compared, or a pointer to
// member and a null pointer constant. Pointer to member conversions
// (4.11) and qualification conversions (4.4) are performed to bring
// them to a common type. If one operand is a null pointer constant,
// the common type is the type of the other operand. Otherwise, the
// common type is a pointer to member type similar (4.4) to the type
// of one of the operands, with a cv-qualification signature (4.4)
// that is the union of the cv-qualification signatures of the operand
// types.

QualType LHSType = LHS.get()->getType();
QualType RHSType = RHS.get()->getType();
assert((LHSType->isPointerType() && RHSType->isPointerType()) ||
(LHSType->isMemberPointerType() && RHSType->isMemberPointerType()));
assert(LHSType->isPointerType() || RHSType->isPointerType() ||
LHSType->isMemberPointerType() || RHSType->isMemberPointerType());

bool NonStandardCompositeType = false;
bool *BoolPtr = S.isSFINAEContext() ? nullptr : &NonStandardCompositeType;
QualType T = S.FindCompositePointerType(Loc, LHS, RHS, BoolPtr);
QualType T = S.FindCompositePointerType(Loc, LHS, RHS);
if (T.isNull()) {
diagnoseDistinctPointerComparison(S, Loc, LHS, RHS, /*isError*/true);
if ((LHSType->isPointerType() || LHSType->isMemberPointerType()) &&
(RHSType->isPointerType() || RHSType->isMemberPointerType()))
diagnoseDistinctPointerComparison(S, Loc, LHS, RHS, /*isError*/true);
else
S.InvalidOperands(Loc, LHS, RHS);
return true;
}

if (NonStandardCompositeType)
S.Diag(Loc, diag::ext_typecheck_comparison_of_distinct_pointers_nonstandard)
<< LHSType << RHSType << T << LHS.get()->getSourceRange()
<< RHS.get()->getSourceRange();

LHS = S.ImpCastExprToType(LHS.get(), T, CK_BitCast);
RHS = S.ImpCastExprToType(RHS.get(), T, CK_BitCast);
return false;
Expand Down Expand Up @@ -9314,41 +9300,53 @@ QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS,
LHS.get()->getSourceRange());
}

// All of the following pointer-related warnings are GCC extensions, except
// when handling null pointer constants.
if (LHSType->isPointerType() && RHSType->isPointerType()) { // C99 6.5.8p2
QualType LCanPointeeTy =
LHSType->castAs<PointerType>()->getPointeeType().getCanonicalType();
QualType RCanPointeeTy =
RHSType->castAs<PointerType>()->getPointeeType().getCanonicalType();

if (getLangOpts().CPlusPlus) {
if (LCanPointeeTy == RCanPointeeTy)
return ResultTy;
if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
// Valid unless comparison between non-null pointer and function pointer
// This is a gcc extension compatibility comparison.
// In a SFINAE context, we treat this as a hard error to maintain
// conformance with the C++ standard.
if ((LCanPointeeTy->isFunctionType() || RCanPointeeTy->isFunctionType())
&& !LHSIsNull && !RHSIsNull) {
diagnoseFunctionPointerToVoidComparison(
*this, Loc, LHS, RHS, /*isError*/ (bool)isSFINAEContext());

if (isSFINAEContext())
return QualType();

RHS = ImpCastExprToType(RHS.get(), LHSType, CK_BitCast);
return ResultTy;
}
}
if ((LHSType->isIntegerType() && !LHSIsNull) ||
(RHSType->isIntegerType() && !RHSIsNull)) {
// Skip normal pointer conversion checks in this case; we have better
// diagnostics for this below.
} else if (getLangOpts().CPlusPlus) {
// Equality comparison of a function pointer to a void pointer is invalid,
// but we allow it as an extension.
// FIXME: If we really want to allow this, should it be part of composite
// pointer type computation so it works in conditionals too?
if (!IsRelational &&
((LHSType->isFunctionPointerType() && RHSType->isVoidPointerType()) ||
(RHSType->isFunctionPointerType() && LHSType->isVoidPointerType()))) {
// This is a gcc extension compatibility comparison.
// In a SFINAE context, we treat this as a hard error to maintain
// conformance with the C++ standard.
diagnoseFunctionPointerToVoidComparison(
*this, Loc, LHS, RHS, /*isError*/ (bool)isSFINAEContext());

if (isSFINAEContext())
return QualType();

RHS = ImpCastExprToType(RHS.get(), LHSType, CK_BitCast);
return ResultTy;
}

// C++ [expr.eq]p2:
// If at least one operand is a pointer [...] bring them to their
// composite pointer type.
// C++ [expr.rel]p2:
// If both operands are pointers, [...] bring them to their composite
// pointer type.
if ((int)LHSType->isPointerType() + (int)RHSType->isPointerType() >=
(IsRelational ? 2 : 1)) {
if (convertPointersToCompositeType(*this, Loc, LHS, RHS))
return QualType();
else
return ResultTy;
}
} else if (LHSType->isPointerType() &&
RHSType->isPointerType()) { // C99 6.5.8p2
// All of the following pointer-related warnings are GCC extensions, except
// when handling null pointer constants.
QualType LCanPointeeTy =
LHSType->castAs<PointerType>()->getPointeeType().getCanonicalType();
QualType RCanPointeeTy =
RHSType->castAs<PointerType>()->getPointeeType().getCanonicalType();

// C99 6.5.9p2 and C99 6.5.8p2
if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
Expand Down Expand Up @@ -9393,36 +9391,63 @@ QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS,
}

if (getLangOpts().CPlusPlus) {
// Comparison of nullptr_t with itself.
if (LHSType->isNullPtrType() && RHSType->isNullPtrType())
return ResultTy;

// Comparison of pointers with null pointer constants and equality
// comparisons of member pointers to null pointer constants.
if (RHSIsNull &&
((LHSType->isAnyPointerType() || LHSType->isNullPtrType()) ||
(!IsRelational &&
(LHSType->isMemberPointerType() || LHSType->isBlockPointerType())))) {
RHS = ImpCastExprToType(RHS.get(), LHSType,
LHSType->isMemberPointerType()
? CK_NullToMemberPointer
: CK_NullToPointer);
// C++ [expr.eq]p4:
// Two operands of type std::nullptr_t or one operand of type
// std::nullptr_t and the other a null pointer constant compare equal.
if (!IsRelational && LHSIsNull && RHSIsNull) {
if (LHSType->isNullPtrType()) {
RHS = ImpCastExprToType(RHS.get(), LHSType, CK_NullToPointer);
return ResultTy;
}
if (RHSType->isNullPtrType()) {
LHS = ImpCastExprToType(LHS.get(), RHSType, CK_NullToPointer);
return ResultTy;
}
}

// Comparison of Objective-C pointers and block pointers against nullptr_t.
// These aren't covered by the composite pointer type rules.
if (!IsRelational && RHSType->isNullPtrType() &&
(LHSType->isObjCObjectPointerType() || LHSType->isBlockPointerType())) {
RHS = ImpCastExprToType(RHS.get(), LHSType, CK_NullToPointer);
return ResultTy;
}
if (LHSIsNull &&
((RHSType->isAnyPointerType() || RHSType->isNullPtrType()) ||
(!IsRelational &&
(RHSType->isMemberPointerType() || RHSType->isBlockPointerType())))) {
LHS = ImpCastExprToType(LHS.get(), RHSType,
RHSType->isMemberPointerType()
? CK_NullToMemberPointer
: CK_NullToPointer);
if (!IsRelational && LHSType->isNullPtrType() &&
(RHSType->isObjCObjectPointerType() || RHSType->isBlockPointerType())) {
LHS = ImpCastExprToType(LHS.get(), RHSType, CK_NullToPointer);
return ResultTy;
}

// Comparison of member pointers.
if (IsRelational &&
((LHSType->isNullPtrType() && RHSType->isPointerType()) ||
(RHSType->isNullPtrType() && LHSType->isPointerType()))) {
// HACK: Relational comparison of nullptr_t against a pointer type is
// invalid per DR583, but we allow it within std::less<> and friends,
// since otherwise common uses of it break.
// FIXME: Consider removing this hack once LWG fixes std::less<> and
// friends to have std::nullptr_t overload candidates.
DeclContext *DC = CurContext;
if (isa<FunctionDecl>(DC))
DC = DC->getParent();
if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(DC)) {
if (CTSD->isInStdNamespace() &&
llvm::StringSwitch<bool>(CTSD->getName())
.Cases("less", "less_equal", "greater", "greater_equal", true)
.Default(false)) {
if (RHSType->isNullPtrType())
RHS = ImpCastExprToType(RHS.get(), LHSType, CK_NullToPointer);
else
LHS = ImpCastExprToType(LHS.get(), RHSType, CK_NullToPointer);
return ResultTy;
}
}
}

// C++ [expr.eq]p2:
// If at least one operand is a pointer to member, [...] bring them to
// their composite pointer type.
if (!IsRelational &&
LHSType->isMemberPointerType() && RHSType->isMemberPointerType()) {
(LHSType->isMemberPointerType() || RHSType->isMemberPointerType())) {
if (convertPointersToCompositeType(*this, Loc, LHS, RHS))
return QualType();
else
Expand Down Expand Up @@ -9531,15 +9556,19 @@ QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS,
// Under a debugger, allow the comparison of pointers to integers,
// since users tend to want to compare addresses.
} else if ((LHSIsNull && LHSType->isIntegerType()) ||
(RHSIsNull && RHSType->isIntegerType())) {
if (IsRelational && !getLangOpts().CPlusPlus)
DiagID = diag::ext_typecheck_ordered_comparison_of_pointer_and_zero;
} else if (IsRelational && !getLangOpts().CPlusPlus)
DiagID = diag::ext_typecheck_ordered_comparison_of_pointer_integer;
else if (getLangOpts().CPlusPlus) {
(RHSIsNull && RHSType->isIntegerType())) {
if (IsRelational) {
isError = getLangOpts().CPlusPlus;
DiagID =
isError ? diag::err_typecheck_ordered_comparison_of_pointer_and_zero
: diag::ext_typecheck_ordered_comparison_of_pointer_and_zero;
}
} else if (getLangOpts().CPlusPlus) {
DiagID = diag::err_typecheck_comparison_of_pointer_integer;
isError = true;
} else
} else if (IsRelational)
DiagID = diag::ext_typecheck_ordered_comparison_of_pointer_integer;
else
DiagID = diag::ext_typecheck_comparison_of_pointer_integer;

if (DiagID) {
Expand Down
40 changes: 8 additions & 32 deletions lib/Sema/SemaExprCXX.cpp
View file
Expand Up @@ -5410,7 +5410,7 @@ QualType Sema::CXXCheckConditionalOperands(ExprResult &Cond, ExprResult &LHS,
// exception specifications, if any.
if (LTy->isFunctionPointerType() || LTy->isMemberFunctionPointerType()) {
Qualifiers Qs = LTy.getQualifiers();
LTy = FindCompositePointerType(QuestionLoc, LHS, RHS, nullptr,
LTy = FindCompositePointerType(QuestionLoc, LHS, RHS,
/*ConvertArgs*/false);
LTy = Context.getQualifiedType(LTy, Qs);

Expand Down Expand Up @@ -5522,19 +5522,9 @@ QualType Sema::CXXCheckConditionalOperands(ExprResult &Cond, ExprResult &LHS,
// performed to bring them to a common type, whose cv-qualification
// shall match the cv-qualification of either the second or the third
// operand. The result is of the common type.
bool NonStandardCompositeType = false;
QualType Composite = FindCompositePointerType(QuestionLoc, LHS, RHS,
isSFINAEContext() ? nullptr
: &NonStandardCompositeType);
if (!Composite.isNull()) {
if (NonStandardCompositeType)
Diag(QuestionLoc,
diag::ext_typecheck_cond_incompatible_operands_nonstandard)
<< LTy << RTy << Composite
<< LHS.get()->getSourceRange() << RHS.get()->getSourceRange();

QualType Composite = FindCompositePointerType(QuestionLoc, LHS, RHS);
if (!Composite.isNull())
return Composite;
}

// Similarly, attempt to find composite type of two objective-c pointers.
Composite = FindCompositeObjCPointerType(LHS, RHS, QuestionLoc);
Expand Down Expand Up @@ -5633,19 +5623,10 @@ mergeExceptionSpecs(Sema &S, FunctionProtoType::ExceptionSpecInfo ESI1,
/// \param Loc The location of the operator requiring these two expressions to
/// be converted to the composite pointer type.
///
/// If \p NonStandardCompositeType is non-NULL, then we are permitted to find
/// a non-standard (but still sane) composite type to which both expressions
/// can be converted. When such a type is chosen, \c *NonStandardCompositeType
/// will be set true.
///
/// \param ConvertArgs If \c false, do not convert E1 and E2 to the target type.
QualType Sema::FindCompositePointerType(SourceLocation Loc,
Expr *&E1, Expr *&E2,
bool *NonStandardCompositeType,
bool ConvertArgs) {
if (NonStandardCompositeType)
*NonStandardCompositeType = false;

assert(getLangOpts().CPlusPlus && "This function assumes C++");

// C++1z [expr]p14:
Expand Down Expand Up @@ -5738,8 +5719,7 @@ QualType Sema::FindCompositePointerType(SourceLocation Loc,

// If we're allowed to create a non-standard composite type, keep track
// of where we need to fill in additional 'const' qualifiers.
if (NonStandardCompositeType &&
Composite1.getCVRQualifiers() != Composite2.getCVRQualifiers())
if (Composite1.getCVRQualifiers() != Composite2.getCVRQualifiers())
NeedConstBefore = QualifierUnion.size();

QualifierUnion.push_back(
Expand All @@ -5756,8 +5736,7 @@ QualType Sema::FindCompositePointerType(SourceLocation Loc,

// If we're allowed to create a non-standard composite type, keep track
// of where we need to fill in additional 'const' qualifiers.
if (NonStandardCompositeType &&
Composite1.getCVRQualifiers() != Composite2.getCVRQualifiers())
if (Composite1.getCVRQualifiers() != Composite2.getCVRQualifiers())
NeedConstBefore = QualifierUnion.size();

QualifierUnion.push_back(
Expand Down Expand Up @@ -5808,16 +5787,13 @@ QualType Sema::FindCompositePointerType(SourceLocation Loc,
}
}

if (NeedConstBefore && NonStandardCompositeType) {
if (NeedConstBefore) {
// Extension: Add 'const' to qualifiers that come before the first qualifier
// mismatch, so that our (non-standard!) composite type meets the
// requirements of C++ [conv.qual]p4 bullet 3.
for (unsigned I = 0; I != NeedConstBefore; ++I) {
if ((QualifierUnion[I] & Qualifiers::Const) == 0) {
for (unsigned I = 0; I != NeedConstBefore; ++I)
if ((QualifierUnion[I] & Qualifiers::Const) == 0)
QualifierUnion[I] = QualifierUnion[I] | Qualifiers::Const;
*NonStandardCompositeType = true;
}
}
}

// Rewrap the composites as pointers or member pointers with the union CVRs.
Expand Down

0 comments on commit 4b6ad14

Please sign in to comment.