chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.76k stars 414 forks source link

Support param folding of use and import statements #17438

Open dlongnecke-cray opened 3 years ago

dlongnecke-cray commented 3 years ago

As a Chapel user, I want support for param folding of use and import statements so that I can write code that is sensitive to the movement/renaming of modules like the following:

proc test1() {
  use Version;

  if chplVerison >= createVersion(2, 0) {
    use SomeTwoPointOhModule;
  } else {
    // Import any arbitrary, existing module.
    use List;
  }
}
test1();

Today, trying to compile this code yields:

Test.chpl:6: In function 'test1':
Test.chpl:10: error: Cannot find module or enum 'SomeTwoPointOhModule'

I ran into this problem recently while trying to adjust Arkouda to silence deprecation warnings in Memory that were introduced for 1.24.

proc getMemUsed() {
  use Version;
  if chplVersion >= createVersion(1, 24) {
    use Memory.Diagnostics;
    return memoryUsed();
  } else {
    use Memory;
    return memoryUsed();
  }
}

However this didn't work as expected.

lydia-duncan commented 3 years ago

Implementation details: I'm pretty sure this is because param folding happens after we've determined use and import statements for a block (because those need to be known at scope resolution time for use during that pass)

lydia-duncan commented 3 years ago

I dunno how reasonable it would be to tackle before compiler 2.0

bradcray commented 3 years ago

I understand and agree with the desire here, but I think we'd want to find another way to express it since use and import statements only apply to the current lexical scope. At least, if I had this feature, I think I'd want it to apply to the code block that follows the version check, not just the then/else clauses of the version check itself. So my head goes to something more like:

use (if chplVersion >= createVersion(1, 24) then Memory.Diagnostics else Memory);

though that's pretty hideous, and also weird/new in that use statements take symbol names as their arguments rather than general expressions.

bradcray commented 3 years ago

(several times over the project's history, we've discussed the concept of a parse-time conditional—or scope-less conditional—to handle cases similar to this. For example, the conditional definition of a procedure:

parseTimeIf chplVersion < 1.24 {
  proc routineAddedInOneTwentyFour() {
    ...
  }
}

where today, this similarly wouldn't work because the procedure is local to the then clause. So in a sense, this could be considered to the cpp #if concept. The assumption has been that the types of things you could make the conditional depend on would be significantly limited relative to the general param-time machinery. However, we haven't ever had the right combination of design and need that have resulted in this being added to the language.

dlongnecke-cray commented 3 years ago

though that's pretty hideous, and also weird/new in that use statements take symbol names as their arguments rather than general expressions.

This form doesn't bother me that much/look that hideous to me. And it seems like the machinery required to implement that would be such that we could do both (the version requested by the OP and this same-scope version). Though that's just a relatively uninformed guess on my part.

We could also compact the syntax to something like:

use Foo if expression else Bar;

Which I think is reasonable even if different from normal if-expression syntax.

bradcray commented 3 years ago

Another approach would be:

use(boolExpr) Foo;

I think the problem in either case then gets to the point Lydia made (and that I alluded to when saying "the types of things you could make the conditional depend on would be significantly limited relative to the general param-time machinery"). Specifically, things like chplVersion and createVersion and <= between them are general symbols and expressions that we'd need to look up and resolve in the normal way, which typically happens long after we've resolved namespaces. So what simple conditional tests could we support early in compilation that would give the richness required without crippling things too much?

dlongnecke-cray commented 3 years ago

Note that this isn't blocking anymore, I found a (albeit ugly) workaround here: https://github.com/mhmerrill/arkouda/compare/master...dlongnecke-cray:fix-memory-memused

With that said, I do wonder if this is something that we should wait to tackle until compiler 2.0 (as Lydia says above). I imagine changing the order of the passes / and or flattening scopeResolve and resolve into a single pass might solve part of this problem.

I'm not sure if the pass constraints/coupling make that squashing impossible, but I feel like we should be able to incrementally resolve (not today, but in a wonderful 2.0 future) something like:

use Version;
use Foo if chplVersion >= createVersion(1,24) else Bar;

Unless I'm seriously misunderstanding something.

bradcray commented 3 years ago

but I feel like we should be able to incrementally resolve (not today, but in a wonderful 2.0 future) something like:

Here's a potential challenge in doing that: As the language is defined today, use and import statements affect the entire lexical scope in which they're defined, regardless of order. That is, a use that followed your use Foo; can introduce symbols that statements that appear before it can refer to. So with those semantics, we could run into potential issues since the if ... clause on a use statement might rely on symbols that another use or import brought into scope. In the worst case, there could be an instability in which the then and else clauses of distinct use statements either brought in (or not) symbols or definitions that changed the conditionals on others, such that depending on evaluation order, you'd get a different result.

Essentially, I think the use or import clauses would turn from declarative statements (in which evaluation order doesn't really matter in Chapel today) to imperative statements (in which it does).

mppf commented 3 years ago

I wonder if this is better handled by some kind of versioned module managed by Mason.

mppf commented 3 years ago

I think that supporting the feature has some implications for the order of resolution for use statements and things depending upon them -- see https://github.com/chapel-lang/chapel/issues/13041#issuecomment-873393725

bradcray commented 2 years ago

In discussing this today, it seemed promising to me that the new compiler's "do all resolution in one pass" approach may help with this issue since for a case like:

proc getMemUsed() {
  use Version;
  if chplVersion >= createVersion(1, 24) {
    use Memory.Diagnostics;
    return memoryUsed();
  } else {
    use Memory;
    return memoryUsed();
  }
}

it would (ideally, hopefully) be resolving the symbols and types/values/etc. of Version and chplVersion >= createVersion at a similar time (i.e., all statements in the main scope of getMemUsed()), where today's compiler resolves all symbols and use/imports early across the entire program and then all resolution of types/values/params/etc. later (so necessarily processes the use statements prior to the folding of the param conditional).

There was some discussion of whether this was related to whether uses had to appear at the top of a scope or not, but that felt like something of a red herring to me (which isn't to say that it's an idea we shouldn't consider) because it feels like the main necessary change is "resolve symbols and expressions for this scope before going on to the child scopes" (or 'scope' in the case of param conditionals). Put another way, if we kept the current "uses in a block apply to that entire block, you could still get:

proc getMemUsed() {
  if chplVersion >= createVersion(1, 24) {
    use Memory.Diagnostics;
    return memoryUsed();
  } else {
    use Memory;
    return memoryUsed();
  }
  use Version;
}

to resolve today (even though it's ugly/poor form) because you'd still reolve the top-level statements, which would still let you fold the param conditional, before going on to the single remaining child statement.

dlongnecke-cray commented 2 years ago

There was some discussion of whether this was related to whether uses had to appear at the top of a scope or not, but that felt like something of a red herring to me (which isn't to say that it's an idea we shouldn't consider) because it feels like the main necessary change

In particular, whether uses could be imperative was brought up in the context of "how can we make this more useful for Arkouda?", where a developer might want to write many implementations of a procedure with the same name foo(), where the module to use to get at foo() depends on the platform the code is being compiled on.

In some cases, writing a function to wrap the different foos() (or call fooForArm64(), fooForWindows(), similar to what you illustrated) is unavoidable, however I would hope that in some cases a developer could just utilize the approach of moving different implementations of foo() into different modules in order to better organize the code and to avoid this boilerplate.

While it might be possible for the current 2.0 effort to achieve what you illustrated while keeping the current declarative semantics for use/import statements, if we entertain changing use statements to be imperative then that might require further adjustments to how symbol tables are built up when resolving scopes. I think @mppf might have alluded to something like this, I'm not sure.

With all that being said, even if we could make your example work, it might be good to have the "imperative use" conversation just so that we can get the implementation details in order and avoid a duplication of effort. I'm not sure how difficult later changes would be or if it's really worth the preparation, but it's just something I'm worried about.

vasslitvinov commented 2 years ago

Maybe we can support compatibility modules that work somewhat like this:

Brad+Michael suggest using require for this, enabling it to adjust the module search path from anywhere in the program with -M path/to/compat/module. Then the require along these lines could be inside of a folded if, or maybe it could be using a param string.

mppf commented 2 years ago

Actually now I am thinking that we can allow things like

use Version;
use Foo if chplVersion >= createVersion(1, 24) else Bar;

or even

proc getModuleToUse() param {
  use Version;
  if chplVersion >= createVersion(1, 24) {
    return "Foo";
  } else {
    return "Bar";
}
use getModuleToUse()

without having to change a whole lot of the language rules around scoping (which #13041 discusses). (Please note that at this point, I am using these example features to describe how the scoping could work - I am not yet advocating for any particular syntax/feature). How? These features are identifiable syntactically (the first from the use/if form; the second from the use something() form). When the compiler sees one of these, it creates a temporary scope just for resolving the expression. The temporary scope contains all modules used/imported up to this point.

Let's focus on this example:

module M {
  use A;
  use B;
  use Version;
  use Foo if chplVersion >= createVersion(1, 24) else Bar;
  use OtherModule;
  ...
}

The compiler is in the process of figuring out what symbols are available within M. It sees the 'use Foo if' syntactical form and constructs a scope inside of M containing the use/imports up to that point and the expression we need to resolve, like this:

module M {
  { // this is a temporary scope
    use A;
    use B;
    use Version;
    chplVersion >= createVersion(1, 24)
  } 
}

Then, it does the regular scope resolution + function resolution to resolve the last expression there. It uses the last expression there to resolve the use if. Then, it uses that to compute the usual scope + what symbols are available for M. After this, the temporary scope is never used again.

What I like about this approach is that it allows us to consider straightforward features extending use/import for these cases without much impact on the rest of the language. The bit about processing the use statements in order is arguably more observable / noticeable with these features and any general expression inside of a use statement (like chplVersion >= createVersion(1, 24)) will not have the ability to see symbols brought in by later use / import statements. (Or later variables or functions, for that matter). But this is a already existing property of the use statement itself, rather than something about those symbols.

Here is an example (originally from https://github.com/chapel-lang/chapel/issues/13041#issuecomment-873393725 ) showing why it is that the order of the use statements does matter when we are resolving the use statements:

module Library {
  module SubModule { }
}

module Program {
  // this does not currently compile, but swap these two statements and it does
  use SubModule;
  use Library;
}
vasslitvinov commented 2 years ago

I like that, Michael!

Another feature we can consider is scope-less blocks that allow us to do things like:

depending on a param conditional.