Skip to content

CFE accepts records with non-primitive equality as elements of const sets #50515

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

Closed
chloestefantsova opened this issue Nov 18, 2022 · 3 comments
Assignees
Labels
legacy-area-front-end Legacy: Use area-dart-model instead.

Comments

@chloestefantsova
Copy link
Contributor

chloestefantsova commented Nov 18, 2022

UPDATE: As mentioned by @eernstg in the comment below, the non-trivial equality of objects should affect the constant sets with such objects, not the ability of the objects to be constant. The original contents of the issue is left at the bottom, for archival purposes.

The following program is compiled by the CFE without any compile-time error.

class A {
  const A();

  bool operator==(Object other) => false;
}

test() => const <(A,)>{ (A(),) };

A compile-time error is expected because A declares a non-trivial equality, and records containing such objects can't be elements of constant sets.

The analyzer displays the following error message for that program.

error - asdf.dart:7:25 - The type of an element in a constant set can't override the '==' operator, but the type '(A)' does. Try using a different value for the element, or removing the keyword 'const' from the set. - const_set_element_type_implements_equals

THE ORIGINAL ISSUE CONTENTS:

The following code is accepted by the CFE, but a compile-time error is expected. Note that const A() is accepted even outside of record literals.

class A {
  const A();

  bool operator==(Object other) => false;
}

test() => const (A(), A());

test2() => const A();

The Kernel output is the following:

library /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;

class A extends core::Object /*hasConstConstructor*/  {
  const constructor •() → self::A
    : super core::Object::•()
    ;
  operator ==(core::Object other) → core::bool
    return false;
}
static method test() → dynamic
  return #C2;
static method test2() → dynamic
  return #C1;
constants  {
  #C1 = self::A {}
  #C2 = (#C1, #C1)
}
@chloestefantsova chloestefantsova added the legacy-area-front-end Legacy: Use area-dart-model instead. label Nov 18, 2022
@asashour

This comment was marked as outdated.

@eernstg
Copy link
Member

eernstg commented Nov 18, 2022

This is not a problem. A class can have a constant constructor and be used in constant expressions, and still have an operator == that runs any code it wants. However, there should be an error if const A() is used in a situation where it is required to have a primitive operator ==, like const s = {A()};. I do get that error from both the CFE and the analyzer. I think it's fine!

@chloestefantsova chloestefantsova changed the title CFE accepts objects with non-primitive equality as constants CFE accepts records with non-primitive equality as elements of const sets Nov 21, 2022
@chloestefantsova
Copy link
Contributor Author

This is not a problem. A class can have a constant constructor and be used in constant expressions, and still have an operator == that runs any code it wants. However, there should be an error if const A() is used in a situation where it is required to have a primitive operator ==, like const s = {A()};. I do get that error from both the CFE and the analyzer. I think it's fine!

Thanks for the clarification, Erik! I updated the text of the issue accordingly.

@johnniwinther johnniwinther self-assigned this Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-front-end Legacy: Use area-dart-model instead.
Projects
None yet
Development

No branches or pull requests

4 participants