Add docstring and attribute support to interfaces (#25825)

Adds compiler support for interfaces to have attributes applied to them and have docstrings recognized by chpldoc Resolves #17383 Relies on chapel-lang/sphinxcontrib-chapeldomain#95 This PR also updates the interface `Allocators.allocator`. #25821 will update the existing Sort module allocators. No other allocators are currently shown in our documentation While adding tests of `@unstable` with interfaces, I found that some tests weren't actually testing what we thought they were (i.e. `test/unstable-keyword/enum_test1.chpl`), so I updated them Testing - [x] full paratest with/without comm [Reviewed by @lydia-duncan and @DanilaFe]
chapel-lang · Aug 28, 2024 · 42f8bb0 · 42f8bb0
2 parents b636f2b + 1956634
commit 42f8bb0
Show file tree

Hide file tree

Showing 18 changed files with 5,487 additions and 5,221 deletions.
diff --git a/frontend/lib/parsing/ParserContext.h b/frontend/lib/parsing/ParserContext.h
@@ -742,8 +742,8 @@ struct ParserContext {
                                 PODUniqueString name);
 
   CommentsAndStmt buildInterfaceStmt(YYLTYPE location,
-                                     YYLTYPE identLocation,
-                                     PODUniqueString name,
+                                     YYLTYPE headerLoc,
+                                     TypeDeclParts parts,
                                      ParserExprList* formals,
                                      YYLTYPE locBody,
                                      CommentsAndStmt body);

diff --git a/frontend/lib/parsing/ParserContextImpl.h b/frontend/lib/parsing/ParserContextImpl.h
@@ -3263,16 +3263,21 @@ AstNode* ParserContext::buildInterfaceFormal(YYLTYPE location,
 }
 
 CommentsAndStmt ParserContext::buildInterfaceStmt(YYLTYPE location,
-                                                  YYLTYPE identLocation,
-                                                  PODUniqueString name,
+                                                  YYLTYPE headerLoc,
+                                                  TypeDeclParts parts,
                                                   ParserExprList* formals,
                                                   YYLTYPE locBody,
                                                   CommentsAndStmt body) {
-  std::vector<ParserComment>* comments;
+  // interfaces do not have visibility, linkage, or linkage names
+  CHPL_ASSERT(parts.visibility == Decl::DEFAULT_VISIBILITY);
+  CHPL_ASSERT(parts.linkage == Decl::DEFAULT_LINKAGE);
+  CHPL_ASSERT(parts.linkageName == nullptr);
+
+  std::vector<ParserComment>* bodyComments;
   ParserExprList* bodyExprLst;
   BlockStyle blockStyle;
 
-  prepareStmtPieces(comments, bodyExprLst, blockStyle, location,
+  prepareStmtPieces(bodyComments, bodyExprLst, blockStyle, location,
                     false,
                     locBody,
                     body);
@@ -3291,16 +3296,17 @@ CommentsAndStmt ParserContext::buildInterfaceStmt(YYLTYPE location,
   const bool isFormalListPresent = formals != nullptr;
 
   auto node = Interface::build(builder, convertLocation(location),
-                               buildAttributeGroup(location),
+                               toOwned(parts.attributeGroup),
                                visibility,
-                               name,
+                               parts.name,
                                isFormalListPresent,
                                std::move(formalList),
                                std::move(bodyStmts));
-  builder->noteDeclNameLocation(node.get(), convertLocation(identLocation));
+  builder->noteDeclNameLocation(node.get(), convertLocation(parts.locName));
+  builder->noteDeclHeaderLocation(node.get(), convertLocation(headerLoc));
 
 
-  CommentsAndStmt cs = { .comments=comments, .stmt=node.release() };
+  CommentsAndStmt cs = { .comments=parts.comments, .stmt=node.release() };
 
   return cs;
 }

diff --git a/frontend/lib/parsing/bison-chpl-lib.cpp b/frontend/lib/parsing/bison-chpl-lib.cpp
diff --git a/frontend/lib/parsing/chpl.ypp b/frontend/lib/parsing/chpl.ypp
@@ -551,6 +551,7 @@
 %type <exprList> import_ls
 %type <exprList> renames_ls use_renames_ls opt_only_ls
 
+%type <typeDeclParts> interface_start
 // These are exprList so that they can include preceeding comments
 %type <commentsAndStmt> implements_stmt interface_stmt
 %type <exprList>  ifc_formal_ls
@@ -715,7 +716,6 @@ stmt_base:
 | require_stmt              { $$ = context->finishStmt($1); }
 | extern_block_stmt         { $$ = context->finishStmt($1); }
 | implements_stmt           { $$ = context->finishStmt($1); }
-| interface_stmt            { $$ = context->finishStmt($1); }
 | TDEFER stmt
   {
     std::vector<ParserComment>* comments;
@@ -924,6 +924,7 @@ tryable_stmt:
 decl_base:
   module_decl_stmt
 | class_level_stmt
+| interface_stmt
 ;
 
 collect_attributes:
@@ -1898,14 +1899,25 @@ ifvar:
   }
 ;
 
+interface_start:
+  TINTERFACE ident_def
+  {
+    $$ = context->enterScopeAndBuildTypeDeclParts(@1, @2, $2, asttags::Interface);
+  }
+;
+
 interface_stmt:
-  TINTERFACE ident_def TLP ifc_formal_ls TRP block_stmt
+  interface_start TLP ifc_formal_ls TRP block_stmt
   {
-    $$ = context->buildInterfaceStmt(@$, @2, $2, $4, @6, $6);
+    TypeDeclParts parts = $1;
+    $$ = context->buildInterfaceStmt(@$, @1, parts, $3, @5, $5);
+    context->exitScope(asttags::Interface, parts.name);
   }
-| TINTERFACE ident_def                       block_stmt
+| interface_start                       block_stmt
   {
-    $$ = context->buildInterfaceStmt(@$, @2, $2, nullptr, @3, $3);
+    TypeDeclParts parts = $1;
+    $$ = context->buildInterfaceStmt(@$, @1, parts, nullptr, @2, $2);
+    context->exitScope(asttags::Interface, parts.name);
   }
 ;
 

diff --git a/frontend/test/parsing/testParseAttributes.cpp b/frontend/test/parsing/testParseAttributes.cpp
@@ -1116,6 +1116,58 @@ static void test22(Parser* parser) {
   assert(guard.realizeErrors()==1);
 }
 
+// check interfaces
+static void test23(Parser* parser) {
+  auto ctx = parser->context();
+  ErrorGuard guard(ctx);
+  std::string program = R""""(
+    /*I am the docstring for the interface*/
+    @unstable("I am unstable")
+    interface foo {
+      /*I am the docstring for the method*/
+      @deprecated("I am deprecated")
+      proc Self.bar();
+    }
+  )"""";
+  auto parseResult = parseStringAndReportErrors(parser, "test23.chpl",
+                                                program.c_str());
+  assert(!guard.realizeErrors());
+  auto mod = parseResult.singleModule();
+  assert(mod);
+  assert(mod->numStmts() == 2);
+
+  auto comment = mod->stmt(0)->toComment();
+  assert(comment);
+  assert(comment->str() == "/*I am the docstring for the interface*/");
+
+  auto intf = mod->stmt(1)->toInterface();
+  assert(intf);
+
+  auto intfAttr = intf->attributeGroup();
+  assert(intfAttr);
+  assert(intfAttr->isUnstable());
+  assert(!intf->isFormalListExplicit());
+  assert(intf->numFormals() == 1);
+  assert(intf->numStmts() == 2);
+
+  auto formal = intf->formal(0)->toFormal();
+  assert(formal);
+  assert(formal->name() == "Self");
+
+  auto methodComment = intf->stmt(0)->toComment();
+  assert(methodComment);
+  assert(methodComment->str() == "/*I am the docstring for the method*/");
+
+  auto intfMethod = intf->stmt(1)->toFunction();
+  assert(intfMethod);
+  assert(intfMethod->name() == "bar");
+  auto intfMethodAttr = intfMethod->attributeGroup();
+  assert(intfMethodAttr);
+  assert(intfMethodAttr->isDeprecated());
+
+
+}
+
 int main() {
   Context context;
   Context* ctx = &context;
@@ -1149,6 +1201,7 @@ int main() {
   test20(p);
   test21(p);
   test22(p);
+  test23(p);
 
   return 0;
 }
diff --git a/modules/standard/Allocators.chpl b/modules/standard/Allocators.chpl
@@ -40,15 +40,18 @@
      deleteWithAllocator(alloc, x);
 
   Custom allocators can be defined as classes or records that implement the
-  ``allocator`` interface. The interface has two methods: ``allocate`` and
-  ``deallocate``. The ``allocate`` method accepts an integer ``n`` and returns
-  a :type:`~CTypes.c_ptr` to the allocated memory. The ``deallocate``
-  method accepts a :type:`~CTypes.c_ptr` to the allocated memory. Allocators are
-  free to implement their own memory management strategies.
+  :interface:`allocator` interface. The interface has two methods:
+  :proc:`~allocator.Self.allocate` and :proc:`~allocator.Self.deallocate`. The
+  :proc:`~allocator.Self.allocate` method accepts an integer ``n`` and returns
+  a :type:`~CTypes.c_ptr` to the allocated memory. The
+  :proc:`~allocator.Self.deallocate` method accepts a :type:`~CTypes.c_ptr` to
+  the allocated memory. Allocators are free to implement their own memory
+  management strategies.
 
   Limitations:
-    * The ``newWithAllocator`` and ``deleteWithAllocator`` procedures are meant
-      to be stand-ins pending a more elegant syntax to use custom allocators.
+    * The :proc:`newWithAllocator` and :proc:`deleteWithAllocator` procedures
+      are meant to be stand-ins pending a more elegant syntax to use custom
+      allocators.
     * This module currently only supports allocating Chapel classes. In the
       future we hope to support other heap objects like arrays.
     * Allocating managed (:type:`~OwnedObject.owned`/:type:`~SharedObject.shared`)
@@ -71,12 +74,13 @@ module Allocators {
     return alignedPtr:c_ptr(void);
   }
 
-  // TODO: interfaces are not supported by chpldoc yet: https://github.com/chapel-lang/chapel/issues/17383
   /*
     All allocators must implement this interface. The interface has two
-    methods: ``allocate`` and ``deallocate``. The ``allocate`` method accepts
-    an integer size and returns a pointer to the allocated memory. The
-    ``deallocate`` method accepts a pointer to the allocated memory.
+    methods: :proc:`~allocator.Self.allocate` and
+    :proc:`~allocator.Self.deallocate`. The :proc:`~allocator.Self.allocate`
+    method accepts an integer size and returns a pointer to the allocated
+    memory. The :proc:`~allocator.Self.deallocate` method accepts a pointer
+    to the allocated memory.
 
     Allocators may either be classes or records.
   */
@@ -109,8 +113,9 @@ module Allocators {
   }
 
   /*
-    Allocate a new unmanaged class with type ``T`` by invoking the ``allocate``
-    method of the given ``alloc``. This is a drop-in replacement for ``new``.
+    Allocate a new unmanaged class with type ``T`` by invoking the
+    :proc:`~allocator.Self.allocate` method of the given ``alloc``. This is a
+    drop-in replacement for ``new``.
 
     Example:
 
@@ -161,8 +166,9 @@ module Allocators {
   }
 
   /*
-    Delete the ``objects`` by invoking the ``deallocate`` method of the given
-    ``allocator``. This is a drop-in replacement for ``delete``.
+    Delete the ``objects`` by invoking the :proc:`~allocator.Self.deallocate`
+    method of the given :interface:`allocator`. This is a drop-in replacement
+    for ``delete``.
   */
   pragma "docs only"
   inline proc deleteWithAllocator(alloc: allocator, objects...) {

diff --git a/test/chpldoc/interface.doc.catfiles b/test/chpldoc/interface.doc.catfiles
@@ -0,0 +1 @@
+docs/source/modules/interface.doc.rst
diff --git a/test/chpldoc/interface.doc.chpl b/test/chpldoc/interface.doc.chpl
@@ -0,0 +1,27 @@
+/*
+  An interface without formals
+*/
+interface myInterface {
+  /* The foo method */
+  proc Self.foo();
+  @deprecated("bar is deprecated")
+  proc Self.bar();
+}
+
+/*
+  An interface with formals
+*/
+@unstable("I am unstable")
+interface myInterfaceWithFormals(T1, T2) {
+  proc Self.foo(x: T1);
+  /* The bar method */
+  proc Self.bar(y: T2);
+  @chpldoc.nodoc
+  proc Self.hidden();
+}
+
+@chpldoc.nodoc
+interface hiddenInterface {
+  proc Self.hidden();
+}
+
diff --git a/test/chpldoc/interface.doc.good b/test/chpldoc/interface.doc.good
@@ -0,0 +1,49 @@
+.. default-domain:: chpl
+
+.. module:: interface.doc
+
+interface.doc
+=============
+**Usage**
+
+.. code-block:: chapel
+
+   use interface.doc;
+
+
+or
+
+.. code-block:: chapel
+
+   import interface.doc;
+
+.. interface:: interface myInterface
+
+
+   An interface without formals
+
+   .. method:: proc Self.foo()
+
+   The foo method 
+
+   .. method:: proc Self.bar()
+
+   .. warning::
+
+      bar is deprecated
+
+.. interface:: interface myInterfaceWithFormals(type T1, type T2)
+
+   .. warning::
+
+      I am unstable
+
+
+   An interface with formals
+
+   .. method:: proc Self.foo(x: T1)
+
+   .. method:: proc Self.bar(y: T2)
+
+   The bar method 
+
diff --git a/test/deprecated-keyword/handleInterface.chpl b/test/deprecated-keyword/handleInterface.chpl
@@ -0,0 +1,30 @@
+
+@deprecated("I am deprecated")
+interface myInterface {
+  proc Self.foo();
+  proc Self.bar();
+}
+
+record R1: myInterface { // should warn
+  proc foo() {
+    writeln("foo");
+  }
+  proc bar() {
+    writeln("bar");
+  }
+}
+var r1 = new R1();
+r1.foo();
+r1.bar();
+
+record R2 {}
+R2 implements myInterface; // should warn
+proc R2.foo() {
+  writeln("foo");
+}
+proc R2.bar() {
+  writeln("bar");
+}
+var r2 = new R2();
+r2.foo();
+r2.bar();
diff --git a/test/deprecated-keyword/handleInterface.good b/test/deprecated-keyword/handleInterface.good
@@ -0,0 +1,6 @@
+handleInterface.chpl:21: warning: I am deprecated
+handleInterface.chpl:8: warning: I am deprecated
+foo
+bar
+foo
+bar
diff --git a/...table-keyword/catchUnstableError.compopts → test/unstable-keyword/COMPOPTS b/...table-keyword/catchUnstableError.compopts → test/unstable-keyword/COMPOPTS
diff --git a/test/unstable-keyword/enum_test1.good b/test/unstable-keyword/enum_test1.good
@@ -1,3 +1,7 @@
+enum_test1.chpl:4: In function 'main':
+enum_test1.chpl:5: warning: a is unstable
+enum_test1.chpl:6: warning: a is unstable
+enum_test1.chpl:11: warning: a is unstable
 a
 a
 a