Merge pull request #13253 from lydia-duncan/privateUse

Support public and private use statements
[reviewed by @benharsh]

Prior to this change, all use statements were public, with no option to
change that.  This meant that symbols needed for your module would
get bled into your users' namespaces.  For instance, in the following
code:
```chapel
module A {
  proc someFunc() { ... }
}
module B {
  use A;
  ...
}
module C {
  use B;
}
```
module C would have `someFunc` as part of its namespace.  This could
lead to problems like name conflicts, shadowing due to one function having
a more specific signature, etc.

We would frequently work around this by limiting the scope of the
use statement, e.g. putting the use inside of the function where the
symbols were needed - but this could lead to redundant use statements
in the case where multiple functions in a module needed the same
module.

This change adds the ability to declare a use statement as public or
private.  Public is still the default (though we may look into changing
that).  Declaring a use as private will mean that the symbols it brings
into scope are only accessible by the scope in which the use is declared.

The changes to support it are fairly minor.  We needed to add parser
support and a new field to UseStmts (impacting how we create them),
and then check that field when performing scope and function resolution.
The most effort was put into function resolution, where we needed to
check the visibility relative to the scope where the call occurred, and
relative to the instantiation point of the call if it was within a generic
function.

I also renamed a method on ModuleSymbol - the method would drop
use statement information about pretty much everything except the
module being used on the floor, but won't cause problems since it is
only used by dead code elimination.  In discussing with Michael, we
don't like that it does this, but don't think it is worth taking on the
effort to fix it until it causes problems.  I left a note at the offending
line for if we get back to it - hopefully renaming will ensure that no
one tries to use it when doing so would cause problems.

Resolves #6093 

This change adds 30 tests of the new feature.  They check:
- except lists on the private use to ensure the avoided symbols are not
accidentally exposed, both in the module with the private use and in
any clients of that module
- private use statements containing multiple modules, to ensure that
all modules used are used privately
- only lists on the private use to ensure the symbols that aren't named are
not accidentally exposed, both in the module with the private use and
in any clients of that module
- renaming symbols via a private use, to ensure that the rename works in the
module with the use, and that the new name is not visible to clients of that
module
- privately using a module that defines a secondary method, to ensure the
method is not exposed beyond the module with the private use.
- privately and publicly using the same module so that some symbols are available
to clients and the rest only to the module with the use statement
- privately using a module that defines a type, to ensure the type is not exposed
beyond the module with the private use
- that privately using a module with a public use doesn't behave badly
- and various point of instantiation tests that are difficult to describe succinctly
(ran them by Michael to be sure I wasn't crazy, though most of them resemble
tests that already exist and are not a serious departure from them)

Passed a full paratest with futures.

Later todos:
- modify error message so that it isn't confusing when a symbol is not found
because of privacy
- private uses by default?
- private use of:
  - root module
  - ChapelStandard
  - Fortran module

compiler/AST/ModuleSymbol.cpp

@@ -635,19 +635,19 @@ void ModuleSymbol::addDefaultUses() {
       // If this is a Fortran compilation, we need to use ISO_Fortran_binding
       if (fLibraryFortran) {
         modRef = new UnresolvedSymExpr("ISO_Fortran_binding");
-        block->insertAtHead(new UseStmt(modRef));
+        block->insertAtHead(new UseStmt(modRef, false));
       }
 
       modRef = new UnresolvedSymExpr("ChapelStandard");
-      block->insertAtHead(new UseStmt(modRef));
+      block->insertAtHead(new UseStmt(modRef, false));
     }
 
   // We don't currently have a good way to fetch the root module by name.
   // Insert it directly rather than by name
   } else if (this == baseModule) {
     SET_LINENO(this);
 
-    block->useListAdd(rootModule);
+    block->useListAdd(rootModule, false);
   }
 }
 
@@ -681,7 +681,7 @@ void ModuleSymbol::moduleUseAdd(ModuleSymbol* mod) {
 //
 // At this time this is only used for deadCodeElimination and
 // it is not clear if there will be other uses.
-void ModuleSymbol::moduleUseRemove(ModuleSymbol* mod) {
+void ModuleSymbol::deadCodeModuleUseRemove(ModuleSymbol* mod) {
   int index = modUseList.index(mod);
 
   if (index >= 0) {
@@ -699,7 +699,9 @@ void ModuleSymbol::moduleUseRemove(ModuleSymbol* mod) {
         SET_LINENO(this);
 
         if (inBlock == true) {
-          block->useListAdd(modUsedByDeadMod);
+          // Note: this drops only and except lists, renamings, and private uses
+          // on the floor.
+          block->useListAdd(modUsedByDeadMod, false);
         }
 
         modUseList.add(modUsedByDeadMod);
@@ -722,7 +724,7 @@ void initStringLiteralModule() {
                                                    MOD_INTERNAL,
                                                    new BlockStmt());
 
-  stringLiteralModule->block->useListAdd(new UseStmt(new UnresolvedSymExpr("ChapelStandard")));
+  stringLiteralModule->block->useListAdd(new UseStmt(new UnresolvedSymExpr("ChapelStandard"), false));
 
   stringLiteralModule->filename = astr("<internal>");
 

compiler/AST/UseStmt.cpp

@@ -27,7 +27,8 @@
 
 #include <algorithm>
 
-UseStmt::UseStmt(BaseAST* source) : Stmt(E_UseStmt) {
+UseStmt::UseStmt(BaseAST* source, bool isPrivate) : Stmt(E_UseStmt) {
+  this->isPrivate = isPrivate;
   src    = NULL;
   except = false;
 
@@ -47,9 +48,11 @@ UseStmt::UseStmt(BaseAST* source) : Stmt(E_UseStmt) {
 UseStmt::UseStmt(BaseAST*                            source,
                  std::vector<const char*>*           args,
                  bool                                exclude,
-                 std::map<const char*, const char*>* renames) :
+                 std::map<const char*, const char*>* renames,
+                 bool isPrivate) :
   Stmt(E_UseStmt) {
 
+  this->isPrivate = isPrivate;
   src    = NULL;
   except = exclude;
 
@@ -88,9 +91,9 @@ UseStmt* UseStmt::copyInner(SymbolMap* map) {
   UseStmt *_this = 0;
 
   if (named.size() > 0) { // MPF: should this have || renamed.size() > 0?
-    _this = new UseStmt(COPY_INT(src), &named, except, &renamed);
+    _this = new UseStmt(COPY_INT(src), &named, except, &renamed, isPrivate);
   } else {
-    _this = new UseStmt(COPY_INT(src));
+    _this = new UseStmt(COPY_INT(src), isPrivate);
   }
 
   for_vector(const char, sym, methodsAndFields) {
@@ -672,7 +675,7 @@ UseStmt* UseStmt::applyOuterUse(const UseStmt* outer) {
         // The only list will be shorter, create a new UseStmt with it.
         SET_LINENO(this);
 
-        return new UseStmt(src, &newOnlyList, false, &newRenamed);
+        return new UseStmt(src, &newOnlyList, false, &newRenamed, isPrivate);
       }
 
     } else {
@@ -729,7 +732,7 @@ UseStmt* UseStmt::applyOuterUse(const UseStmt* outer) {
           // outer 'only' list)
           SET_LINENO(this);
 
-          return new UseStmt(src, &newOnlyList, false, &newRenamed);
+          return new UseStmt(src, &newOnlyList, false, &newRenamed, isPrivate);
 
         } else {
           // all the 'only' identifiers were in the 'except'
@@ -785,7 +788,7 @@ UseStmt* UseStmt::applyOuterUse(const UseStmt* outer) {
           // There were symbols that were in both 'only' lists, so
           // this module use is still interesting.
           SET_LINENO(this);
-          return new UseStmt(src, &newOnlyList, false, &newRenamed);
+          return new UseStmt(src, &newOnlyList, false, &newRenamed, isPrivate);
 
         } else {
           // all of the 'only' identifiers in the outer use

compiler/AST/build.cpp

@@ -371,8 +371,9 @@ static void addModuleToSearchList(UseStmt* newUse, BaseAST* module) {
 }
 
 
-static BlockStmt* buildUseList(BaseAST* module, BlockStmt* list) {
-  UseStmt* newUse = new UseStmt(module);
+static BlockStmt* buildUseList(BaseAST* module, BlockStmt* list,
+                               bool privateUse) {
+  UseStmt* newUse = new UseStmt(module, privateUse);
   addModuleToSearchList(newUse, module);
   if (list == NULL) {
     return buildChapelStmt(newUse);
@@ -430,7 +431,8 @@ static void useListError(Expr* expr, bool except) {
 //
 // Build a 'use' statement with an 'except'/'only' list
 //
-BlockStmt* buildUseStmt(Expr* mod, std::vector<OnlyRename*>* names, bool except) {
+BlockStmt* buildUseStmt(Expr* mod, std::vector<OnlyRename*>* names, bool except,
+                        bool privateUse) {
   std::vector<const char*> namesList;
   std::map<const char*, const char*> renameMap;
 
@@ -479,7 +481,8 @@ BlockStmt* buildUseStmt(Expr* mod, std::vector<OnlyRename*>* names, bool except)
 
   }
 
-  UseStmt* newUse = new UseStmt(mod, &namesList, except, &renameMap);
+  UseStmt* newUse = new UseStmt(mod, &namesList, except, &renameMap,
+                                privateUse);
   addModuleToSearchList(newUse, mod);
 
   delete names;
@@ -490,15 +493,15 @@ BlockStmt* buildUseStmt(Expr* mod, std::vector<OnlyRename*>* names, bool except)
 //
 // Build a 'use' statement
 //
-BlockStmt* buildUseStmt(CallExpr* args) {
+BlockStmt* buildUseStmt(CallExpr* args, bool privateUse) {
   BlockStmt* list = NULL;
 
   //
   // Iterate over the expressions being 'use'd, processing them
   //
   for_actuals(expr, args) {
     Expr* useArg = expr->remove();
-    list = buildUseList(useArg, list);
+    list = buildUseList(useArg, list, privateUse);
   }
 
   //

compiler/AST/stmt.cpp

@@ -451,8 +451,8 @@ BlockStmt::length() const {
 
 
 void
-BlockStmt::useListAdd(ModuleSymbol* mod) {
-  useListAdd(new UseStmt(mod));
+BlockStmt::useListAdd(ModuleSymbol* mod, bool privateUse) {
+  useListAdd(new UseStmt(mod, privateUse));
 }
 
 void

compiler/include/ModuleSymbol.h

@@ -75,7 +75,7 @@ class ModuleSymbol : public Symbol {
   void                    addDefaultUses();
 
   void                    moduleUseAdd(ModuleSymbol* module);
-  void                    moduleUseRemove(ModuleSymbol* module);
+  void                    deadCodeModuleUseRemove(ModuleSymbol* module);
 
   void                    printDocs(std::ostream* file,
                                     unsigned int  tabs,

compiler/include/UseStmt.h

@@ -26,12 +26,13 @@ class ResolveScope;
 
 class UseStmt : public Stmt {
 public:
-                  UseStmt(BaseAST* source);
+  UseStmt(BaseAST* source, bool isPrivate);
 
                   UseStmt(BaseAST*                            source,
                           std::vector<const char*>*           args,
                           bool                                exclude,
-                          std::map<const char*, const char*>* renames);
+                          std::map<const char*, const char*>* renames,
+                          bool isPrivate);
 
   DECLARE_COPY(UseStmt);
 
@@ -63,6 +64,8 @@ class UseStmt : public Stmt {
 
   bool            providesNewSymbols(const UseStmt* other)               const;
 
+  bool            isVisible(BaseAST* scope)                              const;
+
   BaseAST*        getSearchScope()                                       const;
 
   void            writeListPredicate(FILE* mFP)                          const;
@@ -92,6 +95,7 @@ class UseStmt : public Stmt {
   Expr*                              src;
   std::vector<const char*>           named;
   std::map<const char*, const char*> renamed;
+  bool isPrivate;
 
 private:
   bool                               except;

compiler/include/bison-chapel.h

@@ -1,4 +1,4 @@
-/* A Bison parser, made by GNU Bison 3.1.  */
+/* A Bison parser, made by GNU Bison 3.0.5.  */
 
 /* Bison interface for Yacc-like parsers in C
 
@@ -40,12 +40,12 @@
 extern int yydebug;
 #endif
 /* "%code requires" blocks.  */
-#line 32 "chapel.ypp" /* yacc.c:1919  */
+#line 32 "chapel.ypp" /* yacc.c:1916  */
 
   #include <string>
   extern int         captureTokens;
   extern std::string captureString;
-#line 45 "chapel.ypp" /* yacc.c:1919  */
+#line 45 "chapel.ypp" /* yacc.c:1916  */
 
   #ifndef _BISON_CHAPEL_DEFINES_0_
   #define _BISON_CHAPEL_DEFINES_0_
@@ -60,7 +60,7 @@ extern int yydebug;
   void stringBufferInit();
 
   #endif
-#line 65 "chapel.ypp" /* yacc.c:1919  */
+#line 65 "chapel.ypp" /* yacc.c:1916  */
 
   #ifndef _BISON_CHAPEL_DEFINES_1_
   #define _BISON_CHAPEL_DEFINES_1_
@@ -135,7 +135,7 @@ extern int yydebug;
   };
 
   #endif
-#line 145 "chapel.ypp" /* yacc.c:1919  */
+#line 145 "chapel.ypp" /* yacc.c:1916  */
 
   #ifndef _BISON_CHAPEL_DEFINES_2_
   #define _BISON_CHAPEL_DEFINES_2_
@@ -153,7 +153,7 @@ extern int yydebug;
   #define YYLTYPE_IS_TRIVIAL  1
 
   #endif
-#line 168 "chapel.ypp" /* yacc.c:1919  */
+#line 168 "chapel.ypp" /* yacc.c:1916  */
 
   #ifndef _BISON_CHAPEL_DEFINES_3_
   #define _BISON_CHAPEL_DEFINES_3_
@@ -181,7 +181,7 @@ extern int yydebug;
 
   #endif
 
-#line 185 "../include/bison-chapel.h" /* yacc.c:1919  */
+#line 185 "../include/bison-chapel.h" /* yacc.c:1916  */
 
 /* Token type.  */
 #ifndef YYTOKENTYPE
@@ -386,14 +386,14 @@ int yypush_parse (yypstate *ps, int pushed_char, YYSTYPE const *pushed_val, YYLT
 yypstate * yypstate_new (void);
 void yypstate_delete (yypstate *ps);
 /* "%code provides" blocks.  */
-#line 199 "chapel.ypp" /* yacc.c:1919  */
+#line 199 "chapel.ypp" /* yacc.c:1916  */
 
   extern int yydebug;
 
   void yyerror(YYLTYPE*       ignored,
                ParserContext* context,
                const char*    str);
 
-#line 398 "../include/bison-chapel.h" /* yacc.c:1919  */
+#line 398 "../include/bison-chapel.h" /* yacc.c:1916  */
 
 #endif /* !YY_YY_INCLUDE_BISON_CHAPEL_H_INCLUDED  */

compiler/include/build.h

@@ -61,8 +61,9 @@ Expr* buildDotExpr(const char* base, const char* member);
 BlockStmt* buildChapelStmt(Expr* expr = NULL);
 BlockStmt* buildErrorStandin();
 
-BlockStmt* buildUseStmt(CallExpr* modules);
-BlockStmt* buildUseStmt(Expr* mod, std::vector<OnlyRename*>* names, bool except);
+BlockStmt* buildUseStmt(CallExpr* modules, bool privateUse);
+BlockStmt* buildUseStmt(Expr* mod, std::vector<OnlyRename*>* names, bool except,
+                        bool privateUse);
 bool processStringInRequireStmt(const char* str, bool parseTime,
                                 const char* modFilename);
 BlockStmt* buildRequireStmt(CallExpr* args);

compiler/include/stmt.h

@@ -129,7 +129,7 @@ class BlockStmt : public Stmt {
 
   int                 length()                                     const;
 
-  void                useListAdd(ModuleSymbol* mod);
+  void                useListAdd(ModuleSymbol* mod, bool isPrivate);
   void                useListAdd(UseStmt*      use);
   bool                useListRemove(ModuleSymbol* mod);
   void                useListClear();

compiler/optimizations/deadCodeElimination.cpp

@@ -438,7 +438,7 @@ static void deadModuleElimination() {
       // Inform every module about the dead module
       forv_Vec(ModuleSymbol, modThatMightUse, allModules) {
         if (modThatMightUse != mod) {
-          modThatMightUse->moduleUseRemove(mod);
+          modThatMightUse->deadCodeModuleUseRemove(mod);
         }
       }
     }