ilya-biryukov created this revision.
ilya-biryukov added reviewers: aaron.ballman, erichkeane.
Herald added a subscriber: arphaman.
Herald added a project: All.
ilya-biryukov requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
This also reverts 282cae0b9a602267ad7ef622f770066491332a11
<https://reviews.llvm.org/rG282cae0b9a602267ad7ef622f770066491332a11> as the
particular crash is now handled by the new code.
Before this change Clang would always leave declarations inside the
type-locs as `null` if the declarator had an invalid type. This patch
populates declarations even for invalid types if the structure of the
type and the type-locs match.
There are certain cases that may still cause crashes. These happen when
Clang recovers the type in a way that is not reflected in the
declarator's structure, e.g. adding a pointer when it was not present in
the code for ObjC interfaces or ignoring pointers written in the code
in C++ with auto return type (`auto* foo() -> int`). Those cases look
fixable with a better recovery strategy and I plan to follow up with
more patches to address those.
The first attempt caused 31 tests from `check-clang` to crash due to
different structure of the types and type-locs after certain errors. The
good news is that the failure is localized and mismatch in structures is
discovered by assertions inside `DeclaratorLocFiller`. Some notable
cases caught by existing tests:
- Invalid chunks when type is fully ignored and replace with int or now.
Crashed in `C/C2x/n2838.c`.
- Invalid return types in lambdas. Crashed in `CXX/drs/dr6xx.cpp`.
- Invalid member pointers. Crashed in
`CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p3-generic-lambda-1y.cpp`
- ObjC recovery that adds pointers. Crashed in `SemaObjC/blocks.m`
This change also updates the output of `Index/complete-blocks.m`.
Not entirely sure what causes the change, but the new function signature
is closer to the source code, so this seems like an improvement.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D146971
Files:
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaLambda.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/SemaType.cpp
clang/test/Index/complete-blocks.m
Index: clang/test/Index/complete-blocks.m
===================================================================
--- clang/test/Index/complete-blocks.m
+++ clang/test/Index/complete-blocks.m
@@ -85,4 +85,4 @@
// CHECK-CC8: ObjCInstanceMethodDecl:{ResultType id}{TypedText method7:}{Placeholder ^int(int x, int y)b} (35)
// RUN: c-index-test -code-completion-at=%s:59:6 %s | FileCheck -check-prefix=CHECK-CC9 %s
-// CHECK-CC9: ObjCInstanceMethodDecl:{ResultType void}{TypedText foo:}{Placeholder ^int *(int)arg} (35)
+// CHECK-CC9: ObjCInstanceMethodDecl:{ResultType void}{TypedText foo:}{Placeholder ^int *(float)arg} (35)
Index: clang/lib/Sema/SemaType.cpp
===================================================================
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4979,6 +4979,12 @@
// Walk the DeclTypeInfo, building the recursive type as we go.
// DeclTypeInfos are ordered from the identifier out, which is
// opposite of what we want :).
+
+ // Track if the produced type matches the structure of the declarator.
+ // This is used later to decide if we can fill `TypeLoc` from
+ // `DeclaratorChunk`s. E.g. it must be false if Clang recovers from
+ // an error by replacing the type with `int`.
+ bool AreDeclaratorChunksValid = true;
for (unsigned i = 0, e = D.getNumTypeObjects(); i != e; ++i) {
unsigned chunkIndex = e - i - 1;
state.setCurrentChunkIndex(chunkIndex);
@@ -5159,6 +5165,7 @@
: diag::err_deduced_return_type);
T = Context.IntTy;
D.setInvalidType(true);
+ AreDeclaratorChunksValid = false;
} else {
S.Diag(D.getDeclSpec().getTypeSpecTypeLoc(),
diag::warn_cxx11_compat_deduced_return_type);
@@ -5169,6 +5176,8 @@
S.Diag(D.getBeginLoc(), diag::err_trailing_return_in_parens)
<< T << D.getSourceRange();
D.setInvalidType(true);
+ // FIXME: recover and fill decls in `TypeLoc`s.
+ AreDeclaratorChunksValid = false;
} else if (D.getName().getKind() ==
UnqualifiedIdKind::IK_DeductionGuideName) {
if (T != Context.DependentTy) {
@@ -5176,6 +5185,8 @@
diag::err_deduction_guide_with_complex_decl)
<< D.getSourceRange();
D.setInvalidType(true);
+ // FIXME: recover and fill decls in `TypeLoc`s.
+ AreDeclaratorChunksValid = false;
}
} else if (D.getContext() != DeclaratorContext::LambdaExpr &&
(T.hasQualifiers() || !isa<AutoType>(T) ||
@@ -5186,6 +5197,8 @@
diag::err_trailing_return_without_auto)
<< T << D.getDeclSpec().getSourceRange();
D.setInvalidType(true);
+ // FIXME: recover and fill decls in `TypeLoc`s.
+ AreDeclaratorChunksValid = false;
}
T = S.GetTypeFromParser(FTI.getTrailingReturnType(), &TInfo);
if (T.isNull()) {
@@ -5226,6 +5239,7 @@
S.Diag(DeclType.Loc, diagID) << T->isFunctionType() << T;
T = Context.IntTy;
D.setInvalidType(true);
+ AreDeclaratorChunksValid = false;
}
// Do not allow returning half FP value.
@@ -5292,6 +5306,8 @@
ObjCObjectPointerTypeLoc TLoc = TLB.push<ObjCObjectPointerTypeLoc>(T);
TLoc.setStarLoc(FixitLoc);
TInfo = TLB.getTypeSourceInfo(Context, T);
+ } else {
+ AreDeclaratorChunksValid = false;
}
D.setInvalidType(true);
@@ -5412,6 +5428,7 @@
T = (!LangOpts.requiresStrictPrototypes() && !LangOpts.OpenCL)
? Context.getFunctionNoProtoType(T, EI)
: Context.IntTy;
+ AreDeclaratorChunksValid = false;
break;
}
@@ -5646,9 +5663,13 @@
if (!ClsType.isNull())
T = S.BuildMemberPointerType(T, ClsType, DeclType.Loc,
D.getIdentifier());
+ else
+ AreDeclaratorChunksValid = false;
+
if (T.isNull()) {
T = Context.IntTy;
D.setInvalidType(true);
+ AreDeclaratorChunksValid = false;
} else if (DeclType.Mem.TypeQuals) {
T = S.BuildQualifiedType(T, DeclType.Loc, DeclType.Mem.TypeQuals);
}
@@ -5666,6 +5687,7 @@
if (T.isNull()) {
D.setInvalidType(true);
T = Context.IntTy;
+ AreDeclaratorChunksValid = false;
}
// See if there are any attributes on this declarator chunk.
@@ -5924,9 +5946,8 @@
}
assert(!T.isNull() && "T must not be null at the end of this function");
- if (D.isInvalidType())
+ if (!AreDeclaratorChunksValid)
return Context.getTrivialTypeSourceInfo(T);
-
return GetTypeSourceInfoForDeclarator(state, T, TInfo);
}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1343,7 +1343,7 @@
CXXMethodDecl *MD = Result.getAs<LambdaExpr>()->getCallOperator();
for (ParmVarDecl *PVD : MD->parameters()) {
- if (!PVD || !PVD->hasDefaultArg())
+ if (!PVD->hasDefaultArg())
continue;
Expr *UninstExpr = PVD->getUninstantiatedDefaultArg();
// FIXME: Obtain the source location for the '=' token.
Index: clang/lib/Sema/SemaLambda.cpp
===================================================================
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -966,11 +966,8 @@
if (!Params.empty()) {
CheckParmsForFunctionDef(Params, /*CheckParameterNames=*/false);
Method->setParams(Params);
- for (auto P : Method->parameters()) {
- if (!P)
- continue;
+ for (auto P : Method->parameters())
P->setOwningFunction(Method);
- }
}
buildLambdaScopeReturnType(*this, LSI, Method, HasExplicitResultType);
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -1712,8 +1712,6 @@
/// Check whether a function's parameter types are all literal types. If so,
/// return true. If not, produce a suitable diagnostic and return false.
-/// If any ParamDecl is null, return false without producing a diagnostic.
-/// The code creating null parameters is responsible for producing a diagnostic.
static bool CheckConstexprParameterTypes(Sema &SemaRef,
const FunctionDecl *FD,
Sema::CheckConstexprKind Kind) {
@@ -1723,8 +1721,6 @@
e = FT->param_type_end();
i != e; ++i, ++ArgIndex) {
const ParmVarDecl *PD = FD->getParamDecl(ArgIndex);
- if (!PD)
- return false;
SourceLocation ParamLoc = PD->getLocation();
if (CheckLiteralType(SemaRef, Kind, ParamLoc, *i,
diag::err_constexpr_non_literal_param, ArgIndex + 1,
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -15879,10 +15879,6 @@
bool CheckParameterNames) {
bool HasInvalidParm = false;
for (ParmVarDecl *Param : Parameters) {
- if (!Param) {
- HasInvalidParm = true;
- continue;
- }
// C99 6.7.5.3p4: the parameters in a parameter type list in a
// function declarator that is part of a function definition of
// that function shall not have incomplete type.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
