[C++] Build fix/correction for UTF32 conversion. #1908
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The twisted way to create a javac like output path breaks many use cases, including very simple ones with lexer + parser grammar in a relative subdir (where parser generation doesn't find the lexer tokens). Additionally, not all projects prefer to have their generated files in an output folder which is comprised of the given output path and the subdir given for the grammar files. The check for absolute paths as decision criterion is a weak one anyway - it makes simple usage difficult and doesn't deal with path expansion on all platforms. If a javac like output path is required it should be easy to construct this upfront and provide it as the output path parameter, instead of implicitely creating it. This patch simplifies things without any WTF moments because generated files end up and unexpected locations. It now can deal with simple cases (no output, no grammar subdir) up to complicated cases (nested and/or absolute output dir, multiple grammar subfolders).
Removed some duplicate classes there too.
Also made converter local vars in conversion routines, instead of static global vars.
This reverts commit defad74.
This reverts commit 47e43cc.
|
TestCompositeParsers still fails with a compiler error, maybe you need to have a second look? |
|
@ericvergnaud Where do you see a compiler error? What fails is what always fails since last fall, job runtime exceeded maximum: https://travis-ci.org/antlr/antlr4/jobs/241708892 |
|
@mike-lischke look in https://travis-ci.org/antlr/antlr4/jobs/241757707, which is from the latest master + non cpp changes |
|
That's a completely different branch. Why do you post the failure notice in this PR? I have a pull request (#1910) for that failure. |
|
I posted here because it seems things started breaking when @parrt merged it
Envoyé de mon iPhone
… Le 25 juin 2017 à 18:21, Mike Lischke ***@***.***> a écrit :
That's a completely different branch. Why do you post the failure notice in this PR? I have a pull request (#1910) for that failure, but there's not much moving in antlr4 these days.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Agreed |
|
Hi guys, i think i've caught up with merges related to bug fixes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a few fixes after the recent UTF32 conversion changes. Also fixes some warnings.