[flang] Finer control over error recovery with GetExpr()

Prior to this patch, the semantics utility GetExpr() will crash
unconditionally if it encounters a typed expression in the parse
tree that has not been set by expression semantics.  This is the
right behavior when called from lowering, by which time it is known
that the program had no fatal user errors, since it signifies a
fatal internal error.  However, prior to lowering, in the statement
semantics checking code, a more nuanced test should be used before
crashing -- specifically, we should not crash in the face of a
missing typed expression when in error recovery mode.

Getting this right requires GetExpr() and its helper class to have
access to the semantics context, so that it can check AnyFatalErrors()
before crashing.  So this patch touches nearly all of its call sites.

Differential Revision: https://reviews.llvm.org/D123873
This commit is contained in:
Peter Klausler 2022-04-15 13:23:16 -07:00
parent 64c045e25b
commit 7e225423d3
16 changed files with 97 additions and 59 deletions

View File

@ -255,22 +255,25 @@ bool ExprHasTypeCategory(
bool ExprTypeKindIsDefault(
const SomeExpr &expr, const SemanticsContext &context);
struct GetExprHelper {
// Specializations for parse tree nodes that have a typedExpr member.
static const SomeExpr *Get(const parser::Expr &);
static const SomeExpr *Get(const parser::Variable &);
static const SomeExpr *Get(const parser::DataStmtConstant &);
static const SomeExpr *Get(const parser::AllocateObject &);
static const SomeExpr *Get(const parser::PointerObject &);
class GetExprHelper {
public:
explicit GetExprHelper(SemanticsContext *context) : context_{context} {}
GetExprHelper() : crashIfNoExpr_{true} {}
template <typename T>
static const SomeExpr *Get(const common::Indirection<T> &x) {
// Specializations for parse tree nodes that have a typedExpr member.
const SomeExpr *Get(const parser::Expr &);
const SomeExpr *Get(const parser::Variable &);
const SomeExpr *Get(const parser::DataStmtConstant &);
const SomeExpr *Get(const parser::AllocateObject &);
const SomeExpr *Get(const parser::PointerObject &);
template <typename T> const SomeExpr *Get(const common::Indirection<T> &x) {
return Get(x.value());
}
template <typename T> static const SomeExpr *Get(const std::optional<T> &x) {
template <typename T> const SomeExpr *Get(const std::optional<T> &x) {
return x ? Get(*x) : nullptr;
}
template <typename T> static const SomeExpr *Get(const T &x) {
template <typename T> const SomeExpr *Get(const T &x) {
static_assert(
!parser::HasTypedExpr<T>::value, "explicit Get overload must be added");
if constexpr (ConstraintTrait<T>) {
@ -281,8 +284,25 @@ struct GetExprHelper {
return nullptr;
}
}
private:
SemanticsContext *context_{nullptr};
const bool crashIfNoExpr_{false};
};
// If a SemanticsContext is passed, even if null, it is possible for a null
// pointer to be returned in the event of an expression that had fatal errors.
// Use these first two forms in semantics checks for best error recovery.
// If a SemanticsContext is not passed, a missing expression will
// cause a crash.
template <typename T>
const SomeExpr *GetExpr(SemanticsContext *context, const T &x) {
return GetExprHelper{context}.Get(x);
}
template <typename T>
const SomeExpr *GetExpr(SemanticsContext &context, const T &x) {
return GetExprHelper{&context}.Get(x);
}
template <typename T> const SomeExpr *GetExpr(const T &x) {
return GetExprHelper{}.Get(x);
}
@ -292,7 +312,7 @@ const evaluate::Assignment *GetAssignment(
const parser::PointerAssignmentStmt &);
template <typename T> std::optional<std::int64_t> GetIntValue(const T &x) {
if (const auto *expr{GetExpr(x)}) {
if (const auto *expr{GetExpr(nullptr, x)}) {
return evaluate::ToInt64(*expr);
} else {
return std::nullopt;

View File

@ -246,7 +246,7 @@ void AssignmentContext::CheckShape(parser::CharBlock at, const SomeExpr *expr) {
template <typename A> void AssignmentContext::PushWhereContext(const A &x) {
const auto &expr{std::get<parser::LogicalExpr>(x.t)};
CheckShape(expr.thing.value().source, GetExpr(expr));
CheckShape(expr.thing.value().source, GetExpr(context_, expr));
++whereDepth_;
}

View File

@ -187,7 +187,7 @@ static std::optional<AllocateCheckerInfo> CheckAllocateOptions(
}
if (info.gotSource || info.gotMold) {
if (const auto *expr{GetExpr(DEREF(parserSourceExpr))}) {
if (const auto *expr{GetExpr(context, DEREF(parserSourceExpr))}) {
parser::CharBlock at{parserSourceExpr->source};
info.sourceExprType = expr->GetType();
if (!info.sourceExprType) {

View File

@ -25,7 +25,7 @@ void ArithmeticIfStmtChecker::Leave(
// R853 Check for a scalar-numeric-expr
// C849 that shall not be of type complex.
auto &parsedExpr{std::get<parser::Expr>(arithmeticIfStmt.t)};
if (const auto *expr{GetExpr(parsedExpr)}) {
if (const auto *expr{GetExpr(context_, parsedExpr)}) {
if (expr->Rank() > 0) {
context_.Say(parsedExpr.source,
"ARITHMETIC IF expression must be a scalar expression"_err_en_US);

View File

@ -240,7 +240,7 @@ void CaseChecker::Enter(const parser::CaseConstruct &construct) {
const auto &selectCase{selectCaseStmt.statement};
const auto &selectExpr{
std::get<parser::Scalar<parser::Expr>>(selectCase.t).thing};
const auto *x{GetExpr(selectExpr)};
const auto *x{GetExpr(context_, selectExpr)};
if (!x) {
return; // expression semantics failed
}

View File

@ -64,7 +64,7 @@ private:
template <typename T>
static void CheckTeamType(SemanticsContext &context, const T &x) {
if (const auto *expr{GetExpr(x)}) {
if (const auto *expr{GetExpr(context, x)}) {
if (!IsTeamType(evaluate::GetDerivedTypeSpec(expr->GetType()))) {
context.Say(parser::FindSourceLocation(x), // C1114
"Team value must be of type TEAM_TYPE from module ISO_FORTRAN_ENV"_err_en_US);

View File

@ -36,7 +36,7 @@ void DeallocateChecker::Leave(const parser::DeallocateStmt &deallocateStmt) {
[&](const parser::StructureComponent &structureComponent) {
// Only perform structureComponent checks it was successfully
// analyzed in expression analysis.
if (GetExpr(allocateObject)) {
if (GetExpr(context_, allocateObject)) {
if (!IsAllocatableOrPointer(
*structureComponent.component.symbol)) { // C932
context_.Say(structureComponent.component.source,

View File

@ -501,7 +501,7 @@ private:
// Semantic checks for the limit and step expressions
void CheckDoExpression(const parser::ScalarExpr &scalarExpression) {
if (const SomeExpr * expr{GetExpr(scalarExpression)}) {
if (const SomeExpr * expr{GetExpr(context_, scalarExpression)}) {
if (!ExprHasTypeCategory(*expr, TypeCategory::Integer)) {
// No warnings or errors for type INTEGER
const parser::CharBlock &loc{scalarExpression.thing.value().source};
@ -569,10 +569,10 @@ private:
return symbols;
}
static UnorderedSymbolSet GatherSymbolsFromExpression(
const parser::Expr &expression) {
UnorderedSymbolSet GatherSymbolsFromExpression(
const parser::Expr &expression) const {
UnorderedSymbolSet result;
if (const auto *expr{GetExpr(expression)}) {
if (const auto *expr{GetExpr(context_, expression)}) {
for (const Symbol &symbol : evaluate::CollectSymbols(*expr)) {
result.insert(ResolveAssociations(symbol));
}
@ -1022,7 +1022,7 @@ void DoForallChecker::Enter(const parser::Expr &parsedExpr) { ++exprDepth_; }
void DoForallChecker::Leave(const parser::Expr &parsedExpr) {
CHECK(exprDepth_ > 0);
if (--exprDepth_ == 0) { // Only check top level expressions
if (const SomeExpr * expr{GetExpr(parsedExpr)}) {
if (const SomeExpr * expr{GetExpr(context_, parsedExpr)}) {
ActualArgumentSet argSet{CollectActualArguments(*expr)};
for (const evaluate::ActualArgumentRef &argRef : argSet) {
CheckIfArgIsDoVar(*argRef, parsedExpr.source, context_);

View File

@ -209,7 +209,7 @@ void IoChecker::Enter(const parser::Format &spec) {
[&](const parser::Label &) { flags_.set(Flag::LabelFmt); },
[&](const parser::Star &) { flags_.set(Flag::StarFmt); },
[&](const parser::Expr &format) {
const SomeExpr *expr{GetExpr(format)};
const SomeExpr *expr{GetExpr(context_, format)};
if (!expr) {
return;
}
@ -299,7 +299,7 @@ void IoChecker::Enter(const parser::IdExpr &) { SetSpecifier(IoSpecKind::Id); }
void IoChecker::Enter(const parser::IdVariable &spec) {
SetSpecifier(IoSpecKind::Id);
const auto *expr{GetExpr(spec)};
const auto *expr{GetExpr(context_, spec)};
if (!expr || !expr->GetType()) {
return;
}
@ -546,7 +546,7 @@ void IoChecker::Enter(const parser::IoUnit &spec) {
if (stmt_ == IoStmtKind::Write) {
CheckForDefinableVariable(*var, "Internal file");
}
if (const auto *expr{GetExpr(*var)}) {
if (const auto *expr{GetExpr(context_, *var)}) {
if (HasVectorSubscript(*expr)) {
context_.Say(parser::FindSourceLocation(*var), // C1201
"Internal file must not have a vector subscript"_err_en_US);
@ -577,7 +577,7 @@ void IoChecker::Enter(const parser::MsgVariable &var) {
void IoChecker::Enter(const parser::OutputItem &item) {
flags_.set(Flag::DataList);
if (const auto *x{std::get_if<parser::Expr>(&item.u)}) {
if (const auto *expr{GetExpr(*x)}) {
if (const auto *expr{GetExpr(context_, *x)}) {
if (evaluate::IsBOZLiteral(*expr)) {
context_.Say(parser::FindSourceLocation(*x), // C7109
"Output item must not be a BOZ literal constant"_err_en_US);

View File

@ -87,7 +87,7 @@ private:
template <typename R, typename T> std::optional<R> GetConstExpr(const T &x) {
using DefaultCharConstantType = evaluate::Ascii;
if (const SomeExpr * expr{GetExpr(x)}) {
if (const SomeExpr * expr{GetExpr(context_, x)}) {
const auto foldExpr{
evaluate::Fold(context_.foldingContext(), common::Clone(*expr))};
if constexpr (std::is_same_v<R, std::string>) {

View File

@ -40,7 +40,7 @@ void NullifyChecker::Leave(const parser::NullifyStmt &nullifyStmt) {
}
},
[&](const parser::StructureComponent &structureComponent) {
if (const auto *checkedExpr{GetExpr(pointerObject)}) {
if (const auto *checkedExpr{GetExpr(context_, pointerObject)}) {
if (!IsPointer(*structureComponent.component.symbol)) { // C951
messages.Say(structureComponent.component.source,
"component in NULLIFY statement must have the POINTER attribute"_err_en_US);

View File

@ -50,8 +50,8 @@ public:
bool Pre(const parser::AssignmentStmt &assignment) {
const auto &var{std::get<parser::Variable>(assignment.t)};
const auto &expr{std::get<parser::Expr>(assignment.t)};
const auto *lhs{GetExpr(var)};
const auto *rhs{GetExpr(expr)};
const auto *lhs{GetExpr(context_, var)};
const auto *rhs{GetExpr(context_, expr)};
if (lhs && rhs) {
Tristate isDefined{semantics::IsDefinedAssignment(
lhs->GetType(), lhs->Rank(), rhs->GetType(), rhs->Rank())};
@ -65,7 +65,7 @@ public:
}
bool Pre(const parser::Expr &expr) {
if (const auto *e{GetExpr(expr)}) {
if (const auto *e{GetExpr(context_, expr)}) {
for (const Symbol &symbol : evaluate::CollectSymbols(*e)) {
const Symbol &root{GetAssociationRoot(symbol)};
if (IsFunction(root) &&
@ -1467,7 +1467,7 @@ void OmpStructureChecker::CheckAtomicUpdateAssignmentStmt(
if (const auto *name =
std::get_if<Fortran::parser::Name>(&dataRef->u)) {
const auto &varSymbol = *name->symbol;
if (const auto *e{GetExpr(expr)}) {
if (const auto *e{GetExpr(context_, expr)}) {
for (const Symbol &symbol :
evaluate::CollectSymbols(*e)) {
if (symbol == varSymbol) {

View File

@ -18,7 +18,7 @@ namespace Fortran::semantics {
void StopChecker::Enter(const parser::StopStmt &stmt) {
const auto &stopCode{std::get<std::optional<parser::StopCode>>(stmt.t)};
if (const auto *expr{GetExpr(stopCode)}) {
if (const auto *expr{GetExpr(context_, stopCode)}) {
const parser::CharBlock &source{parser::FindSourceLocation(stopCode)};
if (ExprHasTypeCategory(*expr, common::TypeCategory::Integer)) {
// C1171 default kind

View File

@ -36,13 +36,13 @@ namespace Fortran::semantics {
// repetition.
template <typename DSV = parser::DataStmtValue> class ValueListIterator {
public:
explicit ValueListIterator(const std::list<DSV> &list)
: end_{list.end()}, at_{list.begin()} {
ValueListIterator(SemanticsContext &context, const std::list<DSV> &list)
: context_{context}, end_{list.end()}, at_{list.begin()} {
SetRepetitionCount();
}
bool hasFatalError() const { return hasFatalError_; }
bool IsAtEnd() const { return at_ == end_; }
const SomeExpr *operator*() const { return GetExpr(GetConstant()); }
const SomeExpr *operator*() const { return GetExpr(context_, GetConstant()); }
parser::CharBlock LocateSource() const { return GetConstant().source; }
ValueListIterator &operator++() {
if (repetitionsRemaining_ > 0) {
@ -64,6 +64,7 @@ private:
return std::get<parser::DataStmtConstant>(GetValue().t);
}
SemanticsContext &context_;
listIterator end_, at_;
ConstantSubscript repetitionsRemaining_{0};
bool hasFatalError_{false};
@ -93,7 +94,7 @@ class DataInitializationCompiler {
public:
DataInitializationCompiler(DataInitializations &inits,
evaluate::ExpressionAnalyzer &a, const std::list<DSV> &list)
: inits_{inits}, exprAnalyzer_{a}, values_{list} {}
: inits_{inits}, exprAnalyzer_{a}, values_{a.context(), list} {}
const DataInitializations &inits() const { return inits_; }
bool HasSurplusValues() const { return !values_.IsAtEnd(); }
bool Scan(const parser::DataStmtObject &);
@ -134,7 +135,7 @@ bool DataInitializationCompiler<DSV>::Scan(
template <typename DSV>
bool DataInitializationCompiler<DSV>::Scan(const parser::Variable &var) {
if (const auto *expr{GetExpr(var)}) {
if (const auto *expr{GetExpr(exprAnalyzer_.context(), var)}) {
exprAnalyzer_.GetFoldingContext().messages().SetLocation(var.GetSource());
if (InitDesignator(*expr)) {
return true;
@ -160,10 +161,13 @@ template <typename DSV>
bool DataInitializationCompiler<DSV>::Scan(const parser::DataImpliedDo &ido) {
const auto &bounds{std::get<parser::DataImpliedDo::Bounds>(ido.t)};
auto name{bounds.name.thing.thing};
const auto *lowerExpr{GetExpr(bounds.lower.thing.thing)};
const auto *upperExpr{GetExpr(bounds.upper.thing.thing)};
const auto *stepExpr{
bounds.step ? GetExpr(bounds.step->thing.thing) : nullptr};
const auto *lowerExpr{
GetExpr(exprAnalyzer_.context(), bounds.lower.thing.thing)};
const auto *upperExpr{
GetExpr(exprAnalyzer_.context(), bounds.upper.thing.thing)};
const auto *stepExpr{bounds.step
? GetExpr(exprAnalyzer_.context(), bounds.step->thing.thing)
: nullptr};
if (lowerExpr && upperExpr) {
// Fold the bounds expressions (again) in case any of them depend
// on outer implied DO loops.

View File

@ -279,6 +279,18 @@ public:
return true;
}
bool Pre(const parser::StmtFunctionStmt &x) {
const auto &parsedExpr{std::get<parser::Scalar<parser::Expr>>(x.t)};
if (const auto *expr{GetExpr(context_, parsedExpr)}) {
for (const Symbol &symbol : evaluate::CollectSymbols(*expr)) {
if (!IsStmtFunctionDummy(symbol)) {
stmtFunctionExprSymbols_.insert(symbol.GetUltimate());
}
}
}
return true;
}
bool Pre(const parser::OpenMPBlockConstruct &);
void Post(const parser::OpenMPBlockConstruct &);

View File

@ -385,8 +385,9 @@ bool ExprTypeKindIsDefault(
// If an analyzed expr or assignment is missing, dump the node and die.
template <typename T>
static void CheckMissingAnalysis(bool absent, const T &x) {
if (absent) {
static void CheckMissingAnalysis(
bool crash, SemanticsContext *context, const T &x) {
if (crash && !(context && context->AnyFatalError())) {
std::string buf;
llvm::raw_string_ostream ss{buf};
ss << "node has not been analyzed:\n";
@ -395,34 +396,35 @@ static void CheckMissingAnalysis(bool absent, const T &x) {
}
}
template <typename T> static const SomeExpr *GetTypedExpr(const T &x) {
CheckMissingAnalysis(!x.typedExpr, x);
return common::GetPtrFromOptional(x.typedExpr->v);
}
const SomeExpr *GetExprHelper::Get(const parser::Expr &x) {
return GetTypedExpr(x);
CheckMissingAnalysis(crashIfNoExpr_ && !x.typedExpr, context_, x);
return x.typedExpr ? common::GetPtrFromOptional(x.typedExpr->v) : nullptr;
}
const SomeExpr *GetExprHelper::Get(const parser::Variable &x) {
return GetTypedExpr(x);
CheckMissingAnalysis(crashIfNoExpr_ && !x.typedExpr, context_, x);
return x.typedExpr ? common::GetPtrFromOptional(x.typedExpr->v) : nullptr;
}
const SomeExpr *GetExprHelper::Get(const parser::DataStmtConstant &x) {
return GetTypedExpr(x);
CheckMissingAnalysis(crashIfNoExpr_ && !x.typedExpr, context_, x);
return x.typedExpr ? common::GetPtrFromOptional(x.typedExpr->v) : nullptr;
}
const SomeExpr *GetExprHelper::Get(const parser::AllocateObject &x) {
return GetTypedExpr(x);
CheckMissingAnalysis(crashIfNoExpr_ && !x.typedExpr, context_, x);
return x.typedExpr ? common::GetPtrFromOptional(x.typedExpr->v) : nullptr;
}
const SomeExpr *GetExprHelper::Get(const parser::PointerObject &x) {
return GetTypedExpr(x);
CheckMissingAnalysis(crashIfNoExpr_ && !x.typedExpr, context_, x);
return x.typedExpr ? common::GetPtrFromOptional(x.typedExpr->v) : nullptr;
}
const evaluate::Assignment *GetAssignment(const parser::AssignmentStmt &x) {
CheckMissingAnalysis(!x.typedAssignment, x);
return common::GetPtrFromOptional(x.typedAssignment->v);
return x.typedAssignment ? common::GetPtrFromOptional(x.typedAssignment->v)
: nullptr;
}
const evaluate::Assignment *GetAssignment(
const parser::PointerAssignmentStmt &x) {
CheckMissingAnalysis(!x.typedAssignment, x);
return common::GetPtrFromOptional(x.typedAssignment->v);
return x.typedAssignment ? common::GetPtrFromOptional(x.typedAssignment->v)
: nullptr;
}
const Symbol *FindInterface(const Symbol &symbol) {
@ -998,7 +1000,7 @@ parser::CharBlock GetImageControlStmtLocation(
}
bool HasCoarray(const parser::Expr &expression) {
if (const auto *expr{GetExpr(expression)}) {
if (const auto *expr{GetExpr(nullptr, expression)}) {
for (const Symbol &symbol : evaluate::CollectSymbols(*expr)) {
if (evaluate::IsCoarray(symbol)) {
return true;