[clang] f27b77e - [Sema] Populate declarations inside TypeLocs for some invalid types
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 6 10:02:51 PDT 2023
Author: Ilya Biryukov
Date: 2023-04-06T19:02:41+02:00
New Revision: f27b77e5c59a1a907fa45eeb5f2e279a5c8bd17d
URL: https://github.com/llvm/llvm-project/commit/f27b77e5c59a1a907fa45eeb5f2e279a5c8bd17d
DIFF: https://github.com/llvm/llvm-project/commit/f27b77e5c59a1a907fa45eeb5f2e279a5c8bd17d.diff
LOG: [Sema] Populate declarations inside TypeLocs for some invalid types
This also reverts 282cae0b9a602267ad7ef622f770066491332a11 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.
Reviewed By: aaron.ballman, erichkeane
Differential Revision: https://reviews.llvm.org/D146971
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 16f29be4947fb..f66eb9fcf13dc 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -15884,10 +15884,7 @@ bool Sema::CheckParmsForFunctionDef(ArrayRef<ParmVarDecl *> Parameters,
bool CheckParameterNames) {
bool HasInvalidParm = false;
for (ParmVarDecl *Param : Parameters) {
- if (!Param) {
- HasInvalidParm = true;
- continue;
- }
+ assert(Param && "null in a parameter list");
// 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.
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 312b6f801c1f0..511e87f229254 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -1712,8 +1712,6 @@ static bool CheckConstexprDestructorSubobjects(Sema &SemaRef,
/// 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,7 @@ static bool CheckConstexprParameterTypes(Sema &SemaRef,
e = FT->param_type_end();
i != e; ++i, ++ArgIndex) {
const ParmVarDecl *PD = FD->getParamDecl(ArgIndex);
- if (!PD)
- return false;
+ assert(PD && "null in a parameter list");
SourceLocation ParamLoc = PD->getLocation();
if (CheckLiteralType(SemaRef, Kind, ParamLoc, *i,
diag::err_constexpr_non_literal_param, ArgIndex + 1,
diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index a809968b66032..7f9bec2f7627e 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -956,8 +956,7 @@ void Sema::CompleteLambdaCallOperator(
CheckParmsForFunctionDef(Params, /*CheckParameterNames=*/false);
Method->setParams(Params);
for (auto P : Method->parameters()) {
- if (!P)
- continue;
+ assert(P && "null in a parameter list");
P->setOwningFunction(Method);
}
}
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index d88748761c148..bdcee77ce67c7 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1355,7 +1355,8 @@ namespace {
CXXMethodDecl *MD = Result.getAs<LambdaExpr>()->getCallOperator();
for (ParmVarDecl *PVD : MD->parameters()) {
- if (!PVD || !PVD->hasDefaultArg())
+ assert(PVD && "null in a parameter list");
+ if (!PVD->hasDefaultArg())
continue;
Expr *UninstExpr = PVD->getUninstantiatedDefaultArg();
// FIXME: Obtain the source location for the '=' token.
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index e195e85ab75b7..f557ff7b203fa 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -4979,6 +4979,12 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
// 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);
@@ -5172,6 +5178,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
: 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);
@@ -5182,6 +5189,8 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
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) {
@@ -5189,6 +5198,8 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
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) ||
@@ -5199,6 +5210,8 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
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()) {
@@ -5239,6 +5252,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
S.Diag(DeclType.Loc, diagID) << T->isFunctionType() << T;
T = Context.IntTy;
D.setInvalidType(true);
+ AreDeclaratorChunksValid = false;
}
// Do not allow returning half FP value.
@@ -5305,6 +5319,8 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
ObjCObjectPointerTypeLoc TLoc = TLB.push<ObjCObjectPointerTypeLoc>(T);
TLoc.setStarLoc(FixitLoc);
TInfo = TLB.getTypeSourceInfo(Context, T);
+ } else {
+ AreDeclaratorChunksValid = false;
}
D.setInvalidType(true);
@@ -5425,6 +5441,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
T = (!LangOpts.requiresStrictPrototypes() && !LangOpts.OpenCL)
? Context.getFunctionNoProtoType(T, EI)
: Context.IntTy;
+ AreDeclaratorChunksValid = false;
break;
}
@@ -5659,9 +5676,13 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
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);
}
@@ -5679,6 +5700,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
if (T.isNull()) {
D.setInvalidType(true);
T = Context.IntTy;
+ AreDeclaratorChunksValid = false;
}
// See if there are any attributes on this declarator chunk.
@@ -5937,9 +5959,8 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
}
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);
}
diff --git a/clang/test/Index/complete-blocks.m b/clang/test/Index/complete-blocks.m
index 9c6c1cbf86db2..bd65e1f91339d 100644
--- a/clang/test/Index/complete-blocks.m
+++ b/clang/test/Index/complete-blocks.m
@@ -85,4 +85,4 @@ void testUnresolved(Foo* f) {
// 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)
More information about the cfe-commits
mailing list