[flang][NFC] Change how error symbols are recorded

When an error is associated with a symbol, it was marked with a flag
from Symbol::Flag. The problem with that is that you need a mutable
symbol to do that. Instead, store the set of error symbols in the
SemanticsContext. This allows for some const_casts to be eliminated.

Also, improve the internal error that occurs if SetError is called
but no fatal error has been reported.

Differential Revision: https://reviews.llvm.org/D86740

flang/include/flang/Semantics/semantics.h

@@ -131,7 +131,7 @@ public:
   bool HasError(const Symbol &);
   bool HasError(const Symbol *);
   bool HasError(const parser::Name &);
-  void SetError(Symbol &, bool = true);
+  void SetError(const Symbol &, bool = true);
 
   template <typename... A> parser::Message &Say(A &&...args) {
     CHECK(location_);
@@ -175,7 +175,7 @@ public:
 private:
   void CheckIndexVarRedefine(
       const parser::CharBlock &, const Symbol &, parser::MessageFixedText &&);
-  bool CheckError(bool);
+  void CheckError(const Symbol &);
 
   const common::IntrinsicTypeDefaultKinds &defaultKinds_;
   const common::LanguageFeatureControl languageFeatures_;
@@ -197,6 +197,7 @@ private:
     IndexVarKind kind;
   };
   std::map<SymbolRef, const IndexVarInfo> activeIndexVars_;
+  std::set<SymbolRef> errorSymbols_;
   std::vector<std::string> tempNames_;
 };
 

flang/include/flang/Semantics/symbol.h

@@ -478,7 +478,6 @@ std::string DetailsToString(const Details &);
 class Symbol {
 public:
   ENUM_CLASS(Flag,
-      Error, // an error has been reported on this symbol
       Function, // symbol is a function
       Subroutine, // symbol is a subroutine
       StmtFunction, // symbol is a statement function (Function is set too)

flang/include/flang/Semantics/type.h

@@ -263,7 +263,7 @@ public:
   // Converts the raw parameter list to a map, naming each actual parameter.
   void CookParameters(evaluate::FoldingContext &);
   // Evaluates type parameter expressions.
-  void EvaluateParameters(evaluate::FoldingContext &);
+  void EvaluateParameters(SemanticsContext &);
   void AddParamValue(SourceName, ParamValue &&);
   // Creates a Scope for the type and populates it with component
   // instantiations that have been specialized with actual type parameter

flang/lib/Semantics/expression.cpp

@@ -922,7 +922,7 @@ MaybeExpr ExpressionAnalyzer::Analyze(const parser::ArrayElement &ae) {
       if (const Symbol * symbol{GetLastSymbol(*baseExpr)}) {
         if (!context_.HasError(symbol)) {
           Say("'%s' is not an array"_err_en_US, symbol->name());
-          context_.SetError(const_cast<Symbol &>(*symbol));
+          context_.SetError(*symbol);
         }
       }
     } else if (std::optional<DataRef> dataRef{
@@ -1824,13 +1824,13 @@ bool ExpressionAnalyzer::ResolveForward(const Symbol &symbol) {
         // allowed in specification parts (10.1.11 para 5).
         Say("The module function '%s' may not be referenced recursively in a specification expression"_err_en_US,
             symbol.name());
-        context_.SetError(const_cast<Symbol &>(symbol));
+        context_.SetError(symbol);
         return false;
       }
     } else { // 10.1.11 para 4
       Say("The internal function '%s' may not be referenced in a specification expression"_err_en_US,
           symbol.name());
-      context_.SetError(const_cast<Symbol &>(symbol));
+      context_.SetError(symbol);
       return false;
     }
   }

flang/lib/Semantics/resolve-names.cpp

@@ -3598,7 +3598,7 @@ void DeclarationVisitor::Post(const parser::DerivedTypeSpec &x) {
   // Normalize parameters to produce a better search key.
   spec->CookParameters(GetFoldingContext());
   if (!spec->MightBeParameterized()) {
-    spec->EvaluateParameters(GetFoldingContext());
+    spec->EvaluateParameters(context());
   }
   if (const DeclTypeSpec *
       extant{currScope().FindInstantiatedDerivedType(*spec, category)}) {
@@ -3647,7 +3647,7 @@ bool DeclarationVisitor::Pre(const parser::DerivedTypeDef &x) {
       BeginAttrs();
       Symbol *typeParam{MakeTypeSymbol(
           paramName, TypeParamDetails{common::TypeParamAttr::Len})};
-      typeParam->set(Symbol::Flag::Error);
+      context().SetError(*typeParam);
       EndAttrs();
     } else if (!symbol->has<TypeParamDetails>()) {
       Say2(paramName, "'%s' is not defined as a type parameter"_err_en_US,
@@ -4778,7 +4778,7 @@ bool DeclarationVisitor::OkToAddComponent(
   for (const Scope *scope{&currScope()}; scope;) {
     CHECK(scope->IsDerivedType());
     if (auto *prev{FindInScope(*scope, name)}) {
-      if (!prev->test(Symbol::Flag::Error)) {
+      if (!context().HasError(*prev)) {
         auto msg{""_en_US};
         if (extends) {
           msg = "Type cannot be extended as it has a component named"

flang/lib/Semantics/semantics.cpp

@@ -221,23 +221,28 @@ bool SemanticsContext::AnyFatalError() const {
       (warningsAreErrors_ || messages_.AnyFatalError());
 }
 bool SemanticsContext::HasError(const Symbol &symbol) {
-  return CheckError(symbol.test(Symbol::Flag::Error));
+  return errorSymbols_.count(symbol) > 0;
 }
 bool SemanticsContext::HasError(const Symbol *symbol) {
-  return CheckError(!symbol || HasError(*symbol));
+  return !symbol || HasError(*symbol);
 }
 bool SemanticsContext::HasError(const parser::Name &name) {
   return HasError(name.symbol);
 }
-void SemanticsContext::SetError(Symbol &symbol, bool value) {
+void SemanticsContext::SetError(const Symbol &symbol, bool value) {
   if (value) {
-    CHECK(AnyFatalError());
-    symbol.set(Symbol::Flag::Error);
+    CheckError(symbol);
+    errorSymbols_.emplace(symbol);
   }
 }
-bool SemanticsContext::CheckError(bool error) {
-  CHECK(!error || AnyFatalError());
-  return error;
+void SemanticsContext::CheckError(const Symbol &symbol) {
+  if (!AnyFatalError()) {
+    std::string buf;
+    llvm::raw_string_ostream ss{buf};
+    ss << symbol;
+    common::die(
+        "No error was reported but setting error on: %s", ss.str().c_str());
+  }
 }
 
 const Scope &SemanticsContext::FindScope(parser::CharBlock source) const {

flang/lib/Semantics/tools.cpp

@@ -965,8 +965,7 @@ SymbolVector OrderParameterDeclarations(const Symbol &typeSymbol) {
 const DeclTypeSpec &FindOrInstantiateDerivedType(Scope &scope,
     DerivedTypeSpec &&spec, SemanticsContext &semanticsContext,
     DeclTypeSpec::Category category) {
-  spec.CookParameters(semanticsContext.foldingContext());
-  spec.EvaluateParameters(semanticsContext.foldingContext());
+  spec.EvaluateParameters(semanticsContext);
   if (const DeclTypeSpec *
       type{scope.FindInstantiatedDerivedType(spec, category)}) {
     return *type;

flang/lib/Semantics/type.cpp

@@ -97,8 +97,8 @@ void DerivedTypeSpec::CookParameters(evaluate::FoldingContext &foldingContext) {
   }
 }
 
-void DerivedTypeSpec::EvaluateParameters(
-    evaluate::FoldingContext &foldingContext) {
+void DerivedTypeSpec::EvaluateParameters(SemanticsContext &context) {
+  evaluate::FoldingContext &foldingContext{context.foldingContext()};
   CookParameters(foldingContext);
   if (evaluated_) {
     return;
@@ -123,7 +123,7 @@ void DerivedTypeSpec::EvaluateParameters(
             continue;
           }
         }
-        if (!symbol.test(Symbol::Flag::Error)) {
+        if (!context.HasError(symbol)) {
           evaluate::SayWithDeclaration(messages, symbol,
               "Value of type parameter '%s' (%s) is not convertible to its"
               " type"_err_en_US,
@@ -150,7 +150,7 @@ void DerivedTypeSpec::EvaluateParameters(
         auto expr{
             evaluate::Fold(foldingContext, common::Clone(details.init()))};
         AddParamValue(name, ParamValue{std::move(*expr), details.attr()});
-      } else if (!symbol.test(Symbol::Flag::Error)) {
+      } else if (!context.HasError(symbol)) {
         messages.Say(name_,
             "Type parameter '%s' lacks a value and has no default"_err_en_US,
             name);
@@ -220,8 +220,7 @@ void DerivedTypeSpec::Instantiate(
         typeSymbol_.name());
     return;
   }
-  CookParameters(foldingContext);
-  EvaluateParameters(foldingContext);
+  EvaluateParameters(context);
   const Scope &typeScope{DEREF(typeSymbol_.scope())};
   if (!MightBeParameterized()) {
     scope_ = &typeScope;