r283508 - PR25890: Fix incoherent error handling in PerformImplicitConversion and
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 6 16:12:59 PDT 2016
Author: rsmith
Date: Thu Oct 6 18:12:58 2016
New Revision: 283508
URL: http://llvm.org/viewvc/llvm-project?rev=283508&view=rev
Log:
PR25890: Fix incoherent error handling in PerformImplicitConversion and
CheckSingleAssignmentConstraints. These no longer produce ExprError() when they
have not emitted an error, and reliably inform the caller when they *have*
emitted an error.
This fixes some serious issues where we would fail to emit any diagnostic for
invalid code and then attempt to emit code for an invalid AST, and conversely
some issues where we would emit two diagnostics for the same problem.
Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaPseudoObject.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/test/CXX/drs/dr0xx.cpp
cfe/trunk/test/CXX/except/except.spec/p5-pointers.cpp
cfe/trunk/test/SemaCXX/ambig-user-defined-conversions.cpp
cfe/trunk/test/SemaCXX/derived-to-base-ambig.cpp
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=283508&r1=283507&r2=283508&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Oct 6 18:12:58 2016
@@ -8835,15 +8835,23 @@ public:
CastKind &Kind,
bool ConvertRHS = true);
- // CheckSingleAssignmentConstraints - Currently used by
- // CheckAssignmentOperands, and ActOnReturnStmt. Prior to type checking,
- // this routine performs the default function/array converions, if ConvertRHS
- // is true.
- AssignConvertType CheckSingleAssignmentConstraints(QualType LHSType,
- ExprResult &RHS,
- bool Diagnose = true,
- bool DiagnoseCFAudited = false,
- bool ConvertRHS = true);
+ /// Check assignment constraints for an assignment of RHS to LHSType.
+ ///
+ /// \brief LHSType The destination type for the assignment.
+ /// \brief RHS The source expression for the assignment.
+ /// \brief Diagnose If \c true, diagnostics may be produced when checking
+ /// for assignability. If a diagnostic is produced, \p RHS will be
+ /// set to ExprError(). Note that this function may still return
+ /// without producing a diagnostic, even for an invalid assignment.
+ /// \brief DiagnoseCFAudited If \c true, the target is a function parameter
+ /// in an audited Core Foundation API and does not need to be checked
+ /// for ARC retain issues.
+ /// \brief ConvertRHS If \c true, \p RHS will be updated to model the
+ /// conversions necessary to perform the assignment. If \c false,
+ /// \p Diagnose must also be \c false.
+ AssignConvertType CheckSingleAssignmentConstraints(
+ QualType LHSType, ExprResult &RHS, bool Diagnose = true,
+ bool DiagnoseCFAudited = false, bool ConvertRHS = true);
// \brief If the lhs type is a transparent union, check whether we
// can initialize the transparent union with the given expression.
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=283508&r1=283507&r2=283508&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Oct 6 18:12:58 2016
@@ -7793,6 +7793,10 @@ Sema::CheckSingleAssignmentConstraints(Q
bool Diagnose,
bool DiagnoseCFAudited,
bool ConvertRHS) {
+ // We need to be able to tell the caller whether we diagnosed a problem, if
+ // they ask us to issue diagnostics.
+ assert((ConvertRHS || !Diagnose) && "can't indicate whether we diagnosed");
+
// If ConvertRHS is false, we want to leave the caller's RHS untouched. Sadly,
// we can't avoid *all* modifications at the moment, so we need some somewhere
// to put the updated value.
@@ -7804,9 +7808,9 @@ Sema::CheckSingleAssignmentConstraints(Q
// C++ 5.17p3: If the left operand is not of class type, the
// expression is implicitly converted (C++ 4) to the
// cv-unqualified type of the left operand.
- ExprResult Res;
+ QualType RHSType = RHS.get()->getType();
if (Diagnose) {
- Res = PerformImplicitConversion(RHS.get(), LHSType.getUnqualifiedType(),
+ RHS = PerformImplicitConversion(RHS.get(), LHSType.getUnqualifiedType(),
AA_Assigning);
} else {
ImplicitConversionSequence ICS =
@@ -7818,17 +7822,15 @@ Sema::CheckSingleAssignmentConstraints(Q
/*AllowObjCWritebackConversion=*/false);
if (ICS.isFailure())
return Incompatible;
- Res = PerformImplicitConversion(RHS.get(), LHSType.getUnqualifiedType(),
+ RHS = PerformImplicitConversion(RHS.get(), LHSType.getUnqualifiedType(),
ICS, AA_Assigning);
}
- if (Res.isInvalid())
+ if (RHS.isInvalid())
return Incompatible;
Sema::AssignConvertType result = Compatible;
if (getLangOpts().ObjCAutoRefCount &&
- !CheckObjCARCUnavailableWeakConversion(LHSType,
- RHS.get()->getType()))
+ !CheckObjCARCUnavailableWeakConversion(LHSType, RHSType))
result = IncompatibleObjCWeakRef;
- RHS = Res;
return result;
}
Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=283508&r1=283507&r2=283508&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Thu Oct 6 18:12:58 2016
@@ -3320,6 +3320,10 @@ Sema::PerformImplicitConversion(Expr *Fr
llvm_unreachable("Cannot perform an ellipsis conversion");
case ImplicitConversionSequence::BadConversion:
+ bool Diagnosed =
+ DiagnoseAssignmentResult(Incompatible, From->getExprLoc(), ToType,
+ From->getType(), From, Action);
+ assert(Diagnosed && "failed to diagnose bad conversion"); (void)Diagnosed;
return ExprError();
}
Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=283508&r1=283507&r2=283508&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Thu Oct 6 18:12:58 2016
@@ -5344,6 +5344,7 @@ TryContextuallyConvertToObjCPointer(Sema
/// PerformContextuallyConvertToObjCPointer - Perform a contextual
/// conversion of the expression From to an Objective-C pointer type.
+/// Returns a valid but null ExprResult if no conversion sequence exists.
ExprResult Sema::PerformContextuallyConvertToObjCPointer(Expr *From) {
if (checkPlaceholderForOverload(*this, From))
return ExprError();
@@ -5353,7 +5354,7 @@ ExprResult Sema::PerformContextuallyConv
TryContextuallyConvertToObjCPointer(*this, From);
if (!ICS.isBad())
return PerformImplicitConversion(From, Ty, ICS, AA_Converting);
- return ExprError();
+ return ExprResult();
}
/// Determine whether the provided type is an integral type, or an enumeration
Modified: cfe/trunk/lib/Sema/SemaPseudoObject.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaPseudoObject.cpp?rev=283508&r1=283507&r2=283508&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaPseudoObject.cpp (original)
+++ cfe/trunk/lib/Sema/SemaPseudoObject.cpp Thu Oct 6 18:12:58 2016
@@ -770,7 +770,8 @@ ExprResult ObjCPropertyOpBuilder::buildS
ExprResult opResult = op;
Sema::AssignConvertType assignResult
= S.CheckSingleAssignmentConstraints(paramType, opResult);
- if (S.DiagnoseAssignmentResult(assignResult, opcLoc, paramType,
+ if (opResult.isInvalid() ||
+ S.DiagnoseAssignmentResult(assignResult, opcLoc, paramType,
op->getType(), opResult.get(),
Sema::AA_Assigning))
return ExprError();
Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=283508&r1=283507&r2=283508&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Thu Oct 6 18:12:58 2016
@@ -3493,6 +3493,8 @@ Sema::ActOnObjCAtSynchronizedOperand(Sou
<< type << operand->getSourceRange();
ExprResult result = PerformContextuallyConvertToObjCPointer(operand);
+ if (result.isInvalid())
+ return ExprError();
if (!result.isUsable())
return Diag(atLoc, diag::error_objc_synchronized_expects_object)
<< type << operand->getSourceRange();
Modified: cfe/trunk/test/CXX/drs/dr0xx.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/drs/dr0xx.cpp?rev=283508&r1=283507&r2=283508&view=diff
==============================================================================
--- cfe/trunk/test/CXX/drs/dr0xx.cpp (original)
+++ cfe/trunk/test/CXX/drs/dr0xx.cpp Thu Oct 6 18:12:58 2016
@@ -286,10 +286,9 @@ namespace dr25 { // dr25: yes
void (A::*i2)() throw () = 0;
void (A::*j)() throw (int, char) = &A::f;
void x() {
- // FIXME: Don't produce the second error here.
- g2 = f; // expected-error {{is not superset}} expected-error {{incompatible}}
+ g2 = f; // expected-error {{is not superset}}
h = f;
- i2 = &A::f; // expected-error {{is not superset}} expected-error {{incompatible}}
+ i2 = &A::f; // expected-error {{is not superset}}
j = &A::f;
}
}
Modified: cfe/trunk/test/CXX/except/except.spec/p5-pointers.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/except/except.spec/p5-pointers.cpp?rev=283508&r1=283507&r2=283508&view=diff
==============================================================================
--- cfe/trunk/test/CXX/except/except.spec/p5-pointers.cpp (original)
+++ cfe/trunk/test/CXX/except/except.spec/p5-pointers.cpp Thu Oct 6 18:12:58 2016
@@ -41,25 +41,25 @@ void fnptrs()
{
// Assignment and initialization of function pointers.
void (*t1)() throw() = &s1; // valid
- t1 = &s2; // expected-error {{not superset}} expected-error {{incompatible type}}
- t1 = &s3; // expected-error {{not superset}} expected-error {{incompatible type}}
+ t1 = &s2; // expected-error {{not superset}}
+ t1 = &s3; // expected-error {{not superset}}
void (&t2)() throw() = s2; // expected-error {{not superset}}
void (*t3)() throw(int) = &s2; // valid
void (*t4)() throw(A) = &s1; // valid
t4 = &s3; // valid
t4 = &s4; // valid
- t4 = &s5; // expected-error {{not superset}} expected-error {{incompatible type}}
+ t4 = &s5; // expected-error {{not superset}}
void (*t5)() = &s1; // valid
t5 = &s2; // valid
t5 = &s6; // valid
t5 = &s7; // valid
- t1 = t3; // expected-error {{not superset}} expected-error {{incompatible type}}
+ t1 = t3; // expected-error {{not superset}}
t3 = t1; // valid
void (*t6)() throw(B1);
- t6 = t4; // expected-error {{not superset}} expected-error {{incompatible type}}
+ t6 = t4; // expected-error {{not superset}}
t4 = t6; // valid
t5 = t1; // valid
- t1 = t5; // expected-error {{not superset}} expected-error {{incompatible type}}
+ t1 = t5; // expected-error {{not superset}}
// return types and arguments must match exactly, no inheritance allowed
void (*(*t7)())() throw(B1) = &s8; // valid
Modified: cfe/trunk/test/SemaCXX/ambig-user-defined-conversions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ambig-user-defined-conversions.cpp?rev=283508&r1=283507&r2=283508&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/ambig-user-defined-conversions.cpp (original)
+++ cfe/trunk/test/SemaCXX/ambig-user-defined-conversions.cpp Thu Oct 6 18:12:58 2016
@@ -65,3 +65,8 @@ namespace rdar8876150 {
bool f(D d) { return !d; } // expected-error{{ambiguous conversion from derived class 'rdar8876150::D' to base class 'rdar8876150::A':}}
}
+
+namespace assignment {
+ struct A { operator short(); operator bool(); }; // expected-note 2{{candidate}}
+ void f(int n, A a) { n = a; } // expected-error{{ambiguous}}
+}
Modified: cfe/trunk/test/SemaCXX/derived-to-base-ambig.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/derived-to-base-ambig.cpp?rev=283508&r1=283507&r2=283508&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/derived-to-base-ambig.cpp (original)
+++ cfe/trunk/test/SemaCXX/derived-to-base-ambig.cpp Thu Oct 6 18:12:58 2016
@@ -6,7 +6,7 @@ class D : public B, public C { };
void f(D* d) {
A* a;
- a = d; // expected-error{{ambiguous conversion from derived class 'D' to base class 'A':}} expected-error{{assigning to 'A *' from incompatible type 'D *'}}
+ a = d; // expected-error{{ambiguous conversion from derived class 'D' to base class 'A':}}
}
class Object2 { };
@@ -20,7 +20,7 @@ class F2 : public E2, public A2 { }; //
void g(E2* e2, F2* f2) {
Object2* o2;
o2 = e2;
- o2 = f2; // expected-error{{ambiguous conversion from derived class 'F2' to base class 'Object2':}} expected-error{{assigning to 'Object2 *' from incompatible type 'F2 *'}}
+ o2 = f2; // expected-error{{ambiguous conversion from derived class 'F2' to base class 'Object2':}}
}
// Test that ambiguous/inaccessibility checking does not trigger too
More information about the cfe-commits
mailing list