[flang-commits] [flang] f0829e7 - [flang] Enforce C815

Peter Klausler via flang-commits flang-commits at lists.llvm.org
Sun Oct 30 14:50:43 PDT 2022


Author: Peter Klausler
Date: 2022-10-30T14:50:31-07:00
New Revision: f0829e7b9575be239c1faee55ad03ad729fe8199

URL: https://github.com/llvm/llvm-project/commit/f0829e7b9575be239c1faee55ad03ad729fe8199
DIFF: https://github.com/llvm/llvm-project/commit/f0829e7b9575be239c1faee55ad03ad729fe8199.diff

LOG: [flang] Enforce C815

A Fortran program may not specify a particular attribute multiple
times for the same entity in a scope.

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

Added: 
    

Modified: 
    flang/include/flang/Semantics/symbol.h
    flang/lib/Semantics/resolve-names.cpp
    flang/test/Lower/call-site-mangling.f90
    flang/test/Semantics/misc-declarations.f90
    flang/test/Semantics/resolve20.f90
    flang/test/Semantics/resolve91.f90

Removed: 
    


################################################################################
diff  --git a/flang/include/flang/Semantics/symbol.h b/flang/include/flang/Semantics/symbol.h
index a81dd66575ebd..c7bd58c40bd63 100644
--- a/flang/include/flang/Semantics/symbol.h
+++ b/flang/include/flang/Semantics/symbol.h
@@ -559,6 +559,8 @@ class Symbol {
   const SourceName &name() const { return name_; }
   Attrs &attrs() { return attrs_; }
   const Attrs &attrs() const { return attrs_; }
+  Attrs &implicitAttrs() { return implicitAttrs_; }
+  const Attrs &implicitAttrs() const { return implicitAttrs_; }
   Flags &flags() { return flags_; }
   const Flags &flags() const { return flags_; }
   bool test(Flag flag) const { return flags_.test(flag); }
@@ -685,6 +687,7 @@ class Symbol {
   const Scope *owner_;
   SourceName name_;
   Attrs attrs_;
+  Attrs implicitAttrs_; // subset of attrs_ that were not explicit
   Flags flags_;
   Scope *scope_{nullptr};
   std::size_t size_{0}; // size in bytes

diff  --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 58d31b9b5a19a..5612f1f642c13 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -598,7 +598,8 @@ class ScopeHandler : public ImplicitRulesVisitor {
             d->set_derivedType(*derivedType);
           } else if (derivedType->CanReplaceDetails(details)) {
             // was forward-referenced
-            derivedType->attrs() |= attrs;
+            CheckDuplicatedAttrs(name, *symbol, attrs);
+            SetExplicitAttrs(*derivedType, attrs);
             derivedType->set_details(std::move(details));
           } else {
             SayAlreadyDeclared(name, *derivedType);
@@ -609,7 +610,8 @@ class ScopeHandler : public ImplicitRulesVisitor {
     }
     if (symbol->CanReplaceDetails(details)) {
       // update the existing symbol
-      symbol->attrs() |= attrs;
+      CheckDuplicatedAttrs(name, *symbol, attrs);
+      SetExplicitAttrs(*symbol, attrs);
       if constexpr (std::is_same_v<SubprogramDetails, D>) {
         // Dummy argument defined by explicit interface?
         details.set_isDummy(IsDummy(*symbol));
@@ -617,7 +619,8 @@ class ScopeHandler : public ImplicitRulesVisitor {
       symbol->set_details(std::move(details));
       return *symbol;
     } else if constexpr (std::is_same_v<UnknownDetails, D>) {
-      symbol->attrs() |= attrs;
+      CheckDuplicatedAttrs(name, *symbol, attrs);
+      SetExplicitAttrs(*symbol, attrs);
       return *symbol;
     } else {
       if (!CheckPossibleBadForwardRef(*symbol)) {
@@ -637,6 +640,23 @@ class ScopeHandler : public ImplicitRulesVisitor {
 
   void MakeExternal(Symbol &);
 
+  // C815 duplicated attribute checking; returns false on error
+  bool CheckDuplicatedAttr(SourceName, const Symbol &, Attr);
+  bool CheckDuplicatedAttrs(SourceName, const Symbol &, Attrs);
+
+  void SetExplicitAttr(Symbol &symbol, Attr attr) const {
+    symbol.attrs().set(attr);
+    symbol.implicitAttrs().reset(attr);
+  }
+  void SetExplicitAttrs(Symbol &symbol, Attrs attrs) const {
+    symbol.attrs() |= attrs;
+    symbol.implicitAttrs() &= ~attrs;
+  }
+  void SetImplicitAttr(Symbol &symbol, Attr attr) const {
+    symbol.attrs().set(attr);
+    symbol.implicitAttrs().set(attr);
+  }
+
 protected:
   FuncResultStack &funcResultStack() { return funcResultStack_; }
 
@@ -2284,7 +2304,8 @@ Symbol *ScopeHandler::FindSymbol(const Scope &scope, const parser::Name &name) {
 Symbol &ScopeHandler::MakeSymbol(
     Scope &scope, const SourceName &name, Attrs attrs) {
   if (Symbol * symbol{FindInScope(scope, name)}) {
-    symbol->attrs() |= attrs;
+    CheckDuplicatedAttrs(name, *symbol, attrs);
+    SetExplicitAttrs(*symbol, attrs);
     return *symbol;
   } else {
     const auto pair{scope.try_emplace(name, attrs, UnknownDetails{})};
@@ -2430,17 +2451,17 @@ bool ScopeHandler::ImplicitlyTypeForwardRef(Symbol &symbol) {
 // the INTRINSIC attribute.  Also set PURE &/or ELEMENTAL as
 // appropriate.
 void ScopeHandler::AcquireIntrinsicProcedureFlags(Symbol &symbol) {
-  symbol.attrs().set(Attr::INTRINSIC);
+  SetImplicitAttr(symbol, Attr::INTRINSIC);
   switch (context().intrinsics().GetIntrinsicClass(symbol.name().ToString())) {
   case evaluate::IntrinsicClass::elementalFunction:
   case evaluate::IntrinsicClass::elementalSubroutine:
-    symbol.attrs().set(Attr::ELEMENTAL);
-    symbol.attrs().set(Attr::PURE);
+    SetExplicitAttr(symbol, Attr::ELEMENTAL);
+    SetExplicitAttr(symbol, Attr::PURE);
     break;
   case evaluate::IntrinsicClass::impureSubroutine:
     break;
   default:
-    symbol.attrs().set(Attr::PURE);
+    SetExplicitAttr(symbol, Attr::PURE);
   }
 }
 
@@ -2586,7 +2607,7 @@ bool ScopeHandler::CheckPossibleBadForwardRef(const Symbol &symbol) {
 
 void ScopeHandler::MakeExternal(Symbol &symbol) {
   if (!symbol.attrs().test(Attr::EXTERNAL)) {
-    symbol.attrs().set(Attr::EXTERNAL);
+    SetImplicitAttr(symbol, Attr::EXTERNAL);
     if (symbol.attrs().test(Attr::INTRINSIC)) { // C840
       Say(symbol.name(),
           "Symbol '%s' cannot have both EXTERNAL and INTRINSIC attributes"_err_en_US,
@@ -2595,6 +2616,30 @@ void ScopeHandler::MakeExternal(Symbol &symbol) {
   }
 }
 
+bool ScopeHandler::CheckDuplicatedAttr(
+    SourceName name, const Symbol &symbol, Attr attr) {
+  if (attr == Attr::SAVE || attr == Attr::BIND_C) {
+    // these are checked elsewhere
+  } else if (symbol.attrs().test(attr)) { // C815
+    if (symbol.implicitAttrs().test(attr)) {
+      // Implied attribute is now confirmed explicitly
+    } else {
+      Say(name, "%s attribute was already specified on '%s'"_err_en_US,
+          EnumToString(attr), name);
+      return false;
+    }
+  }
+  return true;
+}
+
+bool ScopeHandler::CheckDuplicatedAttrs(
+    SourceName name, const Symbol &symbol, Attrs attrs) {
+  bool ok{true};
+  attrs.IterateOverMembers(
+      [&](Attr x) { ok &= CheckDuplicatedAttr(name, symbol, x); });
+  return ok;
+}
+
 // ModuleVisitor implementation
 
 bool ModuleVisitor::Pre(const parser::Only &x) {
@@ -2782,6 +2827,8 @@ void ModuleVisitor::DoAddUse(SourceName location, SourceName localName,
     localSymbol.set_details(UseDetails{localName, useSymbol});
     localSymbol.attrs() =
         useSymbol.attrs() & ~Attrs{Attr::PUBLIC, Attr::PRIVATE};
+    localSymbol.implicitAttrs() =
+        localSymbol.attrs() & Attrs{Attr::ASYNCHRONOUS, Attr::VOLATILE};
     localSymbol.flags() = useSymbol.flags();
     return;
   }
@@ -3051,7 +3098,7 @@ void ModuleVisitor::ApplyDefaultAccess() {
   for (auto &pair : currScope()) {
     Symbol &symbol = *pair.second;
     if (!symbol.attrs().HasAny({Attr::PUBLIC, Attr::PRIVATE})) {
-      symbol.attrs().set(defaultAccess_);
+      SetImplicitAttr(symbol, defaultAccess_);
     }
   }
 }
@@ -3095,7 +3142,7 @@ bool InterfaceVisitor::Pre(const parser::GenericStmt &) {
 }
 void InterfaceVisitor::Post(const parser::GenericStmt &x) {
   if (auto &accessSpec{std::get<std::optional<parser::AccessSpec>>(x.t)}) {
-    GetGenericInfo().symbol->attrs().set(AccessSpecToAttr(*accessSpec));
+    SetExplicitAttr(*GetGenericInfo().symbol, AccessSpecToAttr(*accessSpec));
   }
   const auto &names{std::get<std::list<parser::Name>>(x.t)};
   AddSpecificProcs(names, ProcedureKind::Procedure);
@@ -3447,9 +3494,10 @@ void SubprogramVisitor::Post(const parser::FunctionStmt &stmt) {
 SubprogramDetails &SubprogramVisitor::PostSubprogramStmt(
     const parser::Name &name) {
   Symbol &symbol{*currScope().symbol()};
-  symbol.attrs() |= EndAttrs();
+  SetExplicitAttrs(symbol, EndAttrs());
   if (symbol.attrs().test(Attr::MODULE)) {
     symbol.attrs().set(Attr::EXTERNAL, false);
+    symbol.implicitAttrs().set(Attr::EXTERNAL, false);
   }
   return symbol.get<SubprogramDetails>();
 }
@@ -3528,7 +3576,8 @@ void SubprogramVisitor::CreateEntry(
         if (auto *specific{generic->specific()}) {
           // Forward reference to ENTRY from a generic interface
           entrySymbol = specific;
-          entrySymbol->attrs() |= attrs;
+          CheckDuplicatedAttrs(entryName.source, *entrySymbol, attrs);
+          SetExplicitAttrs(*entrySymbol, attrs);
         }
       }
     } else {
@@ -3715,9 +3764,9 @@ bool SubprogramVisitor::BeginSubprogram(const parser::Name &name,
   if (moduleInterface) {
     newSymbol.get<SubprogramDetails>().set_moduleInterface(*moduleInterface);
     if (moduleInterface->attrs().test(Attr::PRIVATE)) {
-      newSymbol.attrs().set(Attr::PRIVATE);
+      SetImplicitAttr(newSymbol, Attr::PRIVATE);
     } else if (moduleInterface->attrs().test(Attr::PUBLIC)) {
-      newSymbol.attrs().set(Attr::PUBLIC);
+      SetImplicitAttr(newSymbol, Attr::PUBLIC);
     }
   }
   if (entryStmts) {
@@ -3835,7 +3884,7 @@ Symbol &SubprogramVisitor::PushSubprogramScope(const parser::Name &name,
     auto &details{symbol->get<SubprogramDetails>()};
     details.set_isInterface();
     if (isAbstract()) {
-      symbol->attrs().set(Attr::ABSTRACT);
+      SetExplicitAttr(*symbol, Attr::ABSTRACT);
     } else {
       MakeExternal(*symbol);
     }
@@ -4026,7 +4075,7 @@ bool DeclarationVisitor::Pre(const parser::BindEntity &x) {
     symbol = &HandleAttributeStmt(Attr::BIND_C, name);
   } else {
     symbol = &MakeCommonBlockSymbol(name);
-    symbol->attrs().set(Attr::BIND_C);
+    SetExplicitAttr(*symbol, Attr::BIND_C);
   }
   // 8.6.4(1)
   // Some entities such as named constant or module name need to checked
@@ -4274,8 +4323,10 @@ Symbol &DeclarationVisitor::HandleAttributeStmt(
   if (!symbol) {
     symbol = &MakeSymbol(name, EntityDetails{});
   }
-  symbol->attrs().set(attr);
-  symbol->attrs() = HandleSaveName(name.source, symbol->attrs());
+  if (CheckDuplicatedAttr(name.source, *symbol, attr)) {
+    SetExplicitAttr(*symbol, attr);
+    symbol->attrs() = HandleSaveName(name.source, symbol->attrs());
+  }
   return *symbol;
 }
 // C1107
@@ -4684,6 +4735,8 @@ void DeclarationVisitor::Post(const parser::DerivedTypeStmt &x) {
       auto &comp{DeclareEntity<ObjectEntityDetails>(*extendsName, Attrs{})};
       comp.attrs().set(
           Attr::PRIVATE, extendsSymbol.attrs().test(Attr::PRIVATE));
+      comp.implicitAttrs().set(
+          Attr::PRIVATE, extendsSymbol.implicitAttrs().test(Attr::PRIVATE));
       comp.set(Symbol::Flag::ParentComp);
       DeclTypeSpec &type{currScope().MakeDerivedType(
           DeclTypeSpec::TypeDerived, std::move(*extendsType))};
@@ -5072,7 +5125,7 @@ bool DeclarationVisitor::Pre(const parser::TypeBoundGenericStmt &x) {
       return false;
     }
     if (isPrivate) {
-      genericSymbol->attrs().set(Attr::PRIVATE);
+      SetExplicitAttr(*genericSymbol, Attr::PRIVATE);
     }
   }
   for (const parser::Name &bindingName : bindingNames) {
@@ -5482,7 +5535,7 @@ void DeclarationVisitor::AddSaveName(
 // Set the SAVE attribute on symbol unless it is implicitly saved anyway.
 void DeclarationVisitor::SetSaveAttr(Symbol &symbol) {
   if (!IsSaved(symbol)) {
-    symbol.attrs().set(Attr::SAVE);
+    SetImplicitAttr(symbol, Attr::SAVE);
   }
 }
 
@@ -5612,10 +5665,10 @@ bool DeclarationVisitor::HandleUnrestrictedSpecificIntrinsicFunction(
     symbol.set_details(std::move(details));
     symbol.set(Symbol::Flag::Function);
     if (interface->IsElemental()) {
-      symbol.attrs().set(Attr::ELEMENTAL);
+      SetExplicitAttr(symbol, Attr::ELEMENTAL);
     }
     if (interface->IsPure()) {
-      symbol.attrs().set(Attr::PURE);
+      SetExplicitAttr(symbol, Attr::PURE);
     }
     Resolve(name, symbol);
     return true;
@@ -6437,7 +6490,7 @@ void ConstructVisitor::SetAttrsFromAssociation(Symbol &symbol) {
   symbol.attrs() |= attrs &
       Attrs{Attr::TARGET, Attr::ASYNCHRONOUS, Attr::VOLATILE, Attr::CONTIGUOUS};
   if (attrs.test(Attr::POINTER)) {
-    symbol.attrs().set(Attr::TARGET);
+    SetImplicitAttr(symbol, Attr::TARGET);
   }
 }
 
@@ -7037,7 +7090,7 @@ void ResolveNamesVisitor::NoteExecutablePartCall(
         if (symbol->has<ProcEntityDetails>()) {
           symbol->set(flag);
           if (IsDummy(*symbol)) {
-            symbol->attrs().set(Attr::EXTERNAL);
+            SetImplicitAttr(*symbol, Attr::EXTERNAL);
           }
           ApplyImplicitRules(*symbol);
         }
@@ -7590,7 +7643,7 @@ void ResolveNamesVisitor::AddSubpNames(ProgramTree &node) {
   for (auto &child : node.children()) {
     auto &symbol{MakeSymbol(child.name(), SubprogramNameDetails{kind, child})};
     if (child.HasModulePrefix()) {
-      symbol.attrs().set(Attr::MODULE);
+      SetExplicitAttr(symbol, Attr::MODULE);
     }
     auto childKind{child.GetKind()};
     if (childKind == ProgramTree::Kind::Function) {
@@ -7606,7 +7659,7 @@ void ResolveNamesVisitor::AddSubpNames(ProgramTree &node) {
           MakeSymbol(std::get<parser::Name>(entryStmt->t), std::move(details))};
       symbol.set(child.GetSubpFlag());
       if (child.HasModulePrefix()) {
-        symbol.attrs().set(Attr::MODULE);
+        SetExplicitAttr(symbol, Attr::MODULE);
       }
     }
   }

diff  --git a/flang/test/Lower/call-site-mangling.f90 b/flang/test/Lower/call-site-mangling.f90
index 8cf0e4eab7c27..9c9205374a60f 100644
--- a/flang/test/Lower/call-site-mangling.f90
+++ b/flang/test/Lower/call-site-mangling.f90
@@ -112,7 +112,6 @@ subroutine some_bindc_iface() bind(C, name="some_name_some_foo_does_not_inherit"
     end subroutine
   end interface
  procedure(some_bindc_iface) :: foo5
- external :: foo5
  ! CHECK: fir.call @foo5
  call foo5()
 end

diff  --git a/flang/test/Semantics/misc-declarations.f90 b/flang/test/Semantics/misc-declarations.f90
index ec8eb7a7e924d..ca5f6f7ccd976 100644
--- a/flang/test/Semantics/misc-declarations.f90
+++ b/flang/test/Semantics/misc-declarations.f90
@@ -29,7 +29,7 @@ subroutine C867
     volatile :: coarrayComponent
   end subroutine
   subroutine C868(coarray,coarrayComponent)
-    real, volatile :: coarray[*]
+    real :: coarray[*]
     type(hasCoarray) :: coarrayComponent
     block
       !ERROR: VOLATILE attribute may not apply to a coarray accessed by USE or host association

diff  --git a/flang/test/Semantics/resolve20.f90 b/flang/test/Semantics/resolve20.f90
index 938decc499194..be6fe968fbe67 100644
--- a/flang/test/Semantics/resolve20.f90
+++ b/flang/test/Semantics/resolve20.f90
@@ -40,6 +40,10 @@ subroutine forward
   type :: m ! the name of a module can be used as a local identifier
   end type m
 
+  !ERROR: EXTERNAL attribute was already specified on 'a'
+  !ERROR: EXTERNAL attribute was already specified on 'b'
+  !ERROR: EXTERNAL attribute was already specified on 'c'
+  !ERROR: EXTERNAL attribute was already specified on 'd'
   external :: a, b, c, d
   !ERROR: EXTERNAL attribute not allowed on 'm'
   external :: m

diff  --git a/flang/test/Semantics/resolve91.f90 b/flang/test/Semantics/resolve91.f90
index f458823bc5435..9873c5a351a40 100644
--- a/flang/test/Semantics/resolve91.f90
+++ b/flang/test/Semantics/resolve91.f90
@@ -2,12 +2,15 @@
 ! Tests for duplicate definitions and initializations, mostly of procedures
 module m
   procedure(real), pointer :: p
+  !ERROR: EXTERNAL attribute was already specified on 'p'
+  !ERROR: POINTER attribute was already specified on 'p'
   !ERROR: The interface for procedure 'p' has already been declared
   procedure(integer), pointer :: p
 end
 
 module m1
     real, dimension(:), pointer :: realArray => null()
+    !ERROR: POINTER attribute was already specified on 'realarray'
     !ERROR: The type of 'realarray' has already been declared
     real, dimension(:), pointer :: realArray => localArray
 end module m1
@@ -19,6 +22,8 @@ end subroutine sub
   end interface
 
   procedure(sub), pointer :: p1 => null()
+  !ERROR: EXTERNAL attribute was already specified on 'p1'
+  !ERROR: POINTER attribute was already specified on 'p1'
   !ERROR: The interface for procedure 'p1' has already been declared
   procedure(sub), pointer :: p1 => null()
 
@@ -31,6 +36,8 @@ end function fun
   end interface
 
   procedure(fun), pointer :: f1 => null()
+  !ERROR: EXTERNAL attribute was already specified on 'f1'
+  !ERROR: POINTER attribute was already specified on 'f1'
   !ERROR: The interface for procedure 'f1' has already been declared
   procedure(fun), pointer :: f1 => null()
 
@@ -71,6 +78,7 @@ module m8
   integer, target :: jVar = 5
   integer, target :: kVar = 5
   integer, pointer :: pVar => jVar
+  !ERROR: POINTER attribute was already specified on 'pvar'
   !ERROR: The type of 'pvar' has already been declared
   integer, pointer :: pVar => kVar
 end module m8


        


More information about the flang-commits mailing list