Add support for logical assignment operator#6663
Add support for logical assignment operator#6663Kingwl wants to merge 1 commit intochakra-core:masterfrom
Conversation
| FLAGR (Boolean, Intl , "Intl object support", DEFAULT_CONFIG_Intl) | ||
| FLAGNR(Boolean, IntlBuiltIns , "Intl built-in function support", DEFAULT_CONFIG_IntlBuiltIns) | ||
| FLAGNR(Boolean, IntlPlatform , "Make the internal Intl native helper object visible to user code", DEFAULT_CONFIG_IntlPlatform) | ||
| FLAGR(Boolean, Intl, "Intl object support", DEFAULT_CONFIG_Intl) |
There was a problem hiding this comment.
Please can you revert these whitespace changes; the large gaps are intentional; the idea is to show the descriptions of related fields in line with each other - it's not been done perfectly but would rather keep it.
| else if (this->PeekFirst(p, last) == '?' && this->PeekFirst(p + 1, last) == '=') | ||
| { | ||
| p += 2; | ||
| token = tkAsgLogCoalesce; | ||
| } |
There was a problem hiding this comment.
Is this case only possible if NullishCoalescing is disabled? If yes please include a comment about that.
| PTNODE(knopAsgRs2 , ">>>=" , ShrU_A , Bin , fnopBin|fnopAsg , "UnsignedRightShiftAssignExpr" ) | ||
| PTNODE(knopAsgLogOr , "||=" , Nop , Bin , fnopBin|fnopAsg , "LogicalOrAssignExpr") | ||
| PTNODE(knopAsgLogAnd , "&&=" , Nop , Bin , fnopBin|fnopAsg , "LogicalAndAssignExpr") | ||
| PTNODE(knopAsgCoalesce, "??=" , Nop , Bin , fnopBin|fnopAsg , "LogicalCoalesceAssignExpr") |
There was a problem hiding this comment.
Ubuntu sees "??=" as a trigraph (a weird obsolete C language feature) to stop the compile failures I think you'll need to change this to "?\?="
| PrintPnodeWIndent(pnode->AsParseNodeBin()->pnode2, indentAmt + INDENT_SIZE); | ||
| case knopAsgCoalesce: | ||
| Indent(indentAmt); | ||
| Output::Print(_u("??=\n")); |
There was a problem hiding this comment.
Ubuntu sees "??=" as a trigraph (a weird obsolete C language feature) to stop the compile failures I think you'll need to change this to "??="
| // We use a single dest here for the whole generating boolean expr, because we were poorly | ||
| // optimizing the previous version where we had a dest for each level | ||
| funcInfo->AcquireLoc(pnode); | ||
| const Js::RegSlot result_location = funcInfo->AcquireLoc(pnode); |
There was a problem hiding this comment.
NIT: I don't think we use const for any other RegSlot temps, so to be consistent please drop it here. Same point for other uses below.
There was a problem hiding this comment.
In fact there's no need to say result_location here; could revert this edit and use pnode->location below, except, you're assigning it to itself by the looks of things with your call below this may be where it's going wrong.
What is going wrong exactly?
| assert.sameValue = function sameValue(expected, actual, message) { | ||
| assert.areEqual(actual, expected, message); | ||
| } |
There was a problem hiding this comment.
What's wrong with the existing areEqual? We don't need to look the same as test262....
There was a problem hiding this comment.
We need some efforts to change from test262. Especially for a early draft PR.😂
| }, DummyError); | ||
|
|
||
| // assert.throws(function () { | ||
| // var base = null; |
There was a problem hiding this comment.
I assume you're planning more tests?
Perhaps have a look at test/es7/nullish.js for a model - tests I used for the ?? operator.
Also please put these in that folder. (es7 is used for new features that aren't closely tied to something existing)
There was a problem hiding this comment.
Yep. But something is wrong...I'm trying to find it out.
There was a problem hiding this comment.
The CI failures are due to the Trigraph issue I commented on above if you have other problems, let me know what's going wrong and I'll see if I can help.
If it compiles but doesn't run correctly for certain cases try putting the failing case in a file on its own, using a debug build of chakracore and running with the flag -dump:bytecode this will print out the bytecode instructions that are being generated so you can look at what exactly is happening - can also post that and I'll see if I can see the problem.
2 participants
Fixes #6658