manmanren created this revision.
manmanren added reviewers: rjmccall, rsmith.
manmanren added a subscriber: cfe-commits.
The issue —————————————
Given the following test case, we don’t emit any message, and will silently
ignore the variable "foo2", with a release compiler; we will get an assertion
failure with an assert compiler.
typedef struct {
const char *name;
id field;
} Test9;
extern void doSomething(Test9 arg);
void test9() {
Test9 foo2 = {0, 0};
doSomething(foo2);
}
The root cause —————————————
Back in r140457 we gave InitListChecker a verification-only mode, and will use
CanUseDecl instead of DiagnoseUseOfDecl for verification-only mode.
These two functions handle unavailable issues differently:
In Sema::CanUseDecl, we say the decl is invalid with the following condition
if (D->getAvailability() == AR_Unavailable &&
cast<Decl>(CurContext)->getAvailability() != AR_Unavailable)
In Sema::DiagnoseUseOfDecl, we say the decl is usable by ignoring the return
code of DiagnoseAvailabilityOfDecl
So with an assert build, we will hit an assertion in diagnoseListInit
assert(DiagnoseInitList.HadError() &&
"Inconsistent init list check result.");
The question is how to make the two consistent, should we follow what is
implemented in CanUseDecl or DiagnoseUseOfDecl?
If we follow what is implemented in CanUseDecl and treat Decls with unavailable
issues as invalid, the variable decl of “foo2” will be marked as invalid.
Since unavailable checking is processed in delayed diagnostics (r197627), we
will silently ignore the diagnostics when we find out that the variable decl is
invalid.
In Sema::PopParsingDeclaration
switch (diag.Kind) {
case DelayedDiagnostic::Deprecation:
case DelayedDiagnostic::Unavailable:
// Don't bother giving deprecation/unavailable diagnostics if
// the decl is invalid.
if (!decl->isInvalidDecl())
handleDelayedAvailabilityCheck(*this, diag, decl);
The proposed fix ———————————————
It seems that for overload resolution, we want to say decls with unavailable
issues are invalid; but for everything else, we should say they are valid but
emit diagnostics. For this, we can add another flag “UnavailableCheck” for the
verification-only mode. Depending on the value of the flag, CanUseDecl
can return different values for unavailable issues.
Feedback is appreciated. Any other suggestion is welcome.
Cheers,
Manman
http://reviews.llvm.org/D15314
Files:
include/clang/Sema/Initialization.h
include/clang/Sema/Sema.h
lib/Sema/SemaDecl.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaInit.cpp
test/SemaObjC/Inputs/arc-system-header.h
test/SemaObjC/arc-system-header.m
Index: test/SemaObjC/arc-system-header.m
===================================================================
--- test/SemaObjC/arc-system-header.m
+++ test/SemaObjC/arc-system-header.m
@@ -46,6 +46,13 @@
// expected-note@arc-system-header.h:41 4 {{declaration uses type that is ill-formed in ARC}}
// expected-note@arc-system-header.h:41 2 {{property 'prop' is declared unavailable here}}
}
+
+extern void doSomething(Test9 arg);
+void test9() {
+ Test9 foo2 = {0, 0}; // expected-error {{'field' is unavailable in ARC}}
+ // expected-note@arc-system-header.h:56 {{field has non-trivial ownership qualification}}
+ doSomething(foo2);
+}
#endif
// test8 in header
Index: test/SemaObjC/Inputs/arc-system-header.h
===================================================================
--- test/SemaObjC/Inputs/arc-system-header.h
+++ test/SemaObjC/Inputs/arc-system-header.h
@@ -50,3 +50,8 @@
static inline void *test8(id ptr) {
return (__bridge_retain void*) ptr;
}
+
+typedef struct {
+ const char *name;
+ id field;
+} Test9;
Index: lib/Sema/SemaInit.cpp
===================================================================
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -236,6 +236,7 @@
Sema &SemaRef;
bool hadError;
bool VerifyOnly; // no diagnostics, no structure building
+ bool UnavailableCheck; // Used only in VerifyOnly mode.
llvm::DenseMap<InitListExpr *, InitListExpr *> SyntacticToSemantic;
InitListExpr *FullyStructuredList;
@@ -317,7 +318,7 @@
static ExprResult PerformEmptyInit(Sema &SemaRef,
SourceLocation Loc,
const InitializedEntity &Entity,
- bool VerifyOnly);
+ bool VerifyOnly, bool UnavailableCheck);
// Explanation on the "FillWithNoInit" mode:
//
@@ -353,7 +354,8 @@
public:
InitListChecker(Sema &S, const InitializedEntity &Entity,
- InitListExpr *IL, QualType &T, bool VerifyOnly);
+ InitListExpr *IL, QualType &T, bool VerifyOnly,
+ bool UnavailableCheck);
bool HadError() { return hadError; }
// @brief Retrieves the fully-structured initializer list used for
@@ -365,7 +367,8 @@
ExprResult InitListChecker::PerformEmptyInit(Sema &SemaRef,
SourceLocation Loc,
const InitializedEntity &Entity,
- bool VerifyOnly) {
+ bool VerifyOnly,
+ bool UnavailableCheck) {
InitializationKind Kind = InitializationKind::CreateValue(Loc, Loc, Loc,
true);
MultiExprArg SubInit;
@@ -437,7 +440,7 @@
InitSeq.InitializeFrom(
SemaRef, Entity,
InitializationKind::CreateValue(Loc, Loc, Loc, true),
- MultiExprArg(), /*TopLevelOfInitList=*/false);
+ MultiExprArg(), /*TopLevelOfInitList=*/false, UnavailableCheck);
// Emit a warning for this. System header warnings aren't shown
// by default, but people working on system headers should see it.
if (!VerifyOnly) {
@@ -474,7 +477,8 @@
SourceLocation Loc) {
assert(VerifyOnly &&
"CheckEmptyInitializable is only inteded for verification mode.");
- if (PerformEmptyInit(SemaRef, Loc, Entity, /*VerifyOnly*/true).isInvalid())
+ if (PerformEmptyInit(SemaRef, Loc, Entity, /*VerifyOnly*/true,
+ UnavailableCheck).isInvalid())
hadError = true;
}
@@ -535,7 +539,8 @@
}
ExprResult MemberInit = PerformEmptyInit(SemaRef, Loc, MemberEntity,
- /*VerifyOnly*/false);
+ /*VerifyOnly*/false,
+ UnavailableCheck);
if (MemberInit.isInvalid()) {
hadError = true;
return;
@@ -661,7 +666,8 @@
else {
ExprResult ElementInit = PerformEmptyInit(SemaRef, ILE->getLocEnd(),
ElementEntity,
- /*VerifyOnly*/false);
+ /*VerifyOnly*/false,
+ UnavailableCheck);
if (ElementInit.isInvalid()) {
hadError = true;
return;
@@ -710,8 +716,8 @@
InitListChecker::InitListChecker(Sema &S, const InitializedEntity &Entity,
InitListExpr *IL, QualType &T,
- bool VerifyOnly)
- : SemaRef(S), VerifyOnly(VerifyOnly) {
+ bool VerifyOnly, bool UnavailableCheck)
+ : SemaRef(S), VerifyOnly(VerifyOnly), UnavailableCheck(UnavailableCheck) {
// FIXME: Check that IL isn't already the semantic form of some other
// InitListExpr. If it is, we'd create a broken AST.
@@ -1781,7 +1787,7 @@
// Make sure we can use this declaration.
bool InvalidUse;
if (VerifyOnly)
- InvalidUse = !SemaRef.CanUseDecl(*Field);
+ InvalidUse = !SemaRef.CanUseDecl(*Field, UnavailableCheck);
else
InvalidUse = SemaRef.DiagnoseUseOfDecl(*Field,
IList->getInit(Index)->getLocStart());
@@ -2185,7 +2191,7 @@
// Make sure we can use this declaration.
bool InvalidUse;
if (VerifyOnly)
- InvalidUse = !SemaRef.CanUseDecl(*Field);
+ InvalidUse = !SemaRef.CanUseDecl(*Field, UnavailableCheck);
else
InvalidUse = SemaRef.DiagnoseUseOfDecl(*Field, D->getFieldLoc());
if (InvalidUse) {
@@ -3312,7 +3318,8 @@
const InitializedEntity &Entity,
const InitializationKind &Kind,
InitListExpr *InitList,
- InitializationSequence &Sequence);
+ InitializationSequence &Sequence,
+ bool UnavailableCheck);
/// \brief When initializing from init list via constructor, handle
/// initialization of an object of type std::initializer_list<T>.
@@ -3322,7 +3329,8 @@
static bool TryInitializerListConstruction(Sema &S,
InitListExpr *List,
QualType DestType,
- InitializationSequence &Sequence) {
+ InitializationSequence &Sequence,
+ bool UnavailableCheck) {
QualType E;
if (!S.isStdInitializerList(DestType, &E))
return false;
@@ -3341,7 +3349,7 @@
InitializedEntity::InitializeTemporary(ArrayType);
InitializationKind Kind =
InitializationKind::CreateDirectList(List->getExprLoc());
- TryListInitialization(S, HiddenArray, Kind, List, Sequence);
+ TryListInitialization(S, HiddenArray, Kind, List, Sequence, UnavailableCheck);
if (Sequence)
Sequence.AddStdInitializerListConstructionStep(DestType);
return true;
@@ -3590,7 +3598,8 @@
const InitializedEntity &Entity,
const InitializationKind &Kind,
InitListExpr *InitList,
- InitializationSequence &Sequence) {
+ InitializationSequence &Sequence,
+ bool UnavailableCheck) {
// First, catch C++03 where this isn't possible.
if (!S.getLangOpts().CPlusPlus11) {
Sequence.SetFailed(InitializationSequence::FK_ReferenceBindingToInitList);
@@ -3646,7 +3655,8 @@
// Not reference-related. Create a temporary and bind to that.
InitializedEntity TempEntity = InitializedEntity::InitializeTemporary(cv1T1);
- TryListInitialization(S, TempEntity, Kind, InitList, Sequence);
+ TryListInitialization(S, TempEntity, Kind, InitList, Sequence,
+ UnavailableCheck);
if (Sequence) {
if (DestType->isRValueReferenceType() ||
(T1Quals.hasConst() && !T1Quals.hasVolatile()))
@@ -3662,7 +3672,8 @@
const InitializedEntity &Entity,
const InitializationKind &Kind,
InitListExpr *InitList,
- InitializationSequence &Sequence) {
+ InitializationSequence &Sequence,
+ bool UnavailableCheck) {
QualType DestType = Entity.getType();
// C++ doesn't allow scalar initialization with more than one argument.
@@ -3673,7 +3684,8 @@
return;
}
if (DestType->isReferenceType()) {
- TryReferenceListInitialization(S, Entity, Kind, InitList, Sequence);
+ TryReferenceListInitialization(S, Entity, Kind, InitList, Sequence,
+ UnavailableCheck);
return;
}
@@ -3717,7 +3729,7 @@
InitList->getRBraceLoc())
: Kind;
Sequence.InitializeFrom(S, Entity, SubKind, SubInit,
- /*TopLevelOfInitList*/ true);
+ /*TopLevelOfInitList*/ true, UnavailableCheck);
// TryStringLiteralInitialization() (in InitializeFrom()) will fail if
// the element is not an appropriately-typed string literal, in which
@@ -3749,7 +3761,8 @@
// - Otherwise, if T is a specialization of std::initializer_list<E>,
// an initializer_list object constructed [...]
- if (TryInitializerListConstruction(S, InitList, DestType, Sequence))
+ if (TryInitializerListConstruction(S, InitList, DestType, Sequence,
+ UnavailableCheck))
return;
// - Otherwise, if T is a class type, constructors are considered.
@@ -3785,14 +3798,14 @@
: Kind;
Expr *SubInit[1] = { InitList->getInit(0) };
Sequence.InitializeFrom(S, Entity, SubKind, SubInit,
- /*TopLevelOfInitList*/true);
+ /*TopLevelOfInitList*/true, UnavailableCheck);
if (Sequence)
Sequence.RewrapReferenceInitList(Entity.getType(), InitList);
return;
}
InitListChecker CheckInitList(S, Entity, InitList,
- DestType, /*VerifyOnly=*/true);
+ DestType, /*VerifyOnly=*/true, UnavailableCheck);
if (CheckInitList.HadError()) {
Sequence.SetFailed(InitializationSequence::FK_ListInitializationFailed);
return;
@@ -4799,9 +4812,10 @@
const InitializedEntity &Entity,
const InitializationKind &Kind,
MultiExprArg Args,
- bool TopLevelOfInitList)
+ bool TopLevelOfInitList,
+ bool UnavailableCheck)
: FailedCandidateSet(Kind.getLocation(), OverloadCandidateSet::CSK_Normal) {
- InitializeFrom(S, Entity, Kind, Args, TopLevelOfInitList);
+ InitializeFrom(S, Entity, Kind, Args, TopLevelOfInitList, UnavailableCheck);
}
/// Tries to get a FunctionDecl out of `E`. If it succeeds and we can take the
@@ -4819,7 +4833,8 @@
const InitializedEntity &Entity,
const InitializationKind &Kind,
MultiExprArg Args,
- bool TopLevelOfInitList) {
+ bool TopLevelOfInitList,
+ bool UnavailableCheck) {
ASTContext &Context = S.Context;
// Eliminate non-overload placeholder types in the arguments. We
@@ -4873,7 +4888,7 @@
// object is list-initialized (8.5.4).
if (Kind.getKind() != InitializationKind::IK_Direct) {
if (InitListExpr *InitList = dyn_cast_or_null<InitListExpr>(Initializer)) {
- TryListInitialization(S, Entity, Kind, InitList, *this);
+ TryListInitialization(S, Entity, Kind, InitList, *this, UnavailableCheck);
return;
}
}
@@ -4957,7 +4972,7 @@
Entity.getKind() == InitializedEntity::EK_Member &&
Initializer && isa<InitListExpr>(Initializer)) {
TryListInitialization(S, Entity, Kind, cast<InitListExpr>(Initializer),
- *this);
+ *this, UnavailableCheck);
AddParenthesizedArrayInitStep(DestType);
} else if (DestAT->getElementType()->isCharType())
SetFailed(FK_ArrayNeedsInitListOrStringLiteral);
@@ -6497,7 +6512,7 @@
InitializedEntity TempEntity = InitializedEntity::InitializeTemporary(Ty);
InitializedEntity InitEntity = IsTemporary ? TempEntity : Entity;
InitListChecker PerformInitList(S, InitEntity,
- InitList, Ty, /*VerifyOnly=*/false);
+ InitList, Ty, /*VerifyOnly=*/false, false);
if (PerformInitList.HadError())
return ExprError();
@@ -6868,7 +6883,7 @@
}
InitListChecker DiagnoseInitList(S, Entity, InitList, DestType,
- /*VerifyOnly=*/false);
+ /*VerifyOnly=*/false, false);
assert(DiagnoseInitList.HadError() &&
"Inconsistent init list check result.");
}
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -49,7 +49,7 @@
/// \brief Determine whether the use of this declaration is valid, without
/// emitting diagnostics.
-bool Sema::CanUseDecl(NamedDecl *D) {
+bool Sema::CanUseDecl(NamedDecl *D, bool UnavailableCheck) {
// See if this is an auto-typed variable whose initializer we are parsing.
if (ParsingInitForAutoVars.count(D))
return false;
@@ -67,7 +67,7 @@
}
// See if this function is unavailable.
- if (D->getAvailability() == AR_Unavailable &&
+ if (UnavailableCheck && D->getAvailability() == AR_Unavailable &&
cast<Decl>(CurContext)->getAvailability() != AR_Unavailable)
return false;
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -9365,7 +9365,7 @@
if (VDecl->isInvalidDecl())
return;
- InitializationSequence InitSeq(*this, Entity, Kind, Args);
+ InitializationSequence InitSeq(*this, Entity, Kind, Args, false, false);
ExprResult Result = InitSeq.Perform(*this, Entity, Kind, Args, &DclT);
if (Result.isInvalid()) {
VDecl->setInvalidDecl();
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -3544,7 +3544,7 @@
//===--------------------------------------------------------------------===//
// Expression Parsing Callbacks: SemaExpr.cpp.
- bool CanUseDecl(NamedDecl *D);
+ bool CanUseDecl(NamedDecl *D, bool UnavailableCheck);
bool DiagnoseUseOfDecl(NamedDecl *D, SourceLocation Loc,
const ObjCInterfaceDecl *UnknownObjCClass=nullptr,
bool ObjCPropertyAccess=false);
Index: include/clang/Sema/Initialization.h
===================================================================
--- include/clang/Sema/Initialization.h
+++ include/clang/Sema/Initialization.h
@@ -889,10 +889,11 @@
const InitializedEntity &Entity,
const InitializationKind &Kind,
MultiExprArg Args,
- bool TopLevelOfInitList = false);
+ bool TopLevelOfInitList = false,
+ bool UnavailableCheck = true);
void InitializeFrom(Sema &S, const InitializedEntity &Entity,
const InitializationKind &Kind, MultiExprArg Args,
- bool TopLevelOfInitList);
+ bool TopLevelOfInitList, bool UnavailableCheck);
~InitializationSequence();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
