Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request]: Attribute support for auto-generated actuals #26684

Open
e-kayrakli opened this issue Feb 8, 2025 · 1 comment
Open

[Feature Request]: Attribute support for auto-generated actuals #26684

e-kayrakli opened this issue Feb 8, 2025 · 1 comment

Comments

@e-kayrakli
Copy link
Contributor

Summary of Feature

Description:
Can we have an attribute that can be attached to functions, which would make the compiler generate some actuals for the function's formals?

Is this issue currently blocking your progress?
No, but having this feature could improve Arkouda code quality by a big margin.

Code Sample

This is what we have in Arkouda's loggers:

use Reflection;

proc foo() {

  fooLogger.debug(getModuleName(), getRoutineName(), getLineNumber(),
                  "foo called");

}

This is way too cumbersome. It appears 100s of times in Arkouda probably adding 1000s of lines of code. Moreover, it discourages adding more logging output.

Here are some potential ideas.

Without too much thinking:

// first arg to attr is the formal name, second arg is its value at the callsite
@callsiteArg(moduleName, Reflection.getModuleName())
@callsiteArg(routineName, Reflection.getRoutineName())
@callsiteArg(lineNumber, Reflection.getLineNumber())
proc Logger.debug(moduleName, routineName, lineNumber, msg:string) {

}

But then, what if Reflection was not in scope in the callsite? Can we handle that?

Something more tailored for this use case

@passModuleName(moduleName)
@passRoutineName(routineName)
@passLineNumber(lineNumber)
proc Logger.debug(moduleName, routineName, lineNumber, msg:string) {

}

We have some stuff for line/file no propagation in the compiler, but I am guessing this is something completely different implementation-wise. I expect this attribute to be handled very early in compilation to change callsites from

proc foo() {
  fooLogger.debug("foo called");
}

into

proc foo() {

  const chpl_moduleName = Reflection.getModuleName();
  const chpl_routineName = Reflection.getRoutineName();
  const chpl_lineNumber = Reflection.getLineNumber();

  fooLogger.debug("foo called",
                  moduleName=chpl_moduleName,
                  routineName=chpl_routineName,
                  lineNumber=chpl_lineNumber);
}
@e-kayrakli e-kayrakli changed the title [Feature Request]: Attributed support for auto-generated actuals [Feature Request]: Attribute support for auto-generated actuals Feb 8, 2025
@jabraham17
Copy link
Member

To my mind, this feels like a niche workaround. This issue is trying to optimize the following case

foo(Reflection.getLineNumber());
foo(Reflection.getLineNumber()); // why do I have to call this here every time?

proc foo(lineNum) {
  writeln("error on ", lineNum);
}

For better software design, we want to hide the call to getLineNumber. But if we do, we have a problem

foo();
foo();

proc foo() {
  fooInner(Reflection.getLineNumber()); // this gives the wrong line number
}
proc fooInner(lineNum) {
  writeln("error on ", lineNum);
}

Because getLineNumber's behavior is sensitive to its textual location, this doesn't work. So this feature request is really only needed for functions with this property (i.e. textual sensitivity), and thats mostly just a subset of Reflection module functions.

I think my primary issue here is that @callsiteArg fixes a problem with a few functions defined in one module, its not generally useful. Because in other situations, you just write a foo/fooInner and get the same benefits. So if we could defined attributes using Chapel code on a module by module basis (like python decorators), I would be all for it. But we can't, Chapel attributes need special compiler support and that feels wrong to me in this case. Would I like to see the ability to define custom attributes in Chapel code? Yes. But that a long way off and a much bigger design problem.

This issue feels very thematically related to #18788. One of the ideas buried in a thread on that issue is to adjust textually sensitive Reflection module functions to take a call depth argument, similar to how compilerWarning/compilerError work. So you could write the following

foo();
foo();

proc foo() {
  fooInner(Reflection.getLineNumber(), 2); // this gives the line number of the calls to foo.
}
proc fooInner(lineNum) {
  writeln("error on ", lineNum);
}

Aside: While I like this idea, I think it has some implementation issues. For example, above foo is a concrete function called twice. But because the body of the function needs to change in based on the callsite, does it need to re-resolved/stamped out for every single invocation? That seems really undesirable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants