Skip to content

Support an integer type annotation argument#267

Merged
kanghyojun merged 5 commits into
nirum-lang:masterfrom
kanghyojun:annotation-integer
May 12, 2018
Merged

Support an integer type annotation argument#267
kanghyojun merged 5 commits into
nirum-lang:masterfrom
kanghyojun:annotation-integer

Conversation

@kanghyojun

@kanghyojun kanghyojun commented May 11, 2018

Copy link
Copy Markdown
Member

Support an integer type annotation argument which is one of the features that listed on #178. Since this is the blocker issue of #206, we can add constraints for integer types(int32, int64, bigint) after merging this PR.

However, I should create fixture.datetime.datetime-service, because all types except for a method of service couldn't compile the annotation. We have to compile the annotation of the unboxed type first to do #206.

Review, please :) @dahlia @Kroisse @AiOO.

@kanghyojun kanghyojun added typ:enhance Type: Enhancement/new feature cmp:compiler Component: Compiler backend (e.g., annotation processors, code generators) cat:lang Category: Language design labels May 11, 2018
@kanghyojun kanghyojun self-assigned this May 11, 2018
@kanghyojun kanghyojun requested review from AiOO, Kroisse and dahlia May 11, 2018 17:33
@kanghyojun kanghyojun changed the title Support an integer type annotation parameter Support an integer type annotation argument May 11, 2018
Comment thread src/Nirum/Parser.hs Outdated
v <- many digitChar
case readMaybe v of
Just i -> return $ AInt i
Nothing -> fail "digit expected"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could define a more general parser to take one or more digits and returns Integer, i.e., integer :: Parser Integer, and then make this annotationArgumentValue to utilize that.


type AnnotationArgumentSet = M.Map Identifier T.Text
data AnnotationArgument = AText T.Text
| AInt Int

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Int has a limits on its size (up to 0x1FFFFFFF). We'd better to use Integer instead.
  • Data constructors shouldn't be prefixed with A.

dahlia
dahlia previously approved these changes May 12, 2018
@checkmate-bot

checkmate-bot commented May 12, 2018

Copy link
Copy Markdown

Checklist 🤔

src/Nirum/Parser.hs

  • If a new reserved keyword is introduced, it has to be also added to reservedKeywords set in the Nirum.Constructs.Identifier module.

Comment thread CHANGES.md Outdated
import iso (country as iso-country);
import types (country);
~~~~~~~~
- Support an integer type annotation argument. [[#178], [#267]]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the past tense.

@kanghyojun kanghyojun force-pushed the annotation-integer branch 2 times, most recently from cdac2fa to 7941023 Compare May 12, 2018 12:26
@codecov

codecov Bot commented May 12, 2018

Copy link
Copy Markdown

Codecov Report

Merging #267 into master will decrease coverage by 0.05%.
The diff coverage is 84.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #267      +/-   ##
==========================================
- Coverage   76.14%   76.08%   -0.06%     
==========================================
  Files          33       33              
  Lines        2452     2467      +15     
  Branches      131      132       +1     
==========================================
+ Hits         1867     1877      +10     
- Misses        454      458       +4     
- Partials      131      132       +1
Impacted Files Coverage Δ
src/Nirum/Parser.hs 87.4% <100%> (+0.05%) ⬆️
src/Nirum/Constructs/Docs.hs 94.73% <100%> (ø) ⬆️
src/Nirum/Constructs/Annotation/Internal.hs 77.27% <50%> (-6.94%) ⬇️
src/Nirum/Targets/Python.hs 89% <66.66%> (-0.25%) ⬇️
src/Nirum/Constructs/Annotation.hs 91.89% <80%> (-2.4%) ⬇️

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 814f455...e1498ef. Read the comment docs.

@kanghyojun kanghyojun force-pushed the annotation-integer branch from 7941023 to e1498ef Compare May 12, 2018 12:47
@kanghyojun kanghyojun merged commit 5633ee0 into nirum-lang:master May 12, 2018
@dahlia dahlia mentioned this pull request May 12, 2018
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cat:lang Category: Language design cmp:compiler Component: Compiler backend (e.g., annotation processors, code generators) typ:enhance Type: Enhancement/new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants