Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
16286002:
Simplyfy the NumTypeArguments check. (Closed)
|
Created:
7 years, 6 months ago by siva Modified:
7 years, 6 months ago CC:
reviews_dartlang.org Visibility:
Public. |
DescriptionSimplyfy the NumTypeArguments check.
R=regis@google.com
Committed: https://code.google.com/p/dart/source/detail?r=23509
Patch Set 1 #
Total comments: 5
Messages
Total messages: 8 (0 generated)
LGTM
Message was sent while issue was closed.
Committed patchset #1 manually as r23509 (presubmit successful).
Message was sent while issue was closed.
DBC -ip https://codereview.chromium.org/16286002/diff/1/runtime/vm/object.cc File runtime/vm/object.cc (right): https://codereview.chromium.org/16286002/diff/1/runtime/vm/object.cc#newcode1604 runtime/vm/object.cc:1604: // Object is its own super class during bootstrap. Why does Object have to be its own super class during bootstrap? The check would be even simpler if we did not have that mode change.
Message was sent while issue was closed.
https://codereview.chromium.org/16286002/diff/1/runtime/vm/object.cc File runtime/vm/object.cc (right): https://codereview.chromium.org/16286002/diff/1/runtime/vm/object.cc#newcode1604 runtime/vm/object.cc:1604: // Object is its own super class during bootstrap. Not sure, I will have to wait for Regis to answer this question. I looked at Object::Init and don't see super_type being set to its own super class. On 2013/06/01 01:10:34, Ivan Posva wrote: > Why does Object have to be its own super class during bootstrap? The check would > be even simpler if we did not have that mode change.
Message was sent while issue was closed.
https://codereview.chromium.org/16286002/diff/1/runtime/vm/object.cc File runtime/vm/object.cc (right): https://codereview.chromium.org/16286002/diff/1/runtime/vm/object.cc#newcode1604 runtime/vm/object.cc:1604: // Object is its own super class during bootstrap. I do not think we have to set the super_type of class Object to type Object. The parser does it when parsing "class Object", because no extend clause is specified. On 2013/06/03 17:43:38, siva wrote: > Not sure, I will have to wait for Regis to answer this question. > I looked at Object::Init and don't see super_type being set to > its own super class. > > > On 2013/06/01 01:10:34, Ivan Posva wrote: > > Why does Object have to be its own super class during bootstrap? The check > would > > be even simpler if we did not have that mode change. >
Message was sent while issue was closed.
https://codereview.chromium.org/16286002/diff/1/runtime/vm/object.cc File runtime/vm/object.cc (right): https://codereview.chromium.org/16286002/diff/1/runtime/vm/object.cc#newcode1604 runtime/vm/object.cc:1604: // Object is its own super class during bootstrap. If I remove the setting in parser.cc during parsing we get a failure during class finalization: #0 0xf7fdb430 in __kernel_vsyscall () #1 0xf7ce61df in raise () from /lib/i386-linux-gnu/libc.so.6 #2 0xf7ce9825 in abort () from /lib/i386-linux-gnu/libc.so.6 #3 0x080822fa in dart::DynamicAssertionHelper::Fail (this=0xffffbe24, format=0x83fc515 "%s") at runtime/platform/assert.cc:40 #4 0x081e1a26 in dart::AbstractType::IsResolved (this=0x86c1620) at runtime/vm/object.cc:9325 #5 0x08090a37 in dart::ClassFinalizer::ResolveType (cls=..., type=..., finalization=dart::ClassFinalizer::kCanonicalizeWellFormed) at runtime/vm/class_finalizer.cc:419 #6 0x08098579 in dart::ClassFinalizer::ResolveSuperTypeAndInterfaces (cls=..., visited=0xffffc0b0) at runtime/vm/class_finalizer.cc:1711 #7 0x08099365 in dart::ClassFinalizer::FinalizePendingClasses () at runtime/vm/class_finalizer.cc:147 On 2013/06/10 15:19:35, regis wrote: > I do not think we have to set the super_type of class Object to type Object. The > parser does it when parsing "class Object", because no extend clause is > specified. > > On 2013/06/03 17:43:38, siva wrote: > > Not sure, I will have to wait for Regis to answer this question. > > I looked at Object::Init and don't see super_type being set to > > its own super class. > > > > > > On 2013/06/01 01:10:34, Ivan Posva wrote: > > > Why does Object have to be its own super class during bootstrap? The check > > would > > > be even simpler if we did not have that mode change. > > >
Message was sent while issue was closed.
https://codereview.chromium.org/16286002/diff/1/runtime/vm/object.cc File runtime/vm/object.cc (right): https://codereview.chromium.org/16286002/diff/1/runtime/vm/object.cc#newcode1604 runtime/vm/object.cc:1604: // Object is its own super class during bootstrap. Sorry, I was not clear. Classes that do not specify an extend clause must have their super type set to Object type. We cannot remove the setting code from parser.cc. Now, class Object is a special case, because it has no super type. Leaving the default Object type as its super type would create a cycle. This is why object.cc removes the cycle after class Object has been parsed. There is no need to change anything in parser.cc or object.cc. But if you think it is cleaner, we can move the special handling from object.cc to parser.cc. I'll send a cl and you can decide if you like it better. On 2013/06/11 00:34:02, siva wrote: > If I remove the setting in parser.cc during parsing we get a failure during > class finalization: > #0 0xf7fdb430 in __kernel_vsyscall () > #1 0xf7ce61df in raise () from /lib/i386-linux-gnu/libc.so.6 > #2 0xf7ce9825 in abort () from /lib/i386-linux-gnu/libc.so.6 > #3 0x080822fa in dart::DynamicAssertionHelper::Fail (this=0xffffbe24, > format=0x83fc515 "%s") at runtime/platform/assert.cc:40 > #4 0x081e1a26 in dart::AbstractType::IsResolved (this=0x86c1620) > at runtime/vm/object.cc:9325 > #5 0x08090a37 in dart::ClassFinalizer::ResolveType (cls=..., type=..., > finalization=dart::ClassFinalizer::kCanonicalizeWellFormed) > at runtime/vm/class_finalizer.cc:419 > #6 0x08098579 in dart::ClassFinalizer::ResolveSuperTypeAndInterfaces (cls=..., > visited=0xffffc0b0) at runtime/vm/class_finalizer.cc:1711 > #7 0x08099365 in dart::ClassFinalizer::FinalizePendingClasses () > at runtime/vm/class_finalizer.cc:147 > > > > > > On 2013/06/10 15:19:35, regis wrote: > > I do not think we have to set the super_type of class Object to type Object. > The > > parser does it when parsing "class Object", because no extend clause is > > specified. > > > > On 2013/06/03 17:43:38, siva wrote: > > > Not sure, I will have to wait for Regis to answer this question. > > > I looked at Object::Init and don't see super_type being set to > > > its own super class. > > > > > > > > > On 2013/06/01 01:10:34, Ivan Posva wrote: > > > > Why does Object have to be its own super class during bootstrap? The check > > > would > > > > be even simpler if we did not have that mode change. > > > > > > |
||||||||||||||||||||||||||||||||||||||||||||||||||
