Antlr4: initial discussion to start integration of new targets

[parrt]

As promised, I am now ready to integrate the new ANTLR target languages you folks have been working on. This issue is meant to get everybody in sync, check status, and discuss the proper order of integration and resolve issues etc.

There are two administrative details to get out of the way first:

1. Please let me know if there is another github user that should be added to one of the categories. Or, of course, if you would like your user ID removed from this discussion.
2. Nothing can be merged into antlr/antlr4 unless every single committer has added themselves to the contributors.txt file. It's onerous, particularly for simple commits, but it is requirement for anything merged into the master. Eclipse foundation lawyers tell me that we have one of the cleanest licenses out there and it contributes to ANTLR's widespread use because companies are not afraid to use the software. See the genesis of such heinous requirements in SCO v IBM. This means lead target authors have to go back through their committers list quickly and ask them to sign the contributors file with a new commit. Or, they can remove that commit and enter their own version of the functionality, being careful not to violate copyright on the previous.

As we proceed, please keep in mind that I have a difficult role, balancing the needs of multiple targets and keeping discussions in the civil and practical zone. Decisions I make come from the perspective of over 25 years managing and leading this project. I look forward to incorporating your hard work into the main antlr repo.

C++ current location

• @mike-lischke
• @DanMcLaughlin
• @nburles
• @davesisson

Go current location, previous discussion

• @pboyer

Swift current location: unclear, previous discussion

• @jeffreyguenther
• @hanjoes
• @janyou
• @ewanmellor

Likely interested/supporting humans (scraped from github issues):

• @RYDB3RG
• @wjkohnen
• @willfaught
• @parrt
• @sharwell
• @ericvergnaud

[mike-lischke]

ANTLR4 C++ target

The C++ target is now in use (also in production environments) since several months. I haven't got many serious issues (or any type of issue for that matter), but received already a number of pull requests/contributions. Hence, I consider it stable enough for the upstream merge, even though there will be more changes (including in the API) as we continue optimizing it. All contributors are listed in the contributors.txt file.

Build status: unlike other targets the C++ targets needs individual build environments on each supported platform. There are scripts to generate the source code package (which includes cmake files, for Linux + OSX), to create OSX dylibs and to create Windows DLLs. The result is zipped up and ready for upload once the particular script is done. Of course you need a build environment with XCode on a Mac (a Mac mini is enough) and one on a Windows box with VS 2013 and/or 2015 installed (the script will pick either one or both, if found). The Windows script expects 7z to be installed and available on PATH.

Test status: I have run all runtime tests and they succeed for the C++ target, the Java target + the C# target. Tests fail currently for Javascript + Python, because I had to change a few things in the code generation. Unfortunately, I was not able to get Travis to build + run the target tests. Also, tests will not run on Windows, because we are still missing the runtime lib building from the test java file.

Changes affecting all targets (new + old): there are two main areas:

1. The Java code generation has been extended to optionally create 2 files per target class, instead of only one. C++ needs header + implementation files. This is controlled by a new flag (in the Target.java class) which is false by default and set to true currently only for the C++ target. If set to true all code generation is done twice, once with the original code templates and in parallel with templates using the original names but with an additional "Header" suffix. These templates are not required in targets which return false from the Target.needsHeader() function.
2. I did not introduce introduced a few new code templates, but and had to add new parameters here and there, as they are required for C++ code generation. I have updated all existing targets with these additional parameters and new templates. New targets need to take these changes over yet.

Merge status: I'm merging changes in from the main repo from time to time to make sure the back merge will be as painless as possible. I haven't yet merged all the recent changes, but will do this soon. The fork is up to date now.

Doc status: I have updated doc/releasing-antlr.md with details about the C++ target and added doc/cpp-target.md with target specific details.

[parrt]

As C++ requires a few (ANTLR tool) model changes, I suggest we integrate it first and then try pushing changes to Go, Swift to see if it breaks anything. I'm open to other targets going in first, if you guys think it'd be easier.

[DanMcLaughlin]

Makes sense to me

[pboyer]

I just opened a PR for the Go language target: #1321. PTAL.

I'll gladly follow @parrt's advice on merging the C++ target first. At this point, I'm mainly polishing and documenting. All tests pass and I've used it in prod for nearly a year = 1 billion runs.

[parrt]

great! integrating C++ will be the tough one. will keep you up to date

[parrt]

otoh, maybe I should use the easy targets to help me remember the details of target development. what do you guys think?

[willfaught]

It seems to me like the total amount of work for everyone would be less if
targets with the least amount of changes that affect other targets are
integrated first.

On Friday, November 4, 2016, Terence Parr [email protected] wrote:

otoh, maybe I should use the easy targets to help me remember the details
of target development. what do you guys think?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/antlr/antlr4/issues/1319#issuecomment-258483465, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAD5VszznlEMqAzbG_3inMMIVZin9Jhoks5q62BkgaJpZM4Kphsb
.

[mike-lischke]

On the other hand all the required fixes would have to be done in the main repo, by the individual target writers, instead in their own repos and only the final result is merged.

[willfaught]

Each PR will have to be updated to match the new master anyway, right?

On Friday, November 4, 2016, Mike Lischke [email protected] wrote:

On the other hand all the required fixes would have to be done in the main
repo, by the individual target writers, instead in their own repos and only
the final result is merged.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/antlr/antlr4/issues/1319#issuecomment-258486822, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAD5VrBfB0ww3ZTQanTDgkwDOrjWa_lAks5q62NBgaJpZM4Kphsb
.

[parrt]

Hmm...we have model object and template changes. i'd like to get an idea of what the union of such changes for all targets would look like. i'm peeking at Go target to see model changes. might be minor @mike-lischke

[parrt]

Ok, a number of minor changes. @mike-lischke you need recognizer name also for rule functions etc... in C++, right? Will adds stuff like:

public class RuleSempredFunction extends RuleActionFunction {
    public String recognizerName; <--- new

    public RuleSempredFunction(OutputModelFactory factory, Rule r, String ctxType) {
        super(factory, r, ctxType);
        recognizerName = factory.getGrammar().getRecognizerName(); <--- new
    }
}

does that mess with your stuff?

[parrt]

some major changes too such as in StructDecl:

    @ModelElement public OrderedHashSet<Decl> tokenDecls = new OrderedHashSet<Decl>();
    @ModelElement public OrderedHashSet<Decl> tokenTypeDecls = new OrderedHashSet<Decl>();
    @ModelElement public OrderedHashSet<Decl> tokenListDecls = new OrderedHashSet<Decl>();
    @ModelElement public OrderedHashSet<Decl> ruleContextDecls = new OrderedHashSet<Decl>();
    @ModelElement public OrderedHashSet<Decl> ruleContextListDecls = new OrderedHashSet<Decl>();
    @ModelElement public OrderedHashSet<Decl> attributeDecls = new OrderedHashSet<Decl>();

Are these required because of being specific about types in the generated Go code?

[mike-lischke]

Ok, a number of minor changes. @mike-lischke https://github.com/mike-lischke you need recognizer name also for rule functions etc... in C++, right?

Yes, exactly. Have to qualify entity names everywhere and hence need that recognizer name.
Will adds stuff like:

public class RuleSempredFunction extends RuleActionFunction {
public String recognizerName; <--- new

public RuleSempredFunction(OutputModelFactory factory, Rule r, String ctxType) {
    super(factory, r, ctxType);
    recognizerName = factory.getGrammar().getRecognizerName(); <--- new
}

}
does that mess with your stuff?

Looks good. No problem for me.

Mike

www.soft-gems.net

[hanjoes]

I believe the Swift target is almost at the same status when @janyou created it. There is some additional work needs to be done before it is ready for merge. @jeffreyguenther was actively trying to put things together maybe he will have a more thorough update for it.
I will be able to take a peek of the target during the weekend coming.

[parrt]

Ok, @hanjoes, thanks.

[parrt]

Ok, gang, I think we should proceed in small stages so each change to a model or template argument requirement can be easily integrated and tested by the target developers. It's also a way for me to centralize updates to the model to make sure they are generally useful and with an appropriate interface. I suggest as a first step I observe all targets for the introduction of the useful recognizerName property and see if I can come up with a useful intersection. Perhaps I update the master then you guys can just pull that small incremental change in. We can all move forward together. Once we have all of the model changes in, then I can do a deep dive on each target to pull them in as a unit.

[parrt]

Looking at https://github.com/ewanmellor/antlr4/tree/swift-target I don't see any model changes, which means no template argument changes. I'll focus on C++ / Go compatibility then.

[janyou]

TO Swift target developer:

Current status
The Swift target 's runtime testsuite has already been passed and it supports Xcode 8(Swift 3.0).

Main change
The Swift target serializes ATN to JSON string.
The main reason to use JSON serialization is the ANTLR original serialization will cause xCode to crash (Open the generated file and scroll).If you want to change back to the original serialization, may pay attention to this problem. Using JSON serialization will only add some independent code and some class packages(javax.json-1.0.4.jar/javax.json-api-1.0.jar), will not affect the other target code generation.

Merge points
The main job is to merge the test templates. The following points for reference:

1.CompositeParsers/DelegatorRuleOverridesLookaheadInDelegate.stg

_Other target_:

decl : type_ ID ';'
  | type_ ID init ';' {<write("\"JavaDecl: \" + $text")>};
    init : '=' INT;

_Swift target_:

decl : type_ ID ';'
        | type_ ID initIt ';' {<write("\"JavaDecl: \" + $text")>};
    initIt : '=' INT;

_Difference_:
init -> initIt
note: init is a reserved word in Swift.

2.LeftRecursion/MultipleAlternativesWithCommonLabel.stg

_Other target_:

    e returns [int v]
      : e '*' e     {$v = <Cast("BinaryContext","$ctx")>.e(0).v * <Cast("BinaryContext","$ctx")>.e(1).v;}  # binary
      | e '+' e     {$v = <Cast("BinaryContext","$ctx")>.e(0).v + <Cast("BinaryContext","$ctx")>.e(1).v;}  # binary



_Swift target_:

    e returns [int v]
      : e '*' e     {$v = <Cast("BinaryContext","$ctx")>.<OptionalValue("e(0)")>.v * <Cast("BinaryContext","$ctx")>.<OptionalValue("e(1)")>.v;}  # binary
      | e '+' e     {$v = <Cast("BinaryContext","$ctx")>.<OptionalValue("e(0)")>.v + <Cast("BinaryContext","$ctx")>.<OptionalValue("e(1)")>.v;}  # binary

_Difference_:
e(0) -> <OptionalValue("e(0)")
e(1) -> <OptionalValue("e(1)")
note: e(0) and e(1) are optional values.
other targets's .test.stg should be added one line :

OptionalValue(value) ::= ""

3.ParserExec/ReferenceToATN.stg

_Other target_:

    a : (ID|ATN)* ATN? {<writeln("$text")>} ;
    ID : 'a'..'z'+ ;
    ATN : '0'..'9'+;
    WS : (' '|'\n') -> skip ;

_Swift target_:

    a : (ID|Atn)* Atn? {<writeln("$text")>} ;
    ID : 'a'..'z'+ ;
    Atn : '0'..'9'+;
    WS : (' '|'\n') -> skip ;

_Difference_:
ATN -> Atn
note: ATN is the Antlr runtime's class name.

Merge references
1.Manually build
2.Run Runtime Testsuite

[sharwell]

@janyou Please see the comment regarding the proposed change to ReferenceToATN.stg

https://github.com/antlr/antlr4/blob/master/runtime-testsuite/resources/org/antlr/v4/test/runtime/templates/ParserExec/ReferenceToATN.stg#L1-L5

/**
 * This is a regression test for antlr/antlr4#561 "Issue with parser
 * generation in 4.2.2"
 * https://github.com/antlr/antlr4/issues/561
 */

As is clear from the referenced issue #561, any renaming of ATN in this test defeats the _single_ purpose of this test's existence. If you cannot pass this test in the Swift target, you can ignore it using the IgnoredTests template in Swift.test.stg, but do not rename the token.

[sharwell]

@janyou Regarding the proposed change to MultipleAlternativesWithCommonLabel.stg

I recommend a different approach than OptionalValue. Not all targets have the concept of optional values treat this particular reference as an optional value. In particular, the TypeScript target treats this as a required value (at least it does today). I recommend renaming the template to something like <LabelAtIndex("e", 0)>, which targets can implement as optional or required according to the way lists are exposed by the code generator for that target.

[janyou]

I recommend a different approach than OptionalValue. Not all targets have the concept of optional values treat this particular reference as an optional value. In particular, the TypeScript target treats this as a required value (at least it does today). I recommend renaming the template to something like

I agree with you, OptionalValue may not be a good name.

As is clear from the referenced issue #561, any renaming of ATN in this test defeats the single purpose of this test's existence. If you cannot pass this test in the Swift target, you can ignore it using the IgnoredTests template in Swift.test.stg, but do not rename the token.

I made a mistake, Do as you say.

[mike-lischke]

The template changes need some extra work it seems. I also had to change a few of the test templates because they often use a dot for member reference, where in C++ I need an arrow. Hence I introduced a template for member access so that each target can have it's native syntax. This might conflict with changes like thie OptionValue template.

[parrt]

I'm examining targets. Swift has no model changes. I'm removing as many as possible from Go but generic model updates are going into my fork's branch https://github.com/parrt/antlr4/tree/model-updates-for-new-targets which when done should be union of C++ and Go model requirements. It might require tweaks to all targets but I can do those.

[sharwell]

@janyou thank you for keeping track of the changes. It is a huge help. I also need to update the comment in that test so it's more clear what is being tested. :)

[parrt]

Ok, I have Go target almost reduced to minimal model changes. working on generated file names now. Go needs stuff like foo_visitor vs FooVisitor like C++ and Java et al. C++ needs header files which add .h extension vs just .cpp or .java etc... Seems like we should delegate filename to the Target object or abstract it out into templates for each kind of output filename. E.g., Go has:

RecognizerFileName(name, type) ::= "<name; format={lower}>_<type; format={lower}>"
ListenerFileName(name) ::= "<name; format={lower}>_listener"
VisitorFileName(name) ::= "<name; format={lower}>_visitor"
BaseListenerFileName(name) ::= "<name; format={lower}>_base_listener"
BaseVisitorFileName(name) ::= "<name; format={lower}>_base_visitor"

codeFileExtension() ::= ".go"

Probably easier just to have CodeGenerator delegate getListenerFileName(), getVisitorFileName(), etc... to the Target. I can alter Target.java so it re-delegates back to CodeGenerator by default so no target changes needed for js, python, etc...

C++ header issue means the interface should have "is header" parameter. Can't think of better solution:

public String getRecognizerFileName(boolean header) {...}

I can overload so as not to disturb existing target code. adding to my model-updates-for-new-targets branch in parrt/antlr4 so @mike-lischke can try pulling into C++ branch.

Ok, made non-breaking change to my "union" branch: https://github.com/parrt/antlr4/commit/40e19e5d3751872d83365ce1ac3ad1c9e245973d Now to tweak Go target to suit.

[mike-lischke]

@parrt putting that into the templates is probably the best solution, like done in the Go target. That gives most flexibility. We should probably also settle on a common format for template rule names (e.g. camel case, no underscore, starting with uppercase letter). Minor thing, but while you are at it ... :-)

Let me know when I should merge your temp branch to see what needs to be changed for C++ then.

[parrt]

@mike-lischke i decided on specialization in Target so targets can use templates or the functions. I'm updating Go to just return name.toLowerCase()+"_parser.go". Re naming, each target should really have control over case etc... You can see the change to filename stuff here https://github.com/parrt/antlr4/commit/40e19e5d3751872d83365ce1ac3ad1c9e245973d

[parrt]

Ok, Go Target has no model changes now assuming I merge in my "union branch" in parrt/antlr4. @pboyer can you check this out https://github.com/pboyer/antlr4/tree/reduce-model-changes and merge into your master? Then it should be a clean pull for me into antlr/antlr4.

If @mike-lischke confirms my "union branch" (common model changes) integrates ok, then i'll merge those small changes to master.

[parrt]

@mike-lischke: I incorporated in a single commit all changes needed to support generating header files. many fewer diffs with C++ target codegen now.

[pboyer]

@parrt Just checking on travis for your changes. I'll let you know when we're good: https://github.com/pboyer/antlr4/pull/83

[parrt]

@pboyer sounds good. almost there on Go! making great progress on merging C++ codegen/model changes into my "union branch"

[parrt]

@janyou your repo doesn't have the full structure of antlr4 so merging will be more complicated. I _think_ I can keep your name on the commits by pulling into a new branch and then git mving into position.

Have you verified all of your committers have signed contributors.txt?

[parrt]

Ah. Looks like @janyou you yourself need to sign the contributors.txt ;) Can you add a commit for that.

[parrt]

@artyom-razinov will have to sign contributors.txt as well before we can merge the runtime stuff.

[parrt]

Looks like my model-updates-for-new-targets branch doesn't affect existing targets and makes C++ repo look very similar as far as codegen stuff. I'll merge it into master and prep master for 4.6 release that will have new targets and any bug fixes (eventually)

[ewanmellor]

@parrt I have already had a go at getting @janyou's code into the structure of antlr4. That's in https://github.com/ewanmellor/antlr4/tree/swift-target.

There's still a bunch of work that needs to be done on this code, as stated above and in Issue #945. I feel like we need to break out these items into separate issues, because it's just overwhelming if we try and talk about all the problems at the same time.

We also need to choose a branch to do this work in, because there are a bunch of variants of this code kicking around now. That could be a branch on antlr/antlr4, or my branch, or anyone else's as long as they're going to take the pull requests, I don't mind. I'd also be open to putting this into master (disabled for now obviously) if that's the easiest way for us to collaborate on it.

[parrt]

@ewanmellor we should probably decide who is going to lead the effort to finish the target. If that's you, then we can just collaborate on your fork then pull into master when done. I'll also need contributors.txt sorted out. :)

[ewanmellor]

@parrt contributors.txt is easy, I can do that.

I'm happy to organize, but I'm not going to have too much time, no more than a day a week, so if there's someone else who's going to be active on this then I'll step out of the way.

[parrt]

@ewanmellor ok, maybe @hanjoes can help out or take the lead with help from you guys? Can you check to ensure all committers on your fork are in contrib? thanks!

[parrt]

So it looks like Go target is ready for integration. all tests pass with latest master including improved codegen model. shouldn't affect any other targets. Any objections to me integrating Go target first?

[parrt]

Go has been integrated!

[davesisson]

Nice!

On Sat, Nov 5, 2016 at 4:07 PM Terence Parr [email protected]
wrote:

Go has been integrated!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/antlr/antlr4/issues/1319#issuecomment-258648314, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALUC_TTYqYgkGiMCvM7gXYN71JstSMTOks5q7Qw0gaJpZM4Kphsb
.

[parrt]

I just set version to 4.6-SNAPSHOT in antlr/antlr4 and updated test generation. Swift and C++ guys should grab that.

[davesisson]

Just tried out building the new build. I'm getting this exception:

org.antlr.v4.test.runtime.go.TestVisitors: Missing system property:antlr-go

Is there a doc I'm missing about how to get Go going?

[ewanmellor]

Swift Target

I have updated the branch at https://github.com/ewanmellor/antlr4/tree/swift-target. I shall rebase this branch to 4.6-SNAPSHOT. I have also enabled Travis builds for this branch at https://travis-ci.org/ewanmellor/antlr4.

I have created #1327, #1328, #1329, #1330, #1331 for the issues raised above.

I think I have fixed the issue with renaming ATN (https://github.com/ewanmellor/antlr4/commit/8b0373a580deb801e88f54cc40638cd7e023b1cd) so there won't be a need to disable those tests.

[janyou]

@parrt Contributors.txt has been sent to @ewanmellor and has been merged.

[parrt]

@davesisson After I installed Go, I had to add the bin dir to my PATH and then start a new terminal before go woodwork from the command line, which probably also messes up mvn etc...

[parrt]

An update. @mike-lischke has given me green light to attempt C++ integration :)

[parrt]

And....we have C++ https://github.com/antlr/antlr4/pull/1346 thanks so much to @mike-lischke @DanMcLaughlin @davesisson !!!

[parrt]

I'm completely rebuilding the runtime test generation stuff. https://github.com/antlr/antlr4/issues/1347 Going very well. I think we can even avoid code gen completely for building tests! i'm using some serious java mojo.

[parrt]

@ewanmellor how goes Swift? Need any help?

[parrt]

E.g., instead of:

TestType() ::= "Parser"

Options ::= [
    "Debug": false
]

Grammar ::= [
    "T": {<grammar("T")>}
]

Input() ::= "a b c"

Rule() ::= "a"

Output() ::= <<
abc<\n>
>>

Errors() ::= ""

grammar(grammarName) ::= <<
grammar <grammarName>;
a : ID+ {
<writeln("$text")>
};
ID : 'a'..'z'+;
WS : (' '|'\n') -> skip;
>>

we can use

public class APlus extends BaseRuntimeTestDescriptor {
    String input = "a b c";
    String output = "abc\n";
    String startRule = "a";
    String grammarName = "M";
    /**
     grammar T;
     a : ID+ {
     <writeln("$text")>
     };
     ID : 'a'..'z'+;
     WS : (' '|'\n') -> skip;
     */
    @CommentIsString
    public String grammar;
}

cool, right?

[ewanmellor]

@parrt Yes, help on the Swift target (from you or anyone else) would certainly be appreciated!

We currently have 9 test failures (see https://travis-ci.org/ewanmellor/antlr4). Some if not all of those are due to #1332, so that would be a good one.

@sharwell asked for #1331, and OptionalValue is even messier now than when we last discussed it because it got mangled when merging with recent template changes, so that definitely needs to be done before this is ready for master.

You filed #1168 also. I have no idea about this one, but I'm guessing that you do.

I have a partial patch for #1329, but it got ugly so it took longer than I hoped. I will not be able to work on this again until Nov 19 at the earliest, so if someone wants to take it off my hands I can explain the mess, or we could merge with the JSON-based serialization for now.

Could you also please tag those issues with target:swift so that we can keep track (I don't have permission to tag issues).

[parrt]

Hi.I'm cranking on rebuild of runtime test stuff; sorry for delay. I can upgrade your test stuff when you're ready to integrate. @hanjoes can you help out on these issues above?

[hanjoes]

Yeah, for sure. I will setup the workspace today and start looking.

[hanjoes]

@parrt I noticed that tests in testVisitors are ignored for other targets? Those test cases are failing for Swift target, is there any context for this?

[parrt]

The tests were added but the test author didn't spend the time to add to other targets. ;)

[hanjoes]

Okay, I will ignore those tests for now since the intention of the expected output is not totally clear to me without further investigation. I filed an issue just to track this and will move forward with Swift integration. #1368

[parrt]

Status is Go, C++ integrated. Swift is still in progress before it can be integrated I think,right @ewanmellor @janyou @hanjoes ?

[hanjoes]

Yes. Swift is still in progress.

[ewanmellor]

@parrt Yes, Swift is still in progress. I have merged all of @janyou 's recent changes, Travis is running now. We still have #1329 and #1331 as blockers. #1168 and #1368 to my knowledge are not blockers.

[ewanmellor]

@hanjoes #1331 would be a good one, if you have time right now.

[hanjoes]

Cool, I will take a look tonight.

[ewanmellor]

@hanjoes Thanks. Are you suggesting that we should skip testVisitors for now? If so, could you send me a pull request for that (or I'll do it on Saturday).

[hanjoes]

Looks like other targets are also ignoring the tests for testVisitors, at least in our current branch. @parrt is working on some new test mechanism also, so for now I think we can move onto other issues with swift target.

[hanjoes]

I can do it tonight, should be a fast PR.

[parrt]

@hanjoes @ewanmellor You guys should pull from antlr/antlr4 to get new test mechanism. it will ignore visitors test for all but javascript targets automatically.

[hanjoes]

k i can try rebasing the swift-target from @ewanmellor 's fork onto the newest master tonight.

[parrt]

@hanjoes did the rebase work?

[hanjoes]

I might need more time on this, there are a lot of commits and conflicts.

[parrt]

weird. the original fork I saw really only had a few files changed. Are there a bunch of changes in the code generation model?

[hanjoes]

what I went through yesterday are mostly build system and test template changes. Should be okay, but not trivial. A simple merge might be easier.

[parrt]

@hanjoes let me know if you want me to merge the non-swift parts

[hanjoes]

sure

[hanjoes]

About the OptionalValue mentioned in #1331, ran into some conflicts on this when rebasing. I'm planning to remove it since people are not super happy about it. Is there any problems? @parrt @janyou

[parrt]

I am OK with removing

Sent from my iPhone

On Nov 18, 2016, at 2:55 PM, hanjoes [email protected] wrote:

About the OptionalValue mentioned in #1331, ran into some conflicts on this when rebasing. I'm planning to remove it since people are not super happy about it. Is there any problems? @parrt @janyou

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[hanjoes]

Done with rebasing but having issues with Swift tests. Need to switch to the new test mechanism.

[janyou]

@hanjoes OptionalValue can not be removed, only be renamed.

2.LeftRecursion/MultipleAlternativesWithCommonLabel.stg

_Other target_:

  e returns [int v]
    : e '*' e     {$v = <Cast("BinaryContext","$ctx")>.e(0).v * <Cast("BinaryContext","$ctx")>.e(1).v;}  # binary
    | e '+' e     {$v = <Cast("BinaryContext","$ctx")>.e(0).v + <Cast("BinaryContext","$ctx")>.e(1).v;}  # binary



_Swift target_:

  e returns [int v]
    : e '*' e     {$v = <Cast("BinaryContext","$ctx")>.<OptionalValue("e(0)")>.v * <Cast("BinaryContext","$ctx")>.<OptionalValue("e(1)")>.v;}  # binary
    | e '+' e     {$v = <Cast("BinaryContext","$ctx")>.<OptionalValue("e(0)")>.v + <Cast("BinaryContext","$ctx")>.<OptionalValue("e(1)")>.v;}  # binary

_Difference_:
e(0) -> <OptionalValue("e(0)")
e(1) -> <OptionalValue("e(1)")
note: e(0) and e(1) are optional values in Swift.
other targets's .test.stg should be added one line :

OptionalValue(value) ::= ""

[hanjoes]

I need to look more into test templates to know how exactly it works. Maybe i will revert some of the changes. Right now I'm not sure templates are used any more.

[parrt]

@ewanmellor @janyou @hanjoes integrating Swift shortly after burst of work from @hanjoes yesterday. been fighting travis all day but I think it's ready to go. https://github.com/antlr/antlr4/pull/1382

@mike-lischke changed travis in PR for swift so C++ runs on osx.

When I get Swift in, I'll focus on bugs and move towards a 4.6 release.

[ewanmellor]

@parrt Sounds great. Anything I can do?

[parrt]

@ewanmellor Nope. I think I just beat travis into submission. no C++ on osx but it's ok to merge Swift in. Shouldn't be long now.

[parrt]

Looks like the branch/fork will pass all tests. Merged in Swift target! Thanks everyone! Ok, i'm going to close this "issue" with https://github.com/antlr/antlr4/pull/1382 . 💯

[parrt]

I invited active master impl target devs to target team. check your email. i'll mention that team name (just renamed to be @antlr/antlr-targets) when i update a target that could require changes in your target(s).