Add support unary expressions for constants #30693

rpjayasekara · 2021-05-20T10:25:19Z

Purpose

Support unary expressions for constants

Approach

Describe how you are implementing the solutions along with the design details.

Samples

const int MIN_VALUE = (-(10+5));
const int CUI3 = ~2;
const float CUF2 = +(10.0 + 2.0);
const boolean CUB = !(true);

Remarks

List any other known issues, related PRs, TODO items, or any other notes related to the PR.

Check List

rdulmina · 2021-05-20T12:36:44Z

...-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantAnalyzer.java

-import org.wso2.ballerinalang.compiler.tree.expressions.BLangBinaryExpr;
-import org.wso2.ballerinalang.compiler.tree.expressions.BLangConstant;
-import org.wso2.ballerinalang.compiler.tree.expressions.BLangExpression;
-import org.wso2.ballerinalang.compiler.tree.expressions.BLangGroupExpr;
-import org.wso2.ballerinalang.compiler.tree.expressions.BLangLiteral;
-import org.wso2.ballerinalang.compiler.tree.expressions.BLangNumericLiteral;
-import org.wso2.ballerinalang.compiler.tree.expressions.BLangRecordLiteral;
-import org.wso2.ballerinalang.compiler.tree.expressions.BLangSimpleVarRef;
+import org.wso2.ballerinalang.compiler.tree.expressions.*;


We use single class imports

Will add the import statements

rdulmina · 2021-05-20T12:41:57Z

...-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantAnalyzer.java

@@ -127,6 +120,18 @@ public void visit(BLangGroupExpr expr) {
        analyzeExpr(expr.expression);
    }

+    @Override
+    public void visit(BLangUnaryExpr unaryExpr) {
+


We don't add new line here

…ina-lang into fix-ballerina-platform#30019

codecov · 2021-05-21T11:04:27Z

Codecov Report

Merging #30693 (dfcf859) into master (5a5b376) will increase coverage by 0.00%.
The diff coverage is 78.94%.

@@            Coverage Diff            @@
##             master   #30693   +/-   ##
=========================================
  Coverage     69.14%   69.14%           
- Complexity    38174    38193   +19     
=========================================
  Files          2902     2902           
  Lines        159717   159753   +36     
  Branches      20040    20045    +5     
=========================================
+ Hits         110438   110469   +31     
+ Misses        42782    42779    -3     
- Partials       6497     6505    +8

Impacted Files	Coverage Δ
.../compiler/semantics/analyzer/ConstantAnalyzer.java	`88.67% <60.00%> (-3.16%)`	⬇️
...iler/semantics/analyzer/ConstantValueResolver.java	`80.31% <80.00%> (+0.55%)`	⬆️
...alang/compiler/semantics/analyzer/TypeChecker.java	`86.62% <100.00%> (+<0.01%)`	⬆️
.../java/io/ballerina/projects/PackageResolution.java	`87.37% <0.00%> (-2.03%)`	⬇️
...lang/compiler/semantics/analyzer/CodeAnalyzer.java	`71.54% <0.00%> (+0.04%)`	⬆️
.../compiler/bir/codegen/interop/JMethodResolver.java	`78.04% <0.00%> (+0.27%)`	⬆️
...runtime/internal/scheduling/WorkerDataChannel.java	`87.26% <0.00%> (+2.54%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a5b376...dfcf859. Read the comment docs.

rdhananjaya · 2021-05-21T12:34:38Z

tests/jballerina-unit-test/src/test/resources/test-src/types/constant/constant-expression.bal

@@ -114,6 +114,16 @@ function testBitwiseConstExpressions() {
    assertEqual(ZERO_EXT_5, 0x0);
 }

+const int CUI1 = -(10);


Shall we add decimal and float test cases too

…ina-lang into fix-ballerina-platform#30019

rdulmina · 2021-05-24T05:30:44Z

.../src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantValueResolver.java

@@ -215,6 +222,25 @@ private BLangConstantValue calculateConstValue(BLangConstantValue lhs, BLangCons
        return new BLangConstantValue(null, this.currentConstSymbol.type);
    }

+    private BLangConstantValue calculateConstValue(BLangConstantValue value, OperatorKind kind) {
+


Let's remove the unwanted newline in other places as well

will reformat all

…ina-lang into fix-ballerina-platform#30019

rdhananjaya · 2021-06-01T10:51:56Z

...erina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeChecker.java

-            exprType = OperatorKind.ADD.equals(unaryExpr.operator) ? checkExpr(unaryExpr.expr, env, expType) :
+            boolean decimalNegation = OperatorKind.SUB.equals(unaryExpr.operator) && expType.tag == TypeTags.DECIMAL;
+            boolean isAdd = OperatorKind.ADD.equals(unaryExpr.operator);
+            exprType = (decimalNegation || isAdd) ? checkExpr(unaryExpr.expr, env, expType) :


Shall we add a comment saying what we do here

rdhananjaya · 2021-06-01T10:54:25Z

.../src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantValueResolver.java

+
+    private BLangConstantValue calculateBooleanComplement(BLangConstantValue value) {
+        Object result = null;
+        switch (this.currentConstSymbol.type.tag) {


Since this is a single case, let's use a if here

same comment for line:386

rdhananjaya · 2021-06-01T10:54:43Z

.../src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantValueResolver.java

+
+    private BLangConstantValue calculateBooleanComplement(BLangConstantValue value) {
+        Object result = null;
+        switch (this.currentConstSymbol.type.tag) {


same comment for line:386

rdhananjaya · 2021-06-01T10:59:33Z

.../src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantValueResolver.java

@@ -215,6 +218,26 @@ private BLangConstantValue calculateConstValue(BLangConstantValue lhs, BLangCons
        return new BLangConstantValue(null, this.currentConstSymbol.type);
    }

+    private BLangConstantValue calculateConstValue(BLangConstantValue value, OperatorKind kind) {


This method is only called for unary expressions right, shall we change the name of the method to reflect that, something like evaluateUnaryOperator or something. In fact I don't see any value addition by having this bit of logic as a separate method. I think we can just inline this method.

Yes, it only calls for unary expressions. Will change the method name to reflect that

…ina-lang into fix-ballerina-platform#30019

rdulmina

LGTM

rdhananjaya · 2021-06-09T04:47:39Z

.../src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantValueResolver.java

+                break;
+            case TypeTags.DECIMAL:
+                BigDecimal valDecimal = new BigDecimal(String.valueOf(value.value), MathContext.DECIMAL128);
+                BigDecimal negDecimal = new BigDecimal(String.valueOf(-1), MathContext.DECIMAL128);


https://docs.oracle.com/javase/8/docs/api/java/math/BigDecimal.html#negate--

rpjayasekara added 2 commits May 20, 2021 15:43

Support Unary expressions for constants

249b217

Add unary expression constant test cases

e3ac101

rdhananjaya added the Team/CompilerFE All issues related to Language implementation and Compiler, this exclude run times. label May 20, 2021

Reformat unary expression constant test cases

51ffb64

rdulmina requested changes May 20, 2021

View reviewed changes

rdulmina reviewed May 20, 2021

View reviewed changes

rpjayasekara added 4 commits May 21, 2021 11:44

Add negative unary expression constant test cases

82e9e06

Merge branch 'master' of https://github.com/ballerina-platform/baller…

8162360

…ina-lang into fix-ballerina-platform#30019

Reformat import statements

144c1be

Merge branch 'master' of https://github.com/ballerina-platform/baller…

59d1629

…ina-lang into fix-ballerina-platform#30019

rdhananjaya reviewed May 21, 2021

View reviewed changes

rpjayasekara added 2 commits May 24, 2021 09:17

Add new constant unary expression test cases

c52f823

Merge branch 'master' of https://github.com/ballerina-platform/baller…

8cfc263

…ina-lang into fix-ballerina-platform#30019

rdulmina reviewed May 24, 2021

View reviewed changes

rpjayasekara added 8 commits May 24, 2021 16:02

Add support negative decimal unary expression

7a53140

Add support bitwise complement and not operators

e64e7ae

Fix review suggestions

684cdb1

Merge branch 'master' of https://github.com/ballerina-platform/baller…

32e89e1

…ina-lang into fix-ballerina-platform#30019

Fix review suggestions

02f8312

Add negative constant decimal test case

e4a5c58

Remove Byte type handler

198e19d

Merge branch 'master' of https://github.com/ballerina-platform/baller…

65de438

…ina-lang into fix-ballerina-platform#30019

rdhananjaya reviewed Jun 1, 2021

View reviewed changes

rpjayasekara added 2 commits June 1, 2021 17:27

Fix review suggestions

74209f9

Merge branch 'master' of https://github.com/ballerina-platform/baller…

dfcf859

…ina-lang into fix-ballerina-platform#30019

MaryamZi requested review from rdulmina and rdhananjaya June 8, 2021 03:59

MaryamZi added this to the Ballerina Swan Lake - Beta2 milestone Jun 8, 2021

rdulmina approved these changes Jun 9, 2021

View reviewed changes

rdhananjaya reviewed Jun 9, 2021

View reviewed changes

rdhananjaya approved these changes Jun 9, 2021

View reviewed changes

rdhananjaya merged commit 50c6020 into ballerina-platform:master Jun 9, 2021

prakanth97 mentioned this pull request Sep 13, 2022

Implement Constant expressions #13944

Open

22 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support unary expressions for constants #30693

Add support unary expressions for constants #30693

rpjayasekara commented May 20, 2021 •

edited

Loading

rdulmina May 20, 2021

rpjayasekara May 20, 2021

rdulmina May 20, 2021 •

edited

Loading

codecov bot commented May 21, 2021 •

edited

Loading

rdhananjaya May 21, 2021

rdulmina May 24, 2021

rpjayasekara May 24, 2021

rdhananjaya Jun 1, 2021

rdhananjaya Jun 1, 2021

rdhananjaya Jun 1, 2021

rdhananjaya Jun 1, 2021

rdhananjaya Jun 1, 2021

rpjayasekara Jun 1, 2021

rdulmina left a comment

rdhananjaya Jun 9, 2021

Add support unary expressions for constants #30693

Add support unary expressions for constants #30693

Conversation

rpjayasekara commented May 20, 2021 • edited Loading

Purpose

Approach

Samples

Remarks

Check List

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rdulmina May 20, 2021 • edited Loading

Choose a reason for hiding this comment

codecov bot commented May 21, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rdulmina left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rpjayasekara commented May 20, 2021 •

edited

Loading

rdulmina May 20, 2021 •

edited

Loading

codecov bot commented May 21, 2021 •

edited

Loading