Skip to content

Commit

Permalink
[clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueVa…
Browse files Browse the repository at this point in the history 
…lueExprs

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
  • Loading branch information
@kadircet
kadircet committed Aug 16, 2023
1 parent 83695d4 commit 373fcd5
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 22 deletions.
13 changes: 13 additions & 0 deletions clang-tools-extra/clangd/unittests/HoverTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion clang/include/clang/Sema/Sema.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions clang/lib/Parse/ParseCXXInlineMethods.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions clang/lib/Parse/ParseDecl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
35 changes: 19 additions & 16 deletions clang/lib/Sema/SemaDeclCXX.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
Expand All @@ -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
Expand Down
8 changes: 8 additions & 0 deletions clang/test/AST/ast-dump-default-arg-recovery.cpp
View file
Original file line number Diff line number Diff line change
@@ -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'
2 changes: 1 addition & 1 deletion clang/test/Index/complete-optional-params.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 373fcd5

Please sign in to comment.