Skip to content

Commit

Permalink
Improved robustness of internal modules including standard ones
Browse files Browse the repository at this point in the history
This checkin fixes the two userInsteadOfStandard futures as well as
the case-insensitive filename issue that cropped up on the Mac after
my commits yesterday.  In particular, it permits a user module to take
the same name as a standard module without affecting the ability of an
internal module to use that standard module to implement itself.  It
does this without duplicating the storage for the standard module in
the event that both the user code and the internal modules require it.

It does this by changing the build code for the module use statements
to keep a list of the PRIM_USE expressions for all internal modules
using a vector modReqdByInt ("module required by internal module").
After parsing the ChapelBase and ChapelStandard modules, the compiler
attempts to resolve all of their modules using the internal module
path, as before.  As a result, any standard modules required by the
internal modules will not be parsed at this time.  We then clear the
list of all modules that need to be resolved since we have the full
list of modules required by the internal modules in the new
modReqdByInt vector.  We then parse the Chapel files listed on the
command line and then parse any files they depend on as before, which
may include some of the standard modules required by the internal
modules and/or may involve defining some modules with the same name as
those modules.

Next, we iterate over all the modules required by the internal modules
and see whether we've read in such a module as a standard and/or user
module.  If we haven't read in the standard module, we do so now.  If
we did not read a user module with the same name we're done.  If we
have, we rename the standard module to a unique filename (currently
chpl_ followed by the module's original name) and update the use
statements in the internal module to refer to this new name.  This is
reasonable and legal because as far as the user's code is concerned,
this module was never read in by the program.  

Note that the dual approach could be taken (read in the standard
modules for the internal modules first and in the event that the user
modules end up using modules by the same name, rename the user's use
statements and modules.  I took the approach here under the assumption
that (a) in large programs there would be more user modules than
standard modules required by internal modules, and (b) Chapel users
would have an easier time reasoning about their code if their names
were preserved rather than munged.

Removed the .future files for the tests that now work, updated the
printModStuff test to reflect the current parse order (though in
retrospect, perhaps I shouldn't have had the pass print out the
standard modules required by the internal modules with --no-devel
since they aren't part of the user's view of the world).

This change also makes it so that if the --print-module-files flag is
on and we're in developer mode, the internal modules are printed out
in addition to the user and standard ones.



git-svn-id: http://svn.code.sf.net/p/chapel/code/trunk@15935 3a8e244f-b0f2-452b-bcba-4c88e055c3ca
  • Loading branch information
bradcray committed Sep 5, 2009
1 parent a36d0c9 commit 557eba1
Show file tree
Hide file tree
Showing 9 changed files with 78 additions and 26 deletions.
10 changes: 5 additions & 5 deletions compiler/AST/build.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,19 +134,19 @@ BlockStmt* buildChapelStmt(BaseAST* ast) {
}


static void addModuleToSearchList(BaseAST* module) {
static void addModuleToSearchList(CallExpr* newUse, BaseAST* module) {
UnresolvedSymExpr* modNameExpr = toUnresolvedSymExpr(module);
if (modNameExpr) {
addModuleToParseList(modNameExpr->unresolved);
addModuleToParseList(modNameExpr->unresolved, newUse);
} else if (CallExpr* callExpr = toCallExpr(module)) {
addModuleToSearchList(callExpr->argList.first());
addModuleToSearchList(newUse, callExpr->argList.first());
}
}


BlockStmt* buildUseList(BaseAST* module, BlockStmt* list) {
CallExpr* newUse = new CallExpr(PRIM_USE, module);
addModuleToSearchList(module);
addModuleToSearchList(newUse, module);
if (list == NULL) {
return buildChapelStmt(newUse);
} else {
Expand Down Expand Up @@ -299,7 +299,7 @@ void createInitFn(ModuleSymbol* mod) {


ModuleSymbol* buildModule(const char* name, BlockStmt* block, const char* filename) {
ModuleSymbol* mod = new ModuleSymbol(name, moduleType, block);
ModuleSymbol* mod = new ModuleSymbol(name, currentModuleType, block);
mod->filename = astr(filename);
createInitFn(mod);
return mod;
Expand Down
2 changes: 2 additions & 0 deletions compiler/include/files.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ void addModulePathFromFilename(const char* filename);

const char* modNameToFilename(const char* modName, bool isInternal,
bool* isStandard);
const char* stdModNameToFilename(const char* modName);

void printModuleSearchPath(void);

#endif
4 changes: 2 additions & 2 deletions compiler/include/parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@

#include "symbol.h"

extern ModTag moduleType;
extern ModTag currentModuleType;

ModuleSymbol* ParseFile(const char* filename, ModTag modtype);
ModuleSymbol* ParseMod(const char* modname, ModTag modtype);

void addModuleToParseList(const char* name);
void addModuleToParseList(const char* name, CallExpr* newUse);
void parseDependentModules(ModTag modtype);

#endif
58 changes: 54 additions & 4 deletions compiler/parser/parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,24 @@ BlockStmt* yyblock = NULL;
const char* yyfilename;
int chplLineno;
int yystartlineno;
ModTag moduleType;
ModTag currentModuleType;


static Vec<const char*> modNameSet;
static Vec<const char*> modNameList;
static Vec<const char*> modDoneSet;
static Vec<CallExpr*> modReqdByInt; // modules required by internal ones


void addModuleToParseList(const char* name) {
void addModuleToParseList(const char* name, CallExpr* useExpr) {
const char* modName = astr(name);
if (modDoneSet.set_in(modName) || modNameSet.set_in(modName)) {
// printf("We've already seen %s\n", modName);
} else {
// printf("Need to parse %s\n", modName);
if (currentModuleType == MOD_INTERNAL) {
modReqdByInt.add(useExpr);
}
modNameSet.set_add(modName);
modNameList.add(modName);
}
Expand Down Expand Up @@ -67,14 +71,14 @@ static bool firstFile = true;

ModuleSymbol* ParseFile(const char* filename, ModTag modType) {
ModuleSymbol* newModule = NULL;
moduleType = modType;
currentModuleType = modType;
yyfilename = filename;

yylloc.first_column = yylloc.last_column = 0;
yylloc.first_line = yylloc.last_line = yystartlineno = chplLineno = 1;
yyin = openInputFile(filename);

if (printModuleFiles && modType != MOD_INTERNAL) {
if (printModuleFiles && (modType != MOD_INTERNAL || developer)) {
if (firstFile) {
fprintf(stderr, "Parsing module files:\n");
firstFile = false;
Expand Down Expand Up @@ -146,4 +150,50 @@ void parseDependentModules(ModTag modtype) {
}
}
}

// Clear the list of things we need out. On the first pass, this
// will be the standard modules used by the internal modules which
// are already captured in the modReqdByInt vector and will be dealt
// with by the conditional below. On the second pass, we're done
// with these data structures, so clearing them out is just fine.
modNameList.clear();
modNameSet.clear();

// if we've just finished parsing the dependent modules for the
// user, let's make sure that we've parsed all the standard modules
// required for the internal modules require
if (modtype == MOD_USER) {
forv_Vec(CallExpr*, moduse, modReqdByInt) {
BaseAST* moduleExpr = moduse->argList.first();
UnresolvedSymExpr* oldModNameExpr = toUnresolvedSymExpr(moduleExpr);
if (oldModNameExpr == NULL) {
INT_FATAL("It seems an internal module is using a mod.submod form");
}
const char* modName = oldModNameExpr->unresolved;
bool foundInt = false;
bool foundUsr = false;
forv_Vec(ModuleSymbol, mod, allModules) {
if (strcmp(mod->name, modName) == 0) {
if (mod->modTag == MOD_STANDARD || mod->modTag == MOD_INTERNAL) {
foundInt = true;
} else {
foundUsr = true;
}
}
}
// if we haven't found the standard version of the module then we
// need to parse it
if (!foundInt) {
ModuleSymbol* mod = ParseFile(stdModNameToFilename(modName),
MOD_STANDARD);
// if we also found a user module by the same name, we need to
// rename the standard module and the use of it
if (foundUsr) {
mod->name = astr("chpl_", modName);
UnresolvedSymExpr* newModNameExpr = new UnresolvedSymExpr(mod->name);
oldModNameExpr->replace(newModNameExpr);
}
}
}
}
}
9 changes: 9 additions & 0 deletions compiler/util/files.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,15 @@ const char* modNameToFilename(const char* modName, bool isInternal,
return fullfilename;
}

const char* stdModNameToFilename(const char* modName) {
const char* fullfilename = searchPath(stdModPath, astr(modName, ".chpl"),
NULL);
if (fullfilename == NULL) {
INT_FATAL("Can't find standard module %s\n", modName);
}
return fullfilename;
}


static void helpPrintPath(Vec<const char*> path) {
forv_Vec(const char*, dirname, path) {
Expand Down
10 changes: 5 additions & 5 deletions test/modules/bradc/printModStuff/foo.good
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,17 @@ end of module search dirs
Parsing module files:
subdir/bar.chpl
foo.chpl
$CHPL_HOME/modules/standard/Types.chpl
$CHPL_HOME/modules/standard/Math.chpl
$CHPL_HOME/modules/standard/List.chpl
$CHPL_HOME/modules/standard/Sort.chpl
$CHPL_HOME/modules/standard/Search.chpl
subdir/baz.chpl
subdir1/bak.chpl
subdir3/bam.chpl
subdir2/bap.chpl
subdir4/subdir/bax.chpl
./bah.chpl
$CHPL_HOME/modules/standard/Types.chpl
$CHPL_HOME/modules/standard/Math.chpl
$CHPL_HOME/modules/standard/List.chpl
$CHPL_HOME/modules/standard/Sort.chpl
$CHPL_HOME/modules/standard/Search.chpl
In bar
In baz
In bak
Expand Down
5 changes: 0 additions & 5 deletions test/modules/bradc/userInsteadOfStandard/foo.future

This file was deleted.

5 changes: 0 additions & 5 deletions test/modules/bradc/userInsteadOfStandard/foo2.future

This file was deleted.

1 change: 1 addition & 0 deletions test/modules/bradc/userInsteadOfStandard/foo2.good
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
warning: Ambiguous module source file -- using ./Math.chpl over /Users/bradc/chapel/modules/standard/Math.chpl
In my math!
In my foo2

0 comments on commit 557eba1

Please sign in to comment.