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

private member name mangling breaks in unchecked mode #2069

Closed
efortuna opened this issue Mar 9, 2012 · 3 comments
Closed

private member name mangling breaks in unchecked mode #2069

efortuna opened this issue Mar 9, 2012 · 3 comments
Labels
P0 A serious issue requiring immediate resolution

Comments

@efortuna
Copy link
Contributor

efortuna commented Mar 9, 2012

The CL here: https://chromiumcodereview.appspot.com/9621001

unfortunately does not behave quite as expected in unchecked mode. Try running:

tools/test.py -m release -c frogium client/samples/smoketest

The test will fail. We're not seeing this breakage on the buildbots because we're running all of them in checked mode! (I'll change that once this is fixed) If you run the same test with the "--checked" flag, the test will pass.

All of my performance tests on all browsers are broken until the change is fixed. :-(

It looks like in this test, DoubleLinkedQueueEntry has a field called _element, which gets "renamed" to the unique name _dart_coreimpl_element, but the new name doesn't propagate to later references to the field in other methods.

@rakudrama
Copy link
Member

I understand the problem.
Concrete classes and their members are created on demand during codegen.

  1. add corelib DoubleLinkedQueueEntry._element
  2. add otherlib XXX._element, rename to _otherlib_element
    codegen:
  3. generate code using DoubleLinkedQueueEntry._element --- the cat is out of the bag
  4. generate demand for DoubleLinkedQueueEntry<Foo>
  5. add corelib DoubleLinkedQueueEntry<Foo>._element, rename to _dart_corelib_element

The process of renaming incrementally is somewhat unpredictable. Given libA.ClassX._foo and libB.ClassY._foo, one or both could be renamed depending on the order in which the private members are added to the libraries.

I see three ways out of this:

  1. A global flag that disables the renaming during codegen. Hacky.
  2. Rename the current field in all the libraries, not just the current one. This has the disadvantage that no library gets the short version of the name.
  3. Write a small pass to do the renaming just before codegen (or analysis?)

I think #­3 is the best option, and probably not much more code than the current code, just in a different place.
The libraries can be visited in priority order, giving the first library with the name the short version, extending the names only in subsequent libraries with a clash. I would prioritize the main library so we don't mess with the user's main program.
Doing it all in one small pass will also let us prevent clashes between private fields that look too much like renamed fields. There is currently no check that the prefixed name is not also user defined field name.

@rakudrama
Copy link
Member

Fix available: https://chromiumcodereview.appspot.com/9656003

Kasper, I have to go home now. Can you land this or something like it to unblock us?

@kasperl
Copy link

kasperl commented Mar 9, 2012

Thanks, Stephen. Fix landed in r5217.


Added Fixed label.

@efortuna efortuna added Type-Defect P0 A serious issue requiring immediate resolution labels Mar 9, 2012
copybara-service bot pushed a commit that referenced this issue Apr 4, 2023
…ctor, browser_launcher, characters, clock, collection, convert, crypto, csslib, dartdoc, fixnum, glob, html, http, http_multi_server, http_parser, json_rpc_2, logging, matcher, mime, package_config, path, pool, pub_semver, source_maps, source_span, sse, stack_trace, stream_channel, term_glyph, test, test_descriptor, test_process, tools, usage, watcher, web_socket_channel, webdev, yaml, yaml_edit

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

args (https://github.com/dart-lang/args/compare/7a5e3b0..5ac2ba1):
  5ac2ba1  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#238)
  f77b1dc  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#239)

async (https://github.com/dart-lang/async/compare/f454380..0127813):
  0127813  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#236)
  100445b  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#237)

bazel_worker (https://github.com/dart-lang/bazel_worker/compare/53871c5..d5f8837):
  d5f8837  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#70)
  a8a55e6  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#71)

benchmark_harness (https://github.com/dart-lang/benchmark_harness/compare/725534a..e591ec4):
  e591ec4  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#86)
  38bf5b8  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#87)

boolean_selector (https://github.com/dart-lang/boolean_selector/compare/16e6ad3..28dc03d):
  28dc03d  2023-04-04  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#45)

browser_launcher (https://github.com/dart-lang/browser_launcher/compare/bc2dc4e..ba4e028):
  ba4e028  2023-04-04  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#41)

characters (https://github.com/dart-lang/characters/compare/3281cc7..ba8d557):
  ba8d557  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#79)
  60cae68  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#80)

clock (https://github.com/dart-lang/clock/compare/984642e..93d9f56):
  93d9f56  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#48)

collection (https://github.com/dart-lang/collection/compare/30fd0f8..9db854d):
  9db854d  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#278)

convert (https://github.com/dart-lang/convert/compare/83886e3..8812e40):
  8812e40  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#79)
  d28dc33  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#80)

crypto (https://github.com/dart-lang/crypto/compare/9efb888..8a03816):
  8a03816  2023-04-04  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#143)

csslib (https://github.com/dart-lang/csslib/compare/d32bdd4..5836863):
  5836863  2023-04-04  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#177)

dartdoc (https://github.com/dart-lang/dartdoc/compare/9be04e0..1a7952b):
  1a7952b1  2023-04-03  dependabot[bot]  Bump ossf/scorecard-action from 2.1.2 to 2.1.3 (#3382)

fixnum (https://github.com/dart-lang/fixnum/compare/f8379d9..92ec336):
  92ec336  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#108)
  f14fd19  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#109)

glob (https://github.com/dart-lang/glob/compare/f378dc8..eaa878b):
  eaa878b  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#73)
  c0c7e66  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#74)

html (https://github.com/dart-lang/html/compare/08643e9..57b747d):
  57b747d  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#209)
  51c9910  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#210)

http (https://github.com/dart-lang/http/compare/74f9d3d..ffb4438):
  ffb4438  2023-04-04  Brian Quinlan  Fix maxRedirects documentation to mention ClientException rather than RedirectException (#907)
  ad0e1cf  2023-04-03  Bahaa Fathi Yousef  Fix some spelling (#885)

http_multi_server (https://github.com/dart-lang/http_multi_server/compare/7bd190c..e0b5d35):
  e0b5d35  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#53)
  3bbaf22  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#52)

http_parser (https://github.com/dart-lang/http_parser/compare/b3b283b..bbe37dd):
  bbe37dd  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#70)
  f0527a8  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#71)

json_rpc_2 (https://github.com/dart-lang/json_rpc_2/compare/aea3bea..5da2705):
  5da2705  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#94)
  d6ab373  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#95)

logging (https://github.com/dart-lang/logging/compare/abef371..787030a):
  787030a  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#133)
  be6a20e  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#134)

matcher (https://github.com/dart-lang/matcher/compare/61f4347..cb6b68c):
  cb6b68c  2023-04-04  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#216)

mime (https://github.com/dart-lang/mime/compare/1a51be0..2d8496d):
  2d8496d  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#90)
  3b39378  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#91)

package_config (https://github.com/dart-lang/package_config/compare/74ac1cb..7e09db1):
  7e09db1  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#132)
  6dc4072  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#133)

path (https://github.com/dart-lang/path/compare/cd37179..23e3319):
  23e3319  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#140)

pool (https://github.com/dart-lang/pool/compare/338bfb4..650e5d3):
  650e5d3  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#66)

pub_semver (https://github.com/dart-lang/pub_semver/compare/c0e6ea7..860e3d8):
  860e3d8  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#82)
  12eca92  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#83)

source_maps (https://github.com/dart-lang/source_maps/compare/a112e98..0a4b030):
  0a4b030  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#76)
  e753fea  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#75)

source_span (https://github.com/dart-lang/source_span/compare/3951ba5..b739fbf):
  b739fbf  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#94)
  c6547c2  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#95)

sse (https://github.com/dart-lang/sse/compare/8c3efdc..11e83a0):
  11e83a0  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#79)

stack_trace (https://github.com/dart-lang/stack_trace/compare/6ceb191..9c1b1c5):
  9c1b1c5  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#128)
  56a09db  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#129)

stream_channel (https://github.com/dart-lang/stream_channel/compare/fe0f5e4..74646ea):
  74646ea  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#88)

term_glyph (https://github.com/dart-lang/term_glyph/compare/d275a8f..f6856e2):
  f6856e2  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#36)

test (https://github.com/dart-lang/test/compare/a01b185..8ea4298):
  8ea42987  2023-04-04  Jakub Vrána  Make tests compatible with Strict CSP (#1987)
  49f7e17a  2023-04-03  dependabot[bot]  Bump ossf/scorecard-action from 2.1.2 to 2.1.3 (#1982)
  1a4f76b2  2023-04-03  dependabot[bot]  Bump github/codeql-action from 2.2.5 to 2.2.9 (#1985)

test_descriptor (https://github.com/dart-lang/test_descriptor/compare/1d4a967..aa11162):
  aa11162  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#49)
  226fe86  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#50)

test_process (https://github.com/dart-lang/test_process/compare/f76d0b8..946bc27):
  946bc27  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#40)
  441f585  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#41)

tools (https://github.com/dart-lang/tools/compare/d40ca93..0304fbb):
  0304fbb  2023-04-04  Elias Yishak  Add catcherror callback for `sendData` (#72)
  6d1dedf  2023-04-04  Daco Harkes  [cli_config] Pub badges (#71)
  561dce2  2023-04-04  Daco Harkes  [cli_config] Bump version (#68)
  d3909a4  2023-04-04  Daco Harkes  Fix windows path resolving (#67)
  77cf078  2023-04-03  Daco Harkes  [cli_config] Fix optionalString validValues (#69)

usage (https://github.com/dart-lang/usage/compare/399770f..0698711):
  0698711  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#190)
  2cdb5e3  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#189)

watcher (https://github.com/dart-lang/watcher/compare/5968409..00aa79b):
  00aa79b  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#140)
  598038f  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#141)

web_socket_channel (https://github.com/dart-lang/web_socket_channel/compare/e2fe7f6..40eb236):
  40eb236  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#260)
  1823444  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#261)

webdev (https://github.com/dart-lang/webdev/compare/b139649..e887316):
  e887316c  2023-04-03  Elliott Brooks  Prepare DWDS for version `19.0.0` release (#2068)
  704d5086  2023-04-03  Elliott Brooks  Fix typo (#2069)
  2e6e1b63  2023-04-03  Anna Gringauze  Fix getObject failure on record class. (#2063)

yaml (https://github.com/dart-lang/yaml/compare/0f80b12..56dfaf4):
  56dfaf4  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#140)
  d925d7e  2023-04-03  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.0 (#141)

yaml_edit (https://github.com/dart-lang/yaml_edit/compare/fbc5cb3..386fd33):
  386fd33  2023-04-03  dependabot[bot]  Bump dart-lang/setup-dart from 1.4.0 to 1.5.0 (#49)

Change-Id: I986c83f657631813a32e360fbb90f42f7d43440a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/293280
Auto-Submit: Devon Carew <devoncarew@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
copybara-service bot pushed a commit that referenced this issue Aug 7, 2023
…st, tools

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

ecosystem (https://github.com/dart-lang/ecosystem/compare/97fc1a7..dfeda1a):
  dfeda1a  2023-08-01  dependabot[bot]  Bump actions/labeler from 4.0.4 to 4.3.0 (#148)
  c341051  2023-08-01  dependabot[bot]  Bump coverallsapp/github-action from 2.2.0 to 2.2.1 (#147)
  c1c8d1f  2023-08-01  Moritz  Update health.yaml (#146)
  31c5d21  2023-07-27  Devon Carew  misc updates to the label management tool (#145)

http (https://github.com/dart-lang/http/compare/4289e8b..7e9ed12):
  7e9ed12  2023-08-01  dependabot[bot]  Bump actions/labeler from 4.2.0 to 4.3.0 (#1000)
  e8e35db  2023-07-31  Alex James  [java_http] send request body (#995)

mockito (https://github.com/dart-lang/mockito/compare/b421775..ff79de6):
  ff79de6  2023-08-04  Nate Bosch  Allow the latest package:analyzer

native (https://github.com/dart-lang/native/compare/f0dc3e9..0187d0e):
  0187d0e  2023-08-07  Daco Harkes  [native_assets_cli] Rename `Asset` `name` to `id` (#113)
  1b984c7  2023-08-07  Daco Harkes  [native_assets_cli] Replace `TypeError`s with `FormatException`s (#112)
  b2b26db  2023-08-07  Daco Harkes  [native_assets_builder] Stop throwing from BuildRunner (#108)
  c940ac8  2023-08-03  Daco Harkes  [native_assets_builder] return build dependencies (#107)
  63daab8  2023-08-03  Daco Harkes  [native_assets_builder] Fix dry run directory structure (#110)
  3f26f20  2023-08-01  dependabot[bot]  Bump coverallsapp/github-action from 2.2.0 to 2.2.1 (#104)

protobuf (https://github.com/dart-lang/protobuf/compare/d9e8a31..217c030):
  217c030  2023-08-02  Ömer Sinan Ağacan  Avoid holding onto the buffer when parsing unknown length-delimited fields (#863)

shelf (https://github.com/dart-lang/shelf/compare/bd59ead..73edd2b):
  73edd2b  2023-08-01  dependabot[bot]  Bump actions/labeler from 4.2.0 to 4.3.0 (#370)

sse (https://github.com/dart-lang/sse/compare/e241085..8cc5b11):
  8cc5b11  2023-08-01  dependabot[bot]  Bump nanasess/setup-chromedriver from 2.0.0 to 2.1.1 (#86)

test (https://github.com/dart-lang/test/compare/92eb0f7..5d571d6):
  5d571d64  2023-08-03  Nate Bosch  Add --fail-fast flag (#2040)
  a9dcce29  2023-08-01  Nate Bosch  Rerun publish workflow on more PR changes (#2070)
  50d558b2  2023-08-01  dependabot[bot]  Bump github/codeql-action from 2.20.1 to 2.21.2 (#2071)
  9e124e9f  2023-07-31  Nate Bosch  Prepare to publish (#2069)

tools (https://github.com/dart-lang/tools/compare/af3fc99..f14bf2e):
  f14bf2e  2023-08-07  Elias Yishak  Remove unused NoOp classes (#138)
  a2aa1c3  2023-08-07  Devon Carew  add CI; update readme (#140)
  d424568  2023-08-06  Devon Carew  add usage docs to package:cli_config (#141)
  921611a  2023-08-03  Elias Yishak  Survey handler feature (#109)

Change-Id: Iee720272733822d11caf46adcae516f12f11abec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/318801
Commit-Queue: Devon Carew <devoncarew@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Auto-Submit: Devon Carew <devoncarew@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
P0 A serious issue requiring immediate resolution
Projects
None yet
Development

No branches or pull requests

3 participants