Chapel: "use only" shades class methods

[buddha314]

When doing use BThang only the class methods are shaded

module AThang {
  /*
  athang.chpl
   chpl athang.chpl -M/Users/buddha/sandbox/chapel/usey -o thangs
  */
  use BThang only;

  proc main() {
    writeln("Doin' a thang here...");
    var bthang = new BThang.MahThang();
    writeln(bthang.sayMyName());
  }
}

/* bthang.chpl */
module BThang {
  class MahThang {
    proc sayMyName() {
      return "Ain't Nuthin but a B Thang, Baaaaaaby...";
    }
  }
}

athang.chpl:7: In function 'main':
athang.chpl:10: error: unresolved call 'MahThang.sayMyName()'

The function sayMyName() is not recognized as a class function if only is used. Remove only and it works fine.

[ben-albrecht]

@buddha314 - Thanks for reporting. Looks like a bug to me.

Might be good to get input from @bradcray when he gets back.

[lydia-duncan]

This is intentional, though confusing. use BThang only; means none of BThang's symbols are accessible without the module prefix. You were only able to use MahThang's constructor because you called it via BThang.MahThang() - none of MahThang's methods are visible in your current scope. If you change the line to use BThang only MahThang;, the methods will be visible.

[buddha314]

If I have the class object, should I not be able to use the methods of that class? Why let me call the constructor then?

[lydia-duncan]

You're able to find the constructor in your current set up because it isn't a method on the type, it's a function that generates the type instance for you and so you can access it through the module prefix. The type itself isn't really known in your current scope, though. Try declaring the bthang variable with an explicit type and you'll see what I mean - you can't just say var bthang: MahThang = new BThang.MahThang(); because the scope doesn't even know about the type MahThang on its own. Why should it know about the methods or fields on a type whose name it doesn't even know?

You can still pass the instance to some place that can find the methods (like another scope that has used BThang without the limitation provided) so it isn't completely useless to you in that scope, but it is really better to have included the type in your only list.

[lydia-duncan]

I think it would probably be better for us to prevent functions from returning types that the current scope doesn't understand. This is kind of related to the questions raised in #6067

[buddha314]

The best part of this ticket is imagining Lydia trying to talk like Bootsy Collins while writing it.

[bradcray]

[I'm catching up on November mail today...]

I think Brian's intuition is right on this one. The "only" clause should affect the visibility of symbols that are declared at module scope; a method like sayMyName() is in MahThang's scope, not BThang's and therefore shouldn't be affected by the use...only clauses.

[BryantLam]

It doesn't appear intuitive to allow constructing an object with the module prefix, but then prevent use of methods inside that object.

[lydia-duncan]

9983 is a variation of this same issue for deinit specifically. I suspect the solution for #9983 will not be distinct from the solution for this issue and will try to get to solving this issue soon

[lydia-duncan]

To make sure I'm understanding people's positions correctly, do we think these programs should work as well?

secondary-in-same-module.chpl:

module M {
  class Foo { ... }
  proc Foo.secondary() {...} // secondary's scope is M, though it is defined in the same module as Foo
}
module M2 {
  use M only;

  proc main() {
    var x = new M.Foo();
    x.secondary();
  }
}

secondary-diff-module.chpl:

module M {
  class Foo { ... }
}
module M2 {
  use M;
  proc Foo.secondary() { ... }
}
module M3 {
  use M only;

  proc main() {
    var x = new M.Foo();
    x.secondary(); // Shouldn't be accessible because we didn't include M2, right?
  }
}

secondary-diff-module-use.chpl:

module M {
  class Foo { ... }
}
module M2 {
  use M;
  proc Foo.secondary() { ... }
}
module M3 {
  use M only;
  use M2 only;

  proc main() {
    var x = new M.Foo();
    x.secondary(); // Should it be accessible now that M2 was referenced?
  }
}

[lydia-duncan]

Our module code seems to arbitrarily decide between defining methods in the type declaration or outside it, so our decision on the first case may impact common types

[BryantLam]

@lydia-duncan: As a user, I would expect the examples you've listed to behave as the comments on x.secondary() have described.

[lydia-duncan]

So the first case should work, the second fail, and the third work?

[bradcray]

I tend to think the first should work (that there shouldn't be a difference between primary and secondary methods in this respect). I don't have a strong feeling about the second, and it feels related to point-of-instantiation issues to me, which makes me curious what @mppf thinks. The third "feels" to me like it should work, though it could depend on the answer to the second.

[lydia-duncan]

The second is part of why my initial reaction was "we shouldn't support this at all". Having the additional methods in separate modules means that the user controls which symbols are visible by determining which of the supporting modules are used in their program. Having qualified access to the type get around that rule feels like it drastically changes that behavior.

[lydia-duncan]

Extending the scope for primary methods seems reasonable, I'm ambivalent on secondary methods defined in the same module (but mostly because our standard and package library code uses them interchangeably so it makes sense to keep them unified), and am more willing to accept secondary methods in other modules if the other module was explicitly mentioned via only; or except *;

[lydia-duncan]

Having qualified access to the type get around that rule feels like it drastically changes that behavior.

To clarify, I think these programs should behave the same:

module M {
  class Foo { ... }
}
module M2 {
  use M;
  proc Foo.secondary() { ... }
}
module M3 {
  use M only;

  proc main() {
    var x = new M.Foo();
    x.secondary(); // Up for debate
  }
}

module M {
  class Foo { ... }
}
module M2 {
  use M;
  proc Foo.secondary() { ... }
}
module M3 {
  use M;

  proc main() {
    var x = new Foo();
    x.secondary(); // Fails today
  }
}

[BryantLam]

Having the additional methods in separate modules means that the user controls which symbols are visible by determining which of the supporting modules are used in their program. Having qualified access to the type get around that rule feels like it drastically changes that behavior.

I'm interested in how the examples would differ when compared with constrained generic interfaces that I would want to define on a type in another module.

[mppf]

I don't have a strong feeling about the second, and it feels related to point-of-instantiation issues to me, which makes me curious what @mppf thinks.

For point-of-instantiation to be relevant, there has to be a generic type. So let's make the example have a generic Foo:

module M {
  class Foo { var x; }
}
module M2 {
  use M;
  proc Foo.secondary() { ... }
}
module M3 {
  use M only;

  proc main() {
    var x = new M.Foo(5);
    x.secondary(); // Shouldn't be accessible because we didn't include M2, right?
  }
}

Here, main is the point-of-instantiation for the type Foo. But, I don't see any reason that point-of-instantiation would make M2's Foo.secondary visible in this example, since it's not used anywhere and doesn't have any module initialization code to speak of.

Here's an example where the point-of-instantiation rule would be relevant:

module M {
  class Foo { var x; }
}
module M2 {
  use M;
  proc Foo.secondary() { ... }
  var myGlobal = new Foo(5); // might be the point-of-instantiation for Foo(int)
}
module M3 {
  use M only;

  proc initM2() { use M2; }
  initM2();

  proc main() {
    var x = new M.Foo(5); // might reuse instantiation Foo(int) from M2
    x.secondary(); // presumably this method is visible through Foo(int)'s point of instantiation
  }
}

I (still) think we should try to replace the point-of-instantiation rule with constrained generics. Point-of-instantiation is inherently messy in my opinion.

Why would Foo(int)'s point of instantiation need to be included in finding the visible methods?
I don't have the best example but the below one starts to get at the issue:

module Base {
  class Foo { var x; }
}

module Helper {
  use Base;
  record R { }
  proc helper(arg:R) { }
  proc Foo.helper() { helper(this.x); }
  var global = new Foo(new R()); // point of instantiation for Foo(R)
}

module Main {
  use Helper only global;

  proc main() {
    global.helper(); // why is Foo.helper() visible? Maybe Foo(R)'s point of instantiation.
  }
}

[lydia-duncan]

I don't think the point of instantiation would be M2 for your second example, since the module is never referenced and thus never had its contents evaluated. I would expect the point of instantiation for Foo(int) to be main()

[mppf]

@lydia-duncan - yeah, I'll fix the example.

[lydia-duncan]

@mppf - I'm curious about your thoughts on what should happen for the earlier examples. It sounds like you agree that my second example should generate an error, what do you think about the other ones?

[mppf]

secondary-in-same-module.chpl:

I think this should run OK.

secondary-diff-module.chpl:

I wouldn't expect this to compile as-is.

secondary-diff-module-use.chpl:

I would expect this to run.

To me, the main issue here is that the Chapel compiler treats all of these methods the same as top-level functions (with special arguments). But they aren't top-level functions - as methods they are already "scoped" - meaning you can only call them within something of that type - so they don't conflict with the names of top-level functions. I'm saying that because the method receiver is already identified (generally), the issues that would arise from importing too much (e.g. with use M only; if it always imported all methods, say) seem not so big of a deal.

[lydia-duncan]

Closed by #10439