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

Using List and Map of user classes in a Record fails to compile #18005

Closed
arezaii opened this issue Jun 29, 2021 · 4 comments
Closed

Using List and Map of user classes in a Record fails to compile #18005

arezaii opened this issue Jun 29, 2021 · 4 comments

Comments

@arezaii
Copy link
Collaborator

arezaii commented Jun 29, 2021

Summary of Problem

Using both Map and List in a record, without specifying deallocation strategy keywords owned or shared for user classes, results in a compilation error.

Steps to Reproduce

Source Code:

use List;
use Map;

class A {
  var msg:string;
}

class B {
  var name:string;
}

record R {
  var foo: list(A);
  var bar: map(string, B);
}

Compile command:
chpl exampleCode.chpl

Output

exampleCode.chpl:13: In method 'foo':
exampleCode.chpl:13: internal error: invalid attempt to get reference type [AST/primitive.cpp:235]

Note
Curiously, the code above compiles if either map or list is omitted, but not when both are present in the record. I would expect to see similar behavior to the above in either of these cases.

so changing the example code to either of the below examples will result in successful compilation:

record R {
  var foo: A;
  var bar: map(string, B);
}

OR

record R {
  var foo: list(A);
  var bar: B;
}

Alternatively, specifying a deallocation strategy for user classes A and B will successfully compile, e.g:

record R {
 var foo: list(owned A);
 var bar: map(string, shared B);
}

Configuration Information

  • Output of chpl --version: chpl version 1.25.0 pre-release (75a0eeea87)
  • Output of $CHPL_HOME/util/printchplenv --anonymize:
CHPL_TARGET_PLATFORM: darwin
CHPL_TARGET_COMPILER: clang
CHPL_TARGET_ARCH: x86_64
CHPL_TARGET_CPU: native
CHPL_LOCALE_MODEL: flat
CHPL_COMM: none
CHPL_TASKS: qthreads
CHPL_LAUNCHER: none
CHPL_TIMERS: generic
CHPL_UNWIND: none
CHPL_MEM: jemalloc
CHPL_ATOMICS: cstdlib
CHPL_GMP: bundled
CHPL_HWLOC: bundled
CHPL_RE2: bundled
CHPL_LLVM: none *
CHPL_AUX_FILESYS: none
  • Back-end compiler and version, e.g. gcc --version or clang --version:
Apple clang version 12.0.5 (clang-1205.0.22.9)
Target: x86_64-apple-darwin20.5.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin
@mppf
Copy link
Member

mppf commented Jun 30, 2021

I think this is a case where the program shouldn't compile but we need a better error message.

With this version, I see the same (unhelpful) error message with both foo and bar fields or either one commented out.

use List;
use Map;

class A {
  var msg:string;
}

class B {
  var name:string;
}

record R {
  //var foo: list(A);
  var bar: map(string, B);
}

var x = new R();
writeln(x);

The problem here is that A and B refer to the classes with generic management. As a result, list(A) and map(string, B) are generic types. Which is probably not what you intended.

It might be possible or reasonable to make list/map issue an error if the element type is generic. However this might go against some partial instantiation scenarios (maybe I want to say var x: list(SomeGenericType) = constructTheList; as a way to be clearer about the type without specifying everything, say).

@arezaii
Copy link
Collaborator Author

arezaii commented Jul 10, 2024

today this provides a much nicer, helpful error message that helped me change the code to compile, so I will close this as it seems addressed.

@arezaii arezaii closed this as completed Jul 10, 2024
@bradcray
Copy link
Member

@arezaii : Would you consider filing a test locking this behavior in (if there isn't obviously one already)?

@arezaii
Copy link
Collaborator Author

arezaii commented Jul 10, 2024

I found the test partial-no-q-warning covers this type of problem (partial instantiation of a generic without a ?) and demonstrates the warning/error that I get from this example now, but for completeness I added this example in #25493

arezaii added a commit that referenced this issue Jul 18, 2024
Adds a test for partial instantiation of a generic to serve as a
regression test against #18005.

TESTING:

- [x] paratest `[Summary: #Successes = 17375 | #Failures = 0 | #Futures
= 921]`
- [x] paratest gasnet `[Summary: #Successes = 17556 | #Failures = 0 |
#Futures = 927]`

[reviewed by @DanilaFe - 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

4 participants