-
Notifications
You must be signed in to change notification settings - Fork 753
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support constant byte array literal #40624
Conversation
Depends on #40621 |
Getting runtime error for below case
seems same behaviour for the list constant as well. Below works fine.
|
@@ -259,6 +255,12 @@ public void visit(BLangLiteral literalExpr, AnalyzerData data) { | |||
return; | |||
} | |||
|
|||
if (literalType.tag == TypeTags.BYTE_ARRAY) { | |||
BLangListConstructorExpr listConstructorExpr = rewriteByteArrayLiteral(literalExpr); | |||
data.resultType = checkListConstructorCompatibility(literalType, listConstructorExpr, data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should do the type checking with the expected type as well here. Due to that we did log error for below case.
const string[] X = base16 `aabb`;
This should be error according to the spec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can simply check compatibility with expected type here, will change to that.
const byte[] data2 = base64 `ABCD pqrs`; | ||
|
||
const byte[3] data3 = base16 `55 EE 66`; | ||
const byte[6] data4 = base64 `ABCD pqrs`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we adds tests containing inferred array, unions and singletons as expected type as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and subtype of int as expected type
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #40624 +/- ##
============================================
+ Coverage 76.45% 76.61% +0.16%
- Complexity 55279 55321 +42
============================================
Files 3386 3377 -9
Lines 208244 208073 -171
Branches 26954 26941 -13
============================================
+ Hits 159209 159422 +213
+ Misses 40259 39867 -392
- Partials 8776 8784 +8
☔ View full report in Codecov by Sentry. |
@@ -527,7 +549,7 @@ private BType checkMappingConstructorCompatibility(BType expType, BLangRecordLit | |||
switch (possibleType.tag) { | |||
case TypeTags.MAP: | |||
return validateSpecifiedFieldsAndGetType(mappingConstructor, possibleType, data); | |||
case TypeTags.RECORD: | |||
case TypeTags.RECORD : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
case TypeTags.RECORD : | |
case TypeTags.RECORD: |
@@ -2715,7 +2715,7 @@ private HashSet<String> getFieldNames(List<RecordLiteralNode.RecordField> specif | |||
return fieldNames; | |||
} | |||
|
|||
private String getKeyValueFieldName(BLangRecordKeyValueField field) { | |||
public String getKeyValueFieldName(BLangRecordKeyValueField field) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use default-access-level instead of making it public.
public String getKeyValueFieldName(BLangRecordKeyValueField field) { | |
String getKeyValueFieldName(BLangRecordKeyValueField field) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Address here d97530c
&& targetTag == TypeTags.ARRAY | ||
&& isArrayTypesAssignable((BArrayType) source, target, unresolvedTypes); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can write this two lines in-line
@@ -0,0 +1,71 @@ | |||
/* | |||
* Copyright (c) 2023, WSO2 LLC. (http://www.wso2.org) All Rights Reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Copyright (c) 2023, WSO2 LLC. (http://www.wso2.org) All Rights Reserved. | |
* Copyright (c) 2023, WSO2 LLC. (http://www.wso2.com). |
Let's fix the other few places as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed here d97530c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to review this #40621 before this PR
1555bc9
to
d08328e
Compare
Moved to #40979 |
Purpose
Fixes #40536
Fixes #37423
Approach
Samples
Remarks
Check List