Commit b9b758c
committed
[MERGE #5456 @sethbrenith] nicer dynamic casts
Merge pull request #5456 from sethbrenith:user/sethb/is
I recently came across the following code:
```C++
// JsParseSerialized only accepts ArrayBuffer (incl. ExternalArrayBuffer)
if (!Js::ExternalArrayBuffer::Is(bufferVal))
{
return JsErrorInvalidArgument;
}
```
I thought the comment was out of date, and was about to update it, when I realized that `ExternalArrayBuffer::Is` actually invokes `ArrayBuffer::Is`, because static methods are inherited and `ExternalArrayBuffer` doesn't provide an `Is` method. I don't want to live in a world where `ExternalArrayBuffer::Is(bufferVal)` can return something other than whether `bufferVal` is an `ExternalArrayBuffer`, so this change is my proposed solution. It introduces a new template method `VarIs` (in RecyclableObject.h) and uses it consistently for all type-checks and conversions of `RecyclableObject` subclasses. Benefits are:
* Avoid the confusing case above (it would be a linker error)
* Less boilerplate code (this is a net removal of about 1500 lines)
* Every type gets by default an optimization for when the compiler knows the input is `RecyclableObject*` (previously only a few types implemented this)
Most of the change is purely mechanical, and anybody who's willing to review it is a hero. However, a few cases are interesting:
* `DynamicObject`, `JavascriptArray`, and `JavascriptGeneratorFunction` had asymmetrical behavior between `Is` and `(Unsafe)?FromVar`. I have attempted to avoid any behavior changes in this review by updating callers to `Is` to use a new uniquely-named method, and making `VarIs` respect the behavior from `(Unsafe)?FromVar`.
* A few calls have been updated to refer to the thing they were actually calling, not some subclass:
* `JavascriptObject` -> `DynamicObject`
* `ExternalArrayBuffer` -> `ArrayBuffer`
* `JavascriptArrayBuffer` -> `ArrayBuffer`
* `RuntimeFunction` -> `JavascriptFunction`
<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/microsoft/chakracore/5456)
<!-- Reviewable:end -->File tree
226 files changed
+3200
-4654
lines changed- lib
- Backend
- Common/Exceptions
- Jsrt
- Core
- Runtime
- Base
- ByteCode
- Debug
- Language
- i386
- Library
- Math
- Types
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
226 files changed
+3200
-4654
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
142 | 142 | | |
143 | 143 | | |
144 | 144 | | |
145 | | - | |
| 145 | + | |
146 | 146 | | |
147 | 147 | | |
148 | | - | |
| 148 | + | |
149 | 149 | | |
150 | 150 | | |
151 | 151 | | |
| |||
160 | 160 | | |
161 | 161 | | |
162 | 162 | | |
163 | | - | |
| 163 | + | |
164 | 164 | | |
165 | 165 | | |
166 | 166 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
576 | 576 | | |
577 | 577 | | |
578 | 578 | | |
579 | | - | |
| 579 | + | |
580 | 580 | | |
581 | 581 | | |
582 | | - | |
| 582 | + | |
583 | 583 | | |
584 | 584 | | |
585 | 585 | | |
| |||
656 | 656 | | |
657 | 657 | | |
658 | 658 | | |
659 | | - | |
| 659 | + | |
660 | 660 | | |
661 | 661 | | |
662 | 662 | | |
| |||
1004 | 1004 | | |
1005 | 1005 | | |
1006 | 1006 | | |
1007 | | - | |
| 1007 | + | |
1008 | 1008 | | |
1009 | 1009 | | |
1010 | 1010 | | |
| |||
1031 | 1031 | | |
1032 | 1032 | | |
1033 | 1033 | | |
1034 | | - | |
| 1034 | + | |
1035 | 1035 | | |
1036 | 1036 | | |
1037 | 1037 | | |
| |||
1060 | 1060 | | |
1061 | 1061 | | |
1062 | 1062 | | |
1063 | | - | |
| 1063 | + | |
1064 | 1064 | | |
1065 | 1065 | | |
1066 | 1066 | | |
| |||
1076 | 1076 | | |
1077 | 1077 | | |
1078 | 1078 | | |
1079 | | - | |
| 1079 | + | |
1080 | 1080 | | |
1081 | 1081 | | |
1082 | 1082 | | |
| |||
1106 | 1106 | | |
1107 | 1107 | | |
1108 | 1108 | | |
1109 | | - | |
| 1109 | + | |
1110 | 1110 | | |
1111 | 1111 | | |
1112 | 1112 | | |
| |||
1118 | 1118 | | |
1119 | 1119 | | |
1120 | 1120 | | |
1121 | | - | |
| 1121 | + | |
1122 | 1122 | | |
1123 | 1123 | | |
1124 | 1124 | | |
| |||
1162 | 1162 | | |
1163 | 1163 | | |
1164 | 1164 | | |
1165 | | - | |
| 1165 | + | |
1166 | 1166 | | |
1167 | 1167 | | |
1168 | 1168 | | |
| |||
1381 | 1381 | | |
1382 | 1382 | | |
1383 | 1383 | | |
1384 | | - | |
| 1384 | + | |
1385 | 1385 | | |
1386 | 1386 | | |
1387 | 1387 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
17 | | - | |
| 17 | + | |
18 | 18 | | |
19 | | - | |
| 19 | + | |
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
24 | | - | |
| 24 | + | |
25 | 25 | | |
26 | | - | |
| 26 | + | |
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
400 | 400 | | |
401 | 401 | | |
402 | 402 | | |
403 | | - | |
| 403 | + | |
404 | 404 | | |
405 | 405 | | |
406 | 406 | | |
| |||
493 | 493 | | |
494 | 494 | | |
495 | 495 | | |
496 | | - | |
| 496 | + | |
497 | 497 | | |
498 | 498 | | |
499 | 499 | | |
| |||
1284 | 1284 | | |
1285 | 1285 | | |
1286 | 1286 | | |
1287 | | - | |
1288 | | - | |
| 1287 | + | |
| 1288 | + | |
1289 | 1289 | | |
1290 | 1290 | | |
1291 | 1291 | | |
| |||
1473 | 1473 | | |
1474 | 1474 | | |
1475 | 1475 | | |
1476 | | - | |
| 1476 | + | |
1477 | 1477 | | |
1478 | 1478 | | |
1479 | 1479 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2549 | 2549 | | |
2550 | 2550 | | |
2551 | 2551 | | |
2552 | | - | |
| 2552 | + | |
2553 | 2553 | | |
2554 | 2554 | | |
2555 | 2555 | | |
| |||
2725 | 2725 | | |
2726 | 2726 | | |
2727 | 2727 | | |
2728 | | - | |
| 2728 | + | |
2729 | 2729 | | |
2730 | 2730 | | |
2731 | 2731 | | |
| |||
2898 | 2898 | | |
2899 | 2899 | | |
2900 | 2900 | | |
2901 | | - | |
| 2901 | + | |
2902 | 2902 | | |
2903 | 2903 | | |
2904 | 2904 | | |
| |||
2955 | 2955 | | |
2956 | 2956 | | |
2957 | 2957 | | |
2958 | | - | |
| 2958 | + | |
2959 | 2959 | | |
2960 | 2960 | | |
2961 | 2961 | | |
| |||
3023 | 3023 | | |
3024 | 3024 | | |
3025 | 3025 | | |
3026 | | - | |
| 3026 | + | |
3027 | 3027 | | |
3028 | 3028 | | |
3029 | 3029 | | |
| |||
4067 | 4067 | | |
4068 | 4068 | | |
4069 | 4069 | | |
4070 | | - | |
| 4070 | + | |
4071 | 4071 | | |
4072 | 4072 | | |
4073 | 4073 | | |
| |||
4491 | 4491 | | |
4492 | 4492 | | |
4493 | 4493 | | |
4494 | | - | |
| 4494 | + | |
4495 | 4495 | | |
4496 | 4496 | | |
4497 | 4497 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
211 | 211 | | |
212 | 212 | | |
213 | 213 | | |
214 | | - | |
| 214 | + | |
215 | 215 | | |
216 | | - | |
| 216 | + | |
217 | 217 | | |
218 | 218 | | |
219 | 219 | | |
| |||
230 | 230 | | |
231 | 231 | | |
232 | 232 | | |
233 | | - | |
| 233 | + | |
234 | 234 | | |
235 | 235 | | |
236 | 236 | | |
| |||
0 commit comments