Skip to content

Commit 87c16ba

Browse files
MikeHolmanMikeHolman
committed
fix symbols; fix canonicalization of int64, -0
1 parent 24d0906 commit 87c16ba

File tree

7 files changed

+114
-65
lines changed

7 files changed

+114
-65
lines changed

‎lib/Runtime/Language/JavascriptConversion.cpp

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1522,36 +1522,4 @@ namespace Js
15221522
return NumberUtilities::TryToInt64(length);
15231523
}
15241524

1525-
Var JavascriptConversion::TryCanonicalizeAsSimpleVar(Var value)
1526-
{
1527-
TypeId typeId = JavascriptOperators::GetTypeId(value);
1528-
switch (typeId)
1529-
{
1530-
case TypeIds_Integer:
1531-
case TypeIds_Number:
1532-
{
1533-
Var taggedInt = TryCanonicalizeAsTaggedInt<true>(value);
1534-
if (taggedInt)
1535-
{
1536-
return taggedInt;
1537-
}
1538-
1539-
#if FLOATVAR
1540-
return value;
1541-
#else
1542-
return nullptr;
1543-
#endif
1544-
}
1545-
case TypeIds_String:
1546-
case TypeIds_Symbol:
1547-
// TODO: try to canonicalize Int64/Uint64 to TaggedInt
1548-
case TypeIds_Int64Number:
1549-
case TypeIds_UInt64Number:
1550-
return nullptr;
1551-
1552-
default:
1553-
return value;
1554-
}
1555-
}
1556-
15571525
} // namespace Js

‎lib/Runtime/Language/JavascriptConversion.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,17 @@ namespace Js {
9292
static double LongToDouble(__int64 aValue);
9393
static double ULongToDouble(unsigned __int64 aValue);
9494

95-
template <bool allowNegOne>
95+
template <bool allowNegOne, bool acceptLossyConversion>
9696
static Var TryCanonicalizeAsTaggedInt(Var value);
97+
template <bool allowNegOne, bool acceptLossyConversion>
98+
static Var TryCanonicalizeAsTaggedInt(Var value, TypeId typeId);
99+
template <bool acceptLossyConversion>
97100
static Var TryCanonicalizeAsSimpleVar(Var value);
98101

99102
private:
103+
template <typename T, bool allowNegOne>
104+
static Var TryCanonicalizeIntHelper(T val);
105+
100106
static BOOL ToInt32Finite(double value, int32* result);
101107
template<bool zero>
102108
static bool SameValueCommon(Var aValue, Var bValue);

‎lib/Runtime/Language/JavascriptConversion.inl

Lines changed: 83 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -203,40 +203,109 @@ namespace Js {
203203
return SameValueCommon<true>(aValue, bValue);
204204
}
205205

206-
template <bool allowNegOne>
207-
inline Var JavascriptConversion::TryCanonicalizeAsTaggedInt(Var value)
206+
template <typename T, bool allowNegOne>
207+
static Var JavascriptConversion::TryCanonicalizeIntHelper(T val)
208208
{
209-
if (TaggedInt::Is(value))
209+
if (TaggedInt::IsOverflow(val))
210210
{
211-
return (allowNegOne || value != TaggedInt::ToVarUnchecked(-1))
212-
? value
213-
: nullptr;
211+
return nullptr;
214212
}
215213

216-
if (!JavascriptNumber::Is(value))
214+
if (!allowNegOne && val == -1)
217215
{
218216
return nullptr;
219217
}
220218

221-
double doubleVal = JavascriptNumber::GetValue(value);
222-
int32 intVal = 0;
219+
return TaggedInt::ToVarUnchecked((int)val);
220+
}
223221

224-
if (!JavascriptNumber::TryGetInt32Value<true>(doubleVal, &intVal))
222+
template <bool allowNegOne, bool allowLossyConversion>
223+
inline Var JavascriptConversion::TryCanonicalizeAsTaggedInt(Var value, TypeId typeId)
224+
{
225+
switch (typeId)
225226
{
226-
return nullptr;
227+
case TypeIds_Integer:
228+
return (allowNegOne || value != TaggedInt::ToVarUnchecked(-1))
229+
? value
230+
: nullptr;
231+
232+
case TypeIds_Number:
233+
{
234+
double doubleVal = JavascriptNumber::GetValue(value);
235+
int32 intVal = 0;
236+
237+
if (!JavascriptNumber::TryGetInt32Value<allowLossyConversion>(doubleVal, &intVal))
238+
{
239+
return nullptr;
240+
}
241+
return TryCanonicalizeIntHelper<int32, allowNegOne>(intVal);
227242
}
243+
case TypeIds_Int64Number:
244+
{
245+
if (!allowLossyConversion)
246+
{
247+
return nullptr;
248+
}
249+
int64 int64Val = JavascriptInt64Number::UnsafeFromVar(value)->GetValue();
250+
251+
return TryCanonicalizeIntHelper<int64, allowNegOne>(int64Val);
228252

229-
if (TaggedInt::IsOverflow(intVal))
253+
}
254+
case TypeIds_UInt64Number:
230255
{
256+
if (!allowLossyConversion)
257+
{
258+
return nullptr;
259+
}
260+
uint64 uint64Val = JavascriptUInt64Number::UnsafeFromVar(value)->GetValue();
261+
262+
return TryCanonicalizeIntHelper<uint64, allowNegOne>(uint64Val);
263+
}
264+
default:
231265
return nullptr;
232266
}
267+
}
268+
269+
template <bool allowNegOne, bool allowLossyConversion>
270+
inline Var JavascriptConversion::TryCanonicalizeAsTaggedInt(Var value)
271+
{
272+
TypeId typeId = JavascriptOperators::GetTypeId(value);
273+
return TryCanonicalizeAsTaggedInt<allowNegOne, allowLossyConversion>(value, typeId);
274+
}
233275

234-
if (!allowNegOne && intVal == -1)
276+
// Lossy conversion means values are StrictEqual equivalent,
277+
// but we cannot reconstruct the original value after canonicalization
278+
// (e.g. -0 or an Int64Number object)
279+
template <bool acceptLossyConversion>
280+
Var JavascriptConversion::TryCanonicalizeAsSimpleVar(Var value)
281+
{
282+
TypeId typeId = JavascriptOperators::GetTypeId(value);
283+
switch (typeId)
284+
{
285+
case TypeIds_Integer:
286+
case TypeIds_Number:
287+
case TypeIds_Int64Number:
288+
case TypeIds_UInt64Number:
235289
{
290+
Var taggedInt = TryCanonicalizeAsTaggedInt<true, acceptLossyConversion>(value, typeId);
291+
if (taggedInt)
292+
{
293+
return taggedInt;
294+
}
295+
296+
#if FLOATVAR
297+
return value;
298+
#else
236299
return nullptr;
300+
#endif
237301
}
302+
case TypeIds_String:
303+
case TypeIds_Symbol:
304+
return nullptr;
238305

239-
return TaggedInt::ToVarUnchecked(intVal);
306+
default:
307+
return value;
308+
}
240309
}
241310

242311
} // namespace Js

‎lib/Runtime/Library/JavascriptMap.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -378,13 +378,13 @@ JavascriptMap::DeleteFromSimpleVarMap(Var value)
378378
{
379379
Assert(this->kind == MapKind::SimpleVarMap);
380380

381-
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar(value);
381+
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar<true /* allowLossyConversion */>(value);
382382
if (!simpleVar)
383383
{
384384
return false;
385385
}
386386

387-
return this->DeleteFromVarMap<false>(simpleVar);
387+
return this->DeleteFromVarMap<false /* isComplex */>(simpleVar);
388388
}
389389

390390
bool
@@ -427,7 +427,7 @@ JavascriptMap::Get(Var key, Var* value)
427427
return true;
428428
}
429429
// If the key isn't in the map, check if the canonical value is
430-
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar(key);
430+
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar<true /* allowLossyConversion */>(key);
431431
// If the simple var is the same as the original key, we know it isn't in the map
432432
if (!simpleVar || simpleVar == key)
433433
{
@@ -476,7 +476,7 @@ JavascriptMap::Has(Var key)
476476
return true;
477477
}
478478
// If the key isn't in the map, check if the canonical value is
479-
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar(key);
479+
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar<true /* allowLossyConversion */>(key);
480480
// If the simple var is the same as the original key, we know it isn't in the map
481481
if (!simpleVar || simpleVar == key)
482482
{
@@ -517,7 +517,7 @@ void
517517
JavascriptMap::SetOnEmptyMap(Var key, Var value)
518518
{
519519
Assert(this->kind == MapKind::EmptyMap);
520-
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar(key);
520+
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar<false /* allowLossyConversion */>(key);
521521
if (simpleVar)
522522
{
523523
SimpleVarDataMap* newSimpleMap = RecyclerNew(this->GetRecycler(), SimpleVarDataMap, this->GetRecycler());
@@ -548,7 +548,7 @@ JavascriptMap::TrySetOnSimpleVarMap(Var key, Var value)
548548
{
549549
Assert(this->kind == MapKind::SimpleVarMap);
550550

551-
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar(key);
551+
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar<false /* allowLossyConversion */>(key);
552552
if (!simpleVar)
553553
{
554554
return false;

‎lib/Runtime/Library/JavascriptMap.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ namespace Js
2121
// An EmptyMap is a map containing no elements
2222
EmptyMap,
2323
// A SimpleVarMap is a map containing only Vars which are comparable by pointer, and don't require
24-
// pointer comparison
24+
// value comparison
2525
//
2626
// Addition of a Var that is not comparable by pointer value causes the set to be promoted to a ComplexVarSet
2727
SimpleVarMap,

‎lib/Runtime/Library/JavascriptSet.cpp

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ JavascriptSet::AddToEmptySet(Var value)
328328
{
329329
Assert(this->kind == SetKind::EmptySet);
330330
// We cannot store an int with value -1 inside of a bit vector
331-
Var taggedInt = JavascriptConversion::TryCanonicalizeAsTaggedInt<false>(value);
331+
Var taggedInt = JavascriptConversion::TryCanonicalizeAsTaggedInt<false /* allowNegOne */, false /* allowLossyConversion */>(value);
332332
if (taggedInt)
333333
{
334334
int32 intVal = TaggedInt::ToInt32(taggedInt);
@@ -343,7 +343,7 @@ JavascriptSet::AddToEmptySet(Var value)
343343
return;
344344
}
345345

346-
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar(value);
346+
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar<false /* allowLossyConversion */>(value);
347347
if (simpleVar)
348348
{
349349
SimpleVarDataSet* newSimpleSet = RecyclerNew(this->GetRecycler(), SimpleVarDataSet, this->GetRecycler());
@@ -369,7 +369,7 @@ bool
369369
JavascriptSet::TryAddToIntSet(Var value)
370370
{
371371
Assert(this->kind == SetKind::IntSet);
372-
Var taggedInt = JavascriptConversion::TryCanonicalizeAsTaggedInt<false>(value);
372+
Var taggedInt = JavascriptConversion::TryCanonicalizeAsTaggedInt<false /* allowNegOne */, false /* allowLossyConversion */>(value);
373373
if (!taggedInt)
374374
{
375375
return false;
@@ -387,7 +387,7 @@ bool
387387
JavascriptSet::TryAddToSimpleVarSet(Var value)
388388
{
389389
Assert(this->kind == SetKind::SimpleVarSet);
390-
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar(value);
390+
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar<false /* allowLossyConversion */>(value);
391391
if (!simpleVar)
392392
{
393393
return false;
@@ -432,7 +432,7 @@ JavascriptSet::Add(Var value)
432432
return;
433433
}
434434

435-
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar(value);
435+
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar<false /* allowLossyConversion */>(value);
436436
if (simpleVar)
437437
{
438438
this->PromoteToSimpleVarSet();
@@ -490,7 +490,7 @@ bool
490490
JavascriptSet::IsInIntSet(Var value)
491491
{
492492
Assert(this->kind == SetKind::IntSet);
493-
Var taggedInt = JavascriptConversion::TryCanonicalizeAsTaggedInt<false>(value);
493+
Var taggedInt = JavascriptConversion::TryCanonicalizeAsTaggedInt<false /* allowNegOne */, true /* allowLossyConversion */>(value);
494494
if (!taggedInt)
495495
{
496496
return false;
@@ -520,7 +520,7 @@ bool
520520
JavascriptSet::DeleteFromSimpleVarSet(Var value)
521521
{
522522
Assert(this->kind == SetKind::SimpleVarSet);
523-
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar(value);
523+
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar<true /* allowLossyConversion */>(value);
524524
if (!simpleVar)
525525
{
526526
return false;
@@ -553,7 +553,7 @@ bool JavascriptSet::Delete(Var value)
553553
return this->DeleteFromSimpleVarSet(value);
554554

555555
case SetKind::ComplexVarSet:
556-
return this->DeleteFromVarSet<true>(value);
556+
return this->DeleteFromVarSet<true /* isComplex */>(value);
557557

558558
default:
559559
Assume(UNREACHED);
@@ -571,7 +571,7 @@ bool JavascriptSet::Has(Var value)
571571

572572
case SetKind::IntSet:
573573
{
574-
Var taggedInt = JavascriptConversion::TryCanonicalizeAsTaggedInt<false>(value);
574+
Var taggedInt = JavascriptConversion::TryCanonicalizeAsTaggedInt<false /* allowNegOne */, true /* allowLossyConversion */ >(value);
575575
if (!taggedInt)
576576
{
577577
return false;
@@ -588,7 +588,7 @@ bool JavascriptSet::Has(Var value)
588588
return true;
589589
}
590590
// If the value isn't in the set, check if the canonical value is
591-
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar(value);
591+
Var simpleVar = JavascriptConversion::TryCanonicalizeAsSimpleVar<true /* allowLossyConversion */>(value);
592592
// If the simple value is the same as the original value, we know it isn't in the set
593593
if (!simpleVar || simpleVar == value)
594594
{

‎lib/Runtime/Library/SameValueComparer.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,16 @@ namespace Js
8585

8686
case TypeIds_String:
8787
{
88-
JavascriptString* v = JavascriptString::FromVar(i);
88+
JavascriptString* v = JavascriptString::UnsafeFromVar(i);
8989
return JsUtil::CharacterBuffer<WCHAR>::StaticGetHashCode(v->GetString(), v->GetLength());
9090
}
9191

92+
case TypeIds_Symbol:
93+
{
94+
JavascriptSymbol* sym = JavascriptSymbol::UnsafeFromVar(i);
95+
return sym->GetValue()->GetHashCode();
96+
}
97+
9298
default:
9399
return RecyclerPointerComparer<Var>::GetHashCode(i);
94100
}

0 commit comments

Comments
 (0)