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

In frog, generated global symbol conflicts with generated symbol on window #1837

Closed
vsmenon opened this issue Feb 24, 2012 · 7 comments
Closed

Comments

@vsmenon
Copy link
Member

vsmenon commented Feb 24, 2012

The follow is failing when compiled in frog and run in the browser. The 2 expect statements should be testing the same thing. But, the first succeeds and the second (isTrue) fails.

library('WindowEqualityTest');

import('../../../testing/unittest/unittest.dart');

import('dart:dom');

main() {
  var obfuscated = null;

  test('notNull', () {
      Expect.isNotNull(window);
      Expect.isTrue(window != obfuscated);
    });
}

The fundamental problem can be seen in generated code:

function $eq(x, y) {
  if (x == null) return y == null;
  return (typeof(x) != 'object') ? x === y : x.$eq(y);
}
// TODO(jimhug): Should this or should it not match equals?
$defProp(Object.prototype, '$eq', function(other) {
  return this === other;
});
function $ne(x, y) {
  if (x == null) return y != null;
  return (typeof(x) != 'object') ? x !== y : !x.$eq(y);
}

The $ne operator should be invoking the 2nd $eq (the one defined on Object.prototype). Instead, it's invoking the first one. Note, the window object is the global object!

@DartBot
Copy link

DartBot commented Feb 24, 2012

This comment was originally written by ngeoffray@google.com


Thanks Vijay for the bug report, I'll take a look.


Added Started label.

@DartBot
Copy link

DartBot commented Feb 24, 2012

This comment was originally written by ngeoffray@google.com


Should be fixed with https://chromiumcodereview.appspot.com/9447049. Vijay, could you verify?


Set owner to ngeoffray@google.com.
Added Fixed label.

@vsmenon
Copy link
Member Author

vsmenon commented Feb 24, 2012

Thanks! Synced and verified.

@vsmenon
Copy link
Member Author

vsmenon commented Feb 24, 2012

Reopening as the underlying issue is still present. The above is fixed, but I can still get things to break.

Here's a new test - a negative and a positive. Both pass in Dartium. Both fail with frog. The 2nd in particular is bad as the global navigator is masking the one on defined on window.

#library('WindowEqualityTest');
#import('../../../testing/unittest/unittest.dart');
#import('dart:dom', prefix: 'dom');

// Not defined in dom.Window.
foo(x) => x;

// Defined in dom.Window.
get navigator() => "Dummy";

class Unused {
  foo(x) => 'not $x';
}

main() {
  var win = new Unused();
  win = dom.window;

  test('windowNonMethod', () {

      final message = foo("Hello World");
      try {
        String x = win.foo(message);
        Expect.fail('Should not reach here: $x');
      } catch (NoSuchMethodException e) {
        // Expected exception.
      } catch (Exception e) {
        Expect.fail('Wrong exception: $e');
      }
    });

  test('windowMethod', () {

      final message = navigator;
      final x = win.navigator;
      Expect.notEquals(message, x);
    });
}


Added Triaged label.

@vsmenon
Copy link
Member Author

vsmenon commented Feb 24, 2012

Expanding the test case to include Stephen's example. All three pass in Dartium. All three fail with Frog.

#library('WindowEqualityTest');
#import('../../../testing/unittest/unittest.dart');
#import('dart:dom', prefix: 'dom');

// Not defined in dom.Window.
foo(x) => x;

// Defined in dom.Window.
get navigator() => "Dummy";

class Unused {
  foo(x) => 'not $x';
}

$eq(x, y) => false;

main() {
  var win = new Unused();
  win = dom.window;

  test('windowNonMethod', () {

      final message = foo("Hello World");
      try {
        String x = win.foo(message);
        Expect.fail('Should not reach here: $x');
      } catch (NoSuchMethodException e) {
        // Expected exception.
      } catch (Exception e) {
        Expect.fail('Wrong exception: $e');
      }
    });

  test('windowMethod', () {

      final message = navigator;
      final x = win.navigator;
      Expect.notEquals(message, x);
    });

  test('windowEquals', () {
      Expect.isFalse($eq(win, win));
      Expect.isTrue(win == win);
    });
}

@rakudrama
Copy link
Member

The global scope is the window object, so anything that is a direct property of window, or on DOMWindow.prototype or Object.prototype is visible in the global scope.

The instance methods (incl getters, is$checks etc) of DOMWindow live on DOMWindow.prototype, so they are shadowed by a global with the same name (which is a direct property of the window instance). The instance variables (fields) of the window instance conflict directly with a global with the same name.

DOMWindow is marked with a special directive, native '@...'.
This should be causing all globals with names conflicting with members to be renamed, with the library name, e.g. WindowEqualityTest_navigator. So the call to the global get navigator should be get$WindowEqualityTest_navigator()

It is possible there is something wrong with
  * The processing of '@' (i.e. not calling world._addTopName with all the members)
  * The workings of world._addTopName (this is hard to follow since it has to give the same final answer incrementally regardless of the order of calls)
  * The name at the generation of the getter function
  * The name at the call to the getter function

@DartBot
Copy link

DartBot commented Feb 26, 2012

This comment was originally written by ngeoffray@google.com


I have submitted a fix for better mangling in https://chromiumcodereview.appspot.com//9465041. This patch however is not fixing NoSuchMethodException on the global object when calling a top-level element . I have opened issue http://code.google.com/p/dart/issues/detail?id=1863 for this particular problem.

I'm closing this bug, feel free to reopen it if there are still some problems with the mangling.


Added Fixed label.

copybara-service bot pushed a commit that referenced this issue Jan 3, 2023
…rtdoc, ffi, fixnum, glob, http, http_parser, intl, json_rpc_2, lints, logging, matcher, path, protobuf, sse, string_scanner, test, typed_data, usage, webdev

Revisions updated by `dart tools/rev_sdk_deps.dart`.

args (https://github.com/dart-lang/args/compare/da037ac..ac0e2c8):
  ac0e2c8  2023-01-03  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#229)

async (https://github.com/dart-lang/async/compare/c59c7c5..de1ce93):
  de1ce93  2023-01-03  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#227)

characters (https://github.com/dart-lang/characters/compare/4ffccb8..fff80aa):
  fff80aa  2023-01-03  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#69)

cli_util (https://github.com/dart-lang/cli_util/compare/edcf1c3..5a8e8ee):
  5a8e8ee  2023-01-01  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#72)

collection (https://github.com/dart-lang/collection/compare/cdb11d4..85e987c):
  85e987c  2023-01-03  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#262)

convert (https://github.com/dart-lang/convert/compare/4feeb10..20d136c):
  20d136c  2023-01-03  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#73)

dartdoc (https://github.com/dart-lang/dartdoc/compare/1f42216..ce25524):
  ce255246  2023-01-02  dependabot[bot]  Bump actions/cache from 3.2.1 to 3.2.2 (#3287)
  e4948e64  2022-12-28  Sam Rawlins  Refactor search into a stateful class (#3285)
  83a26b87  2022-12-28  Parker Lougheed  Simplify search.html base-href handling (#3259)
  a622a89a  2022-12-27  dependabot[bot]  Bump actions/cache from 3.0.11 to 3.2.1 (#3281)
  f0c7a56a  2022-12-27  dependabot[bot]  Bump ossf/scorecard-action from 2.1.1 to 2.1.2 (#3282)
  702a9d13  2022-12-19  dependabot[bot]  Bump github/codeql-action from 2.1.36 to 2.1.37 (#3276)
  2bbac76b  2022-12-19  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#3277)
  195dc3da  2022-12-19  dependabot[bot]  Bump ossf/scorecard-action from 2.0.6 to 2.1.1 (#3280)
  4a5ed7e6  2022-12-19  Sam Rawlins  Remove usage of now-removed-in-3.0 elements (#3279)

ffi (https://github.com/dart-lang/ffi/compare/17a8142..2a56c2a):
  2a56c2a  2023-01-02  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#172)

fixnum (https://github.com/dart-lang/fixnum/compare/e4f5e97..714381c):
  714381c  2023-01-03  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#102)

glob (https://github.com/dart-lang/glob/compare/7f97bf5..7adf833):
  7adf833  2023-01-03  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#67)

http (https://github.com/dart-lang/http/compare/46a7708..38d5dd9):
  38d5dd9  2022-12-29  Brian Quinlan  Add usage instructions for runWithClient and Flutter (#846)
  3fba812  2022-12-29  Nate Bosch  Drop avoid_redundant_argument_values (#845)
  88f6fc6  2022-12-14  Brian Quinlan  Always using `package:http` for APIs and image loading. (#839)

http_parser (https://github.com/dart-lang/http_parser/compare/c739675..16a4f34):
  16a4f34  2023-01-03  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#65)

intl (https://github.com/dart-lang/intl/compare/6fb07f2..59e7bff):
  59e7bff  2022-12-20  Copybara-Service  Merge pull request #263 from sanekyy:update-ruble-sign
  50f93df  2022-12-19  Copybara-Service  Merge pull request #467 from samlythemanly:patch-1
  548ef63  2022-12-19  Aleksandr Yurkovskiy  fix duplicate_import
  5dc6e8f  2022-12-16  Aleksandr Yurkovskiy  Merge remote-tracking branch 'upstream/master' into update-ruble-sign
  60e8cfa  2022-12-09  Moritz  Merge branch 'master' into patch-1
  758d086  2022-04-28  Sam Markoe  Updated PEN to use the proper currency code.
  83e149f  2021-03-10  Aleksandr Yurkovskiy  Merge branch 'master' into update-ruble-sign
  edbbbce  2020-11-16  Aleksandr Yurkovskiy  fix format
  f25b0a6  2020-11-16  Aleksandr Yurkovskiy  add empty line
  a02de91  2020-11-16  Aleksandr Yurkovskiy  fix merge issues
  8a1716f  2020-11-16  Aleksandr Yurkovskiy  Merge branch 'master' into update-ruble-sign
  d3c56e0  2020-10-05  Aleksandr Yurkovskiy  update changelog
  ed3b5f0  2020-10-05  Aleksandr Yurkovskiy  fix test
  d4ad715  2020-09-22  Aleksandr Yurkovskiy  Merge branch 'master' into update-ruble-sign
  077e653  2020-02-27  Aleksandr Yurkovskiy  update version
  b119da3  2020-02-27  Aleksandr Yurkovskiy  update ruble sign

json_rpc_2 (https://github.com/dart-lang/json_rpc_2/compare/16fed53..bd9f8d9):
  bd9f8d9  2023-01-03  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#87)

lints (https://github.com/dart-lang/lints/compare/16bdefe..dfded5e):
  dfded5e  2023-01-01  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#96)

logging (https://github.com/dart-lang/logging/compare/0373ef8..b525d5c):
  b525d5c  2023-01-03  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#125)

matcher (https://github.com/dart-lang/matcher/compare/7e6a665..deedda1):
  deedda1  2023-01-03  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#202)

path (https://github.com/dart-lang/path/compare/12ce876..1299791):
  1299791  2023-01-03  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#132)

protobuf (https://github.com/dart-lang/protobuf/compare/2706b53..1d1c92a):
  1d1c92a  2023-01-02  Kevin Moore  Latest github actions (#789)

sse (https://github.com/dart-lang/sse/compare/cfa93b1..2de27fe):
  2de27fe  2022-12-28  dependabot[bot]  Bump nanasess/setup-chromedriver from 1.0.5 to 1.1.0 (#65)

string_scanner (https://github.com/dart-lang/string_scanner/compare/4a5cbc5..6ddab2c):
  6ddab2c  2022-12-28  Kevin Moore  Remove unnecessary parens (#51)
  3e451ab  2022-12-28  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#50)

test (https://github.com/dart-lang/test/compare/89a8b12..8235a25):
  8235a25b  2022-12-29  Nate Bosch  Add String.equalsIgnoringWhitespace condition (#1835)
  6df19d2e  2022-12-29  Nate Bosch  Add String.containsInOrder condition (#1836)
  b7f4cb54  2022-12-29  Nate Bosch  Collapse core checks to fewer extensions (#1831)
  101df768  2022-12-29  Nate Bosch  Add Function.returnsNormally condition (#1833)
  f0dfdbfc  2022-12-28  Nate Bosch  Tighten types of Extracted constructors (#1829)
  d1211748  2022-12-27  Nate Bosch  Add String.equalsIgnoringCase (#1828)
  c14260b5  2022-12-22  Nate Bosch  Add String.equals condition (#1826)
  1785348f  2022-12-21  Nate Bosch  Add more numeric matchers (#1823)
  66d89348  2022-12-21  Nate Bosch  Allow slower node test on windows (#1825)
  695894ee  2022-12-21  Nate Bosch  Drop dependency overrides from pkg:checks (#1824)
  9be6023c  2022-12-20  Nate Bosch  Add Iterable.every extension (#1822)
  ca412bbf  2022-12-20  Nate Bosch  Add some tests with full failure messages (#1820)
  6650bbb3  2022-12-20  Nate Bosch  Add pairwiseCompare condition (#1821)
  d20eb34d  2022-12-20  Nate Bosch  Add FailureDetail to allow expressive failures (#1818)
  611faeac  2022-12-20  Nate Bosch  Make the prefixFirst utility public (#1819)
  a0562585  2022-12-19  Jacob MacDonald  change the url secrets for browser tests to be alphanumeric characters (#1817)

typed_data (https://github.com/dart-lang/typed_data/compare/1e838b8..dbf81a7):
  dbf81a7  2023-01-03  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#57)

usage (https://github.com/dart-lang/usage/compare/fee1d9d..2773c7d):
  2773c7d  2023-01-03  dependabot[bot]  Bump actions/checkout from 3.1.0 to 3.2.0 (#186)

webdev (https://github.com/dart-lang/webdev/compare/317288a..8795ca5):
  8795ca5  2022-12-22  Elliott Brooks (she/her)  Tests should serve`main.dart` and `index.html` from the same directory (#1856)
  b127883  2022-12-22  Elliott Brooks (she/her)  Add test timeout of 2 minutes for all tests missing a timeout (#1854)
  c884705  2022-12-22  Elliott Brooks (she/her)  Skip flaky events_test on Windows (#1853)
  5aa3195  2022-12-22  Elliott Brooks (she/her)  Add tests for interacting with the extension panels added to Chrome Devtools (#1836)
  82ac652  2022-12-21  Elliott Brooks (she/her)  Shard DWDS tests and fix CI test flakiness (#1851)
  8e926c0  2022-12-20  Elliott Brooks (she/her)  Fix flaky CI tests (#1848)
  80f8671  2022-12-20  Elliott Brooks (she/her)  File paths in `TestContext` are relative, not absolute (#1843)
  6dedcd5  2022-12-20  Elliott Brooks (she/her)  Move the `scopes` package (used in testing) to the test `fixtures` directory (#1837)
  db5ed71  2022-12-16  Elliott Brooks (she/her)  Fix `package_uri_file_mapper_test` on Linux (#1835)
  243fe42  2022-12-14  Anna Gringauze  Enable weak tests for build daemon and frontend server (#1824)

Change-Id: I0ef4ec924d14d88e4831c7bcb025ff19db288479
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/278083
Auto-Submit: Devon Carew <devoncarew@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants