[clang] 373fcd5 - [clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueValueExprs
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 16 01:32:51 PDT 2023
Author: Kadir Cetinkaya
Date: 2023-08-16T10:22:16+02:00
New Revision: 373fcd5d73a3ed5bedff771bcf6a3aba981155cc
URL: https://github.com/llvm/llvm-project/commit/373fcd5d73a3ed5bedff771bcf6a3aba981155cc
DIFF: https://github.com/llvm/llvm-project/commit/373fcd5d73a3ed5bedff771bcf6a3aba981155cc.diff
LOG: [clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueValueExprs
This makes sure we can preserve invalid-ness for consumers of this
node, it prevents crashes. It also aligns better with rest of the places that
store invalid expressions.
Differential Revision: https://reviews.llvm.org/D157868
Added:
clang/test/AST/ast-dump-default-arg-recovery.cpp
Modified:
clang-tools-extra/clangd/unittests/HoverTests.cpp
clang/include/clang/Sema/Sema.h
clang/lib/Parse/ParseCXXInlineMethods.cpp
clang/lib/Parse/ParseDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/Index/complete-optional-params.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 2ad7f0ee55d726..9cab9a086958d6 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2543,6 +2543,19 @@ TEST(Hover, All) {
HI.Type = "Test &&";
HI.Definition = "Test &&test = {}";
}},
+ {
+ R"cpp(// Shouldn't crash when evaluating the initializer.
+ struct Bar {}; // error-ok
+ struct Foo { void foo(Bar x = y); }
+ void Foo::foo(Bar [[^x]]) {})cpp",
+ [](HoverInfo &HI) {
+ HI.Name = "x";
+ HI.Kind = index::SymbolKind::Parameter;
+ HI.NamespaceScope = "";
+ HI.LocalScope = "Foo::foo::";
+ HI.Type = "Bar";
+ HI.Definition = "Bar x = <recovery - expr>()";
+ }},
{
R"cpp(// auto on alias
typedef int int_type;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 8a63f10a6e1561..05a0e43b5b73d9 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3022,7 +3022,8 @@ class Sema final {
Expr *defarg);
void ActOnParamUnparsedDefaultArgument(Decl *param, SourceLocation EqualLoc,
SourceLocation ArgLoc);
- void ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc);
+ void ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc,
+ Expr* DefaultArg);
ExprResult ConvertParamDefaultArgument(ParmVarDecl *Param, Expr *DefaultArg,
SourceLocation EqualLoc);
void SetParamDefaultArgument(ParmVarDecl *Param, Expr *DefaultArg,
diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp
index 4951eb9aa2802a..573c90a36eeab3 100644
--- a/clang/lib/Parse/ParseCXXInlineMethods.cpp
+++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp
@@ -395,9 +395,10 @@ void Parser::ParseLexedMethodDeclaration(LateParsedMethodDeclaration &LM) {
DefArgResult = ParseBraceInitializer();
} else
DefArgResult = ParseAssignmentExpression();
- DefArgResult = Actions.CorrectDelayedTyposInExpr(DefArgResult);
+ DefArgResult = Actions.CorrectDelayedTyposInExpr(DefArgResult, Param);
if (DefArgResult.isInvalid()) {
- Actions.ActOnParamDefaultArgumentError(Param, EqualLoc);
+ Actions.ActOnParamDefaultArgumentError(Param, EqualLoc,
+ /*DefaultArg=*/nullptr);
} else {
if (Tok.isNot(tok::eof) || Tok.getEofData() != Param) {
// The last two tokens are the terminator and the saved value of
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index b5a3ee1eaf076d..cd7c5dcf275c04 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -7548,7 +7548,8 @@ void Parser::ParseParameterDeclarationClause(
} else {
if (Tok.is(tok::l_paren) && NextToken().is(tok::l_brace)) {
Diag(Tok, diag::err_stmt_expr_in_default_arg) << 0;
- Actions.ActOnParamDefaultArgumentError(Param, EqualLoc);
+ Actions.ActOnParamDefaultArgumentError(Param, EqualLoc,
+ /*DefaultArg=*/nullptr);
// Skip the statement expression and continue parsing
SkipUntil(tok::comma, StopBeforeMatch);
continue;
@@ -7557,7 +7558,8 @@ void Parser::ParseParameterDeclarationClause(
}
DefArgResult = Actions.CorrectDelayedTyposInExpr(DefArgResult);
if (DefArgResult.isInvalid()) {
- Actions.ActOnParamDefaultArgumentError(Param, EqualLoc);
+ Actions.ActOnParamDefaultArgumentError(Param, EqualLoc,
+ /*DefaultArg=*/nullptr);
SkipUntil(tok::comma, tok::r_paren, StopAtSemi | StopBeforeMatch);
} else {
// Inform the actions module about the default argument
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 9b95af722d398f..a7ab0948d01bf3 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -20,6 +20,7 @@
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/EvaluatedExprVisitor.h"
+#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/RecordLayout.h"
#include "clang/AST/RecursiveASTVisitor.h"
@@ -37,11 +38,13 @@
#include "clang/Sema/EnterExpressionEvaluationContext.h"
#include "clang/Sema/Initialization.h"
#include "clang/Sema/Lookup.h"
+#include "clang/Sema/Ownership.h"
#include "clang/Sema/ParsedTemplate.h"
#include "clang/Sema/Scope.h"
#include "clang/Sema/ScopeInfo.h"
#include "clang/Sema/SemaInternal.h"
#include "clang/Sema/Template.h"
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SmallString.h"
@@ -329,23 +332,16 @@ Sema::ActOnParamDefaultArgument(Decl *param, SourceLocation EqualLoc,
ParmVarDecl *Param = cast<ParmVarDecl>(param);
UnparsedDefaultArgLocs.erase(Param);
- auto Fail = [&] {
- Param->setInvalidDecl();
- Param->setDefaultArg(new (Context) OpaqueValueExpr(
- EqualLoc, Param->getType().getNonReferenceType(), VK_PRValue));
- };
-
// Default arguments are only permitted in C++
if (!getLangOpts().CPlusPlus) {
Diag(EqualLoc, diag::err_param_default_argument)
<< DefaultArg->getSourceRange();
- return Fail();
+ return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
}
// Check for unexpanded parameter packs.
- if (DiagnoseUnexpandedParameterPack(DefaultArg, UPPC_DefaultArgument)) {
- return Fail();
- }
+ if (DiagnoseUnexpandedParameterPack(DefaultArg, UPPC_DefaultArgument))
+ return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
// C++11 [dcl.fct.default]p3
// A default argument expression [...] shall not be specified for a
@@ -360,14 +356,14 @@ Sema::ActOnParamDefaultArgument(Decl *param, SourceLocation EqualLoc,
ExprResult Result = ConvertParamDefaultArgument(Param, DefaultArg, EqualLoc);
if (Result.isInvalid())
- return Fail();
+ return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
DefaultArg = Result.getAs<Expr>();
// Check that the default argument is well-formed
CheckDefaultArgumentVisitor DefaultArgChecker(*this, DefaultArg);
if (DefaultArgChecker.Visit(DefaultArg))
- return Fail();
+ return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
SetParamDefaultArgument(Param, DefaultArg, EqualLoc);
}
@@ -389,16 +385,23 @@ void Sema::ActOnParamUnparsedDefaultArgument(Decl *param,
/// ActOnParamDefaultArgumentError - Parsing or semantic analysis of
/// the default argument for the parameter param failed.
-void Sema::ActOnParamDefaultArgumentError(Decl *param,
- SourceLocation EqualLoc) {
+void Sema::ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc,
+ Expr *DefaultArg) {
if (!param)
return;
ParmVarDecl *Param = cast<ParmVarDecl>(param);
Param->setInvalidDecl();
UnparsedDefaultArgLocs.erase(Param);
- Param->setDefaultArg(new (Context) OpaqueValueExpr(
- EqualLoc, Param->getType().getNonReferenceType(), VK_PRValue));
+ ExprResult RE;
+ if (DefaultArg) {
+ RE = CreateRecoveryExpr(EqualLoc, DefaultArg->getEndLoc(), {DefaultArg},
+ Param->getType().getNonReferenceType());
+ } else {
+ RE = CreateRecoveryExpr(EqualLoc, EqualLoc, {},
+ Param->getType().getNonReferenceType());
+ }
+ Param->setDefaultArg(RE.get());
}
/// CheckExtraCXXDefaultArguments - Check for any extra default
diff --git a/clang/test/AST/ast-dump-default-arg-recovery.cpp b/clang/test/AST/ast-dump-default-arg-recovery.cpp
new file mode 100644
index 00000000000000..5ced0ffd826abf
--- /dev/null
+++ b/clang/test/AST/ast-dump-default-arg-recovery.cpp
@@ -0,0 +1,8 @@
+// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -ast-dump -frecovery-ast %s | FileCheck %s
+
+void foo();
+void fun(int arg = foo());
+// CHECK: -ParmVarDecl {{.*}} <col:10, col:24> col:14 invalid arg 'int' cinit
+// CHECK-NEXT: -RecoveryExpr {{.*}} <col:18, col:24> 'int' contains-errors
+// Make sure we also preserve structure of the errorneous expression
+// CHECK: -DeclRefExpr {{.*}} <col:20> 'void ()' {{.*}} 'foo'
diff --git a/clang/test/Index/complete-optional-params.cpp b/clang/test/Index/complete-optional-params.cpp
index 1df2975300a11a..8261a4a143db98 100644
--- a/clang/test/Index/complete-optional-params.cpp
+++ b/clang/test/Index/complete-optional-params.cpp
@@ -79,7 +79,7 @@ int main() {
// CHECK-CC5-NEXT: Objective-C interface
// RUN: c-index-test -code-completion-at=%s:17:11 %s | FileCheck -check-prefix=CHECK-CC6 %s
-// CHECK-CC6: FunctionDecl:{ResultType void}{TypedText foo_2}{LeftParen (}{Optional {Placeholder Bar1 b1 = Bar1()}{Optional {Comma , }{Placeholder Bar2 b2}}}{RightParen )} (50)
+// CHECK-CC6: FunctionDecl:{ResultType void}{TypedText foo_2}{LeftParen (}{Optional {Placeholder Bar1 b1 = Bar1()}{Optional {Comma , }{Placeholder Bar2 b2 = Bar2()}}}{RightParen )} (50)
// CHECK-CC6: Completion contexts:
// CHECK-CC6-NEXT: Any type
// CHECK-CC6-NEXT: Any value
More information about the cfe-commits
mailing list