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

Missing error message when returning an inner function that uses an outer-local var #6538

Closed
e-kayrakli opened this issue Jun 23, 2017 · 6 comments

Comments

@e-kayrakli
Copy link
Contributor

e-kayrakli commented Jun 23, 2017

Summary of Problem

Compiler should create an error message when the user's trying to return an inner function that uses local variables from it's parent function(s).

Current behavior is an internal error, that seems to be coming from a slightly irrelevant part of the compiler. Most likely it is due to a missing actual for an additional formal compiler creates in the inner function's signature to capture the outer variable.

Steps to Reproduce

Source Code:

proc foo() {
  var dummy = 10;
  return lambda (x:int) { return x*dummy; };
}
writeln(foo()(5));

Compile command:
chpl foo.chpl

Execution command:
Doesn't compile. (It shouldn't)

Associated Future Test(s):
PR: #6539

Adds test/users/engin/retInnerFunction.chpl

Configuration Information

  • Output of chpl --version:
chpl Version 1.16.0 pre-release (ad166c86f8)
Copyright (c) 2004-2017, Cray Inc.  (See LICENSE file for more details)
  • Output of $CHPL_HOME/util/printchplenv --anonymize:
CHPL_TARGET_PLATFORM: linux64
CHPL_TARGET_COMPILER: gnu
CHPL_TARGET_ARCH: native
CHPL_LOCALE_MODEL: flat
CHPL_COMM: none
CHPL_TASKS: qthreads
CHPL_LAUNCHER: none
CHPL_TIMERS: generic
CHPL_UNWIND: none
CHPL_MEM: jemalloc
CHPL_MAKE: make
CHPL_ATOMICS: intrinsics
CHPL_GMP: gmp
CHPL_HWLOC: hwloc
CHPL_REGEXP: re2
CHPL_WIDE_POINTERS: struct
CHPL_AUX_FILESYS: none
  • Back-end compiler and version, e.g. gcc --version or clang --version:
gcc (Ubuntu 6.3.0-12ubuntu2) 6.3.0 20170406
@mppf
Copy link
Member

mppf commented Jun 23, 2017

While I agree this is something we should have a better error message for, whether or not you can do this at all is an open design question for Chapel first-class functions. If the inner function is returned, presumably you'd want the dummy variable in the example to continue to exist and be accessible? I'm not sure we want to make the compiler do that. (Remember we have no garbage collection...)

@e-kayrakli
Copy link
Contributor Author

Attaching a piece of heap memory to a function pointer (isn't it what doing this is?) sounds like it might lead to some interesting idioms.

But at the same time, I personally would find something like this a bit confusing (compared to functionality it brings) and avoid writing code like this as much as possible.

@LouisJenkinsCS
Copy link
Member

LouisJenkinsCS commented Jun 23, 2017

My suggestion is something like C++11's lambdas. Capturing it by value if its like above (dummy refers to the value that it had when the lambda was created), or in the case of by reference for something allocated on the stack, perhaps as Engin suggested, in a thunk that gets cleaned up automatically or via delete, etc.

Edit:

Interestingly, if this idea was implemented, perhaps it could be applied to on statements? Like, non-blocking on statements that can reference everything in scope this way? Just an idea~

bradcray added a commit that referenced this issue Jun 27, 2017
Add new error message future for issue #6538

[contributed by @e-kayrakli, reviewed by me]

Adds a future for issue #6538 

Added test is reported as future with standard linux64.
@cassella
Copy link
Contributor

cassella commented Mar 6, 2019

Without --no-compile-time-nil-checking, I get the same error for this test and the one in #5544

buggy2.chpl:2: internal error: number of actuals does not match number of formals in this() [resolution/nilChecking.cpp:581]

With --no-compile-time-nil-checking, I get the same error for this test and for the one in #5544,

internal error: Indexing list out of bounds [AST/alist.cpp:73]

They both seem to be the same issue. In both cases, the lambda is referencing a (different flavor of) outer-local var.

@jabraham17
Copy link
Member

@e-kayrakli, looks like #20991 resolved the future associated with this, I think we can close this?

@e-kayrakli
Copy link
Contributor Author

Looks right based on the fact that that PR retired the future associated with this issue. Thanks!

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

6 participants