[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[
https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16682737#comment-16682737
]
ASF GitHub Bot commented on THRIFT-4662:
jake-ruyi commented on a change in pull request #1624: THRIFT-4662: Rust const
string calls function at compile time
URL: https://github.com/apache/thrift/pull/1624#discussion_r232474522
##
File path: compiler/cpp/src/thrift/generate/t_rs_generator.cc
##
@@ -673,15 +676,22 @@ void t_rs_generator::render_const_value_holder(const
string& name, t_type* ttype
f_gen_ << endl;
}
-void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue) {
+void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue,
bool isowned) {
if (ttype->is_base_type()) {
t_base_type* tbase_type = (t_base_type*)ttype;
switch (tbase_type->get_base()) {
case t_base_type::TYPE_STRING:
if (tbase_type->is_binary()) {
-f_gen_ << "\"" << tvalue->get_string() << "\""<<
".to_owned().into_bytes()";
+if (isowned) {
Review comment:
That explains it. I could have sworn it needed to be `&'static`, but I
found a bunch of examples that were `&str` and assumed I was thinking of
something else...
Do you think it's worth outputting `&'static str` instead so it works with
Rust <1.17?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Rust const string calls function at compile time
>
>
> Key: THRIFT-4662
> URL: https://issues.apache.org/jira/browse/THRIFT-4662
> Project: Thrift
> Issue Type: Bug
> Components: Rust - Compiler
>Affects Versions: 0.11.0
> Environment: C:\Users\jake>rustup show
> Default host: x86_64-pc-windows-msvc
> stable-x86_64-pc-windows-msvc (default)
> rustc 1.30.0 (da5f414c2 2018-10-24)
>Reporter: J W
>Assignee: Allen George
>Priority: Major
>
> *For this thrift:*
> const string broker_playback_message = "mmi.developer.playback"
> *Generates:*
> // thrift -gen rs -out ../rust/thrift/src const_string.thrift
> pub const BROKER_PLAYBACK_MESSAGE: String =
> "mmi.developer.playback".to_owned();
> *Fails to compile:*
> error[E0015]: calls in constants are limited to tuple structs and tuple
> variants
> note: a limited form of compile-time function evaluation is available on a
> nightly compiler via `const fn`
> *Fix:*
> Probably want to output:
> pub const BROKER_PLAYBACK_MESSAGE: &str = "mmi.developer.playback";
>
> Looking at render_const_value() it looks like byte arrays will have the same
> issue.
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[
https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16682738#comment-16682738
]
ASF GitHub Bot commented on THRIFT-4662:
jake-ruyi commented on a change in pull request #1624: THRIFT-4662: Rust const
string calls function at compile time
URL: https://github.com/apache/thrift/pull/1624#discussion_r232474522
##
File path: compiler/cpp/src/thrift/generate/t_rs_generator.cc
##
@@ -673,15 +676,22 @@ void t_rs_generator::render_const_value_holder(const
string& name, t_type* ttype
f_gen_ << endl;
}
-void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue) {
+void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue,
bool isowned) {
if (ttype->is_base_type()) {
t_base_type* tbase_type = (t_base_type*)ttype;
switch (tbase_type->get_base()) {
case t_base_type::TYPE_STRING:
if (tbase_type->is_binary()) {
-f_gen_ << "\"" << tvalue->get_string() << "\""<<
".to_owned().into_bytes()";
+if (isowned) {
Review comment:
That explains it. I could have sworn it needed to be `&'static`, but I
found a bunch of examples that were `&str` and it compiled so I assumed I was
thinking of something else...
Do you think it's worth outputting `&'static str` instead so it works with
Rust <1.17?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Rust const string calls function at compile time
>
>
> Key: THRIFT-4662
> URL: https://issues.apache.org/jira/browse/THRIFT-4662
> Project: Thrift
> Issue Type: Bug
> Components: Rust - Compiler
>Affects Versions: 0.11.0
> Environment: C:\Users\jake>rustup show
> Default host: x86_64-pc-windows-msvc
> stable-x86_64-pc-windows-msvc (default)
> rustc 1.30.0 (da5f414c2 2018-10-24)
>Reporter: J W
>Assignee: Allen George
>Priority: Major
>
> *For this thrift:*
> const string broker_playback_message = "mmi.developer.playback"
> *Generates:*
> // thrift -gen rs -out ../rust/thrift/src const_string.thrift
> pub const BROKER_PLAYBACK_MESSAGE: String =
> "mmi.developer.playback".to_owned();
> *Fails to compile:*
> error[E0015]: calls in constants are limited to tuple structs and tuple
> variants
> note: a limited form of compile-time function evaluation is available on a
> nightly compiler via `const fn`
> *Fix:*
> Probably want to output:
> pub const BROKER_PLAYBACK_MESSAGE: &str = "mmi.developer.playback";
>
> Looking at render_const_value() it looks like byte arrays will have the same
> issue.
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (THRIFT-4529) Rust generation should include #![allow(non_snake_case)] or force conform to Rust style guidelines
[
https://issues.apache.org/jira/browse/THRIFT-4529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16682504#comment-16682504
]
ASF GitHub Bot commented on THRIFT-4529:
allengeorge opened a new pull request #1625: THRIFT-4529: Camel-case enum
variants
URL: https://github.com/apache/thrift/pull/1625
Client: rs
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Rust generation should include #![allow(non_snake_case)] or force conform to
> Rust style guidelines
> --
>
> Key: THRIFT-4529
> URL: https://issues.apache.org/jira/browse/THRIFT-4529
> Project: Thrift
> Issue Type: Improvement
> Components: Rust - Compiler
>Affects Versions: 0.11.0
>Reporter: Joshua
>Assignee: Allen George
>Priority: Minor
>
> Without this, building a project using a thrift file meant for multiple
> languages may end up with many compiler warnings similar to the following:
> {code:sh}
> warning: variant `EXAMPLE_NAME` should have a camel case name such as
> `ExampleName`
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (THRIFT-4529) Rust generation should include #![allow(non_snake_case)] or force conform to Rust style guidelines
[
https://issues.apache.org/jira/browse/THRIFT-4529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16682477#comment-16682477
]
Allen George edited comment on THRIFT-4529 at 11/10/18 3:53 PM:
Humorously (I guess this is the story of all programming) this turns out to be
more complicated than expected.
There are a few cases. I've outlined the cases and the rust-compatible outcome:
1. All lowercase. "foo" => "Foo"
2. All uppercase. "FOO" => "Foo"
3. Screaming snake case (most common, I'd assume). "SCREAMING_SNAKE_CASE" =>
"ScreamingSnakeCase"
4. Snake case. "snake_case" => "SnakeCase"
4. Camel case. "CamelCase" => "CamelCase"
I'm going to hope no one does something bizarre like some mutated combination
of snake and camelcase (for example "Mutant_Case").
The problem is, we have to figure out which one of the situations we're dealing
with, because the conversions we'll perform will differ for every one, which is
supremely annoying. I tried to avoid having to differentiate between the cases,
but, the default implementations of {{t_generator::camelcase}} and
{{t_generator::uppercase}} make this impossible.
was (Author: allengeorge):
Humorously (I guess this is the story of all programming) this turns out to be
more complicated than expected.
There are a few cases. I've outlined the cases and the rust-compatible outcome:
1. All lowercase. "foo" => "Foo"
2. All uppercase. "FOO" => "Foo"
3. Screaming snake case (most common, I'd assume). "SCREAMING_SNAKE_CASE" =>
"ScreamingSnakeCase"
4. Snake case. "snake_case" => "SnakeCase"
4. Camel case. "CamelCase" => "CamelCase"
The problem is, we have to figure out which one of the situations we're dealing
with, because the conversions we'll perform will differ for every one, which is
supremely annoying. I tried to avoid having to differentiate between the cases,
but, the default implementations of {{t_generator::camelcase}} and
{{t_generator::uppercase}} make this impossible.
> Rust generation should include #![allow(non_snake_case)] or force conform to
> Rust style guidelines
> --
>
> Key: THRIFT-4529
> URL: https://issues.apache.org/jira/browse/THRIFT-4529
> Project: Thrift
> Issue Type: Improvement
> Components: Rust - Compiler
>Affects Versions: 0.11.0
>Reporter: Joshua
>Assignee: Allen George
>Priority: Minor
>
> Without this, building a project using a thrift file meant for multiple
> languages may end up with many compiler warnings similar to the following:
> {code:sh}
> warning: variant `EXAMPLE_NAME` should have a camel case name such as
> `ExampleName`
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (THRIFT-4529) Rust generation should include #![allow(non_snake_case)] or force conform to Rust style guidelines
[
https://issues.apache.org/jira/browse/THRIFT-4529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16682477#comment-16682477
]
Allen George edited comment on THRIFT-4529 at 11/10/18 3:52 PM:
Humorously (I guess this is the story of all programming) this turns out to be
more complicated than expected.
There are a few cases. I've outlined the cases and the rust-compatible outcome:
1. All lowercase. "foo" => "Foo"
2. All uppercase. "FOO" => "Foo"
3. Screaming snake case (most common, I'd assume). "SCREAMING_SNAKE_CASE" =>
"ScreamingSnakeCase"
4. Snake case. "snake_case" => "SnakeCase"
4. Camel case. "CamelCase" => "CamelCase"
The problem is, we have to figure out which one of the situations we're dealing
with, because the conversions we'll perform will differ for every one, which is
supremely annoying. I tried to avoid having to differentiate between the cases,
but, the default implementations of {{t_generator::camelcase}} and
{{t_generator::uppercase}} make this impossible.
was (Author: allengeorge):
Humorously (I guess this is the story of all programming) this turns out to be
more complicated than expected.
There are a few cases:
1. All lowercase. "foo" => "Foo"
2. All uppercase. "FOO" => "Foo"
3. Screaming snake case (most common, I'd assume). "SCREAMING_SNAKE_CASE" =>
"ScreamingSnakeCase"
4. Snake case. "snake_case" => "SnakeCase"
4. Camel case. "CamelCase" => "CamelCase"
The problem is, we have to figure out which one of the situations we're dealing
with, because the conversions we'll perform will differ for every one, which is
supremely annoying. I tried to avoid having to differentiate between the cases,
but, the default implementations of {{t_generator::camelcase}} and
{{t_generator::uppercase}} make this impossible.
> Rust generation should include #![allow(non_snake_case)] or force conform to
> Rust style guidelines
> --
>
> Key: THRIFT-4529
> URL: https://issues.apache.org/jira/browse/THRIFT-4529
> Project: Thrift
> Issue Type: Improvement
> Components: Rust - Compiler
>Affects Versions: 0.11.0
>Reporter: Joshua
>Assignee: Allen George
>Priority: Minor
>
> Without this, building a project using a thrift file meant for multiple
> languages may end up with many compiler warnings similar to the following:
> {code:sh}
> warning: variant `EXAMPLE_NAME` should have a camel case name such as
> `ExampleName`
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (THRIFT-4529) Rust generation should include #![allow(non_snake_case)] or force conform to Rust style guidelines
[
https://issues.apache.org/jira/browse/THRIFT-4529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16682477#comment-16682477
]
Allen George commented on THRIFT-4529:
--
Humorously (I guess this is the story of all programming) this turns out to be
more complicated than expected.
There are a few cases:
1. All lowercase. "foo" => "Foo"
2. All uppercase. "FOO" => "Foo"
3. Screaming snake case (most common, I'd assume). "SCREAMING_SNAKE_CASE" =>
"ScreamingSnakeCase"
4. Snake case. "snake_case" => "SnakeCase"
4. Camel case. "CamelCase" => "CamelCase"
The problem is, we have to figure out which one of the situations we're dealing
with, because the conversions we'll perform will differ for every one, which is
supremely annoying. I tried to avoid having to differentiate between the cases,
but, the default implementations of {{t_generator::camelcase}} and
{{t_generator::uppercase}} make this impossible.
> Rust generation should include #![allow(non_snake_case)] or force conform to
> Rust style guidelines
> --
>
> Key: THRIFT-4529
> URL: https://issues.apache.org/jira/browse/THRIFT-4529
> Project: Thrift
> Issue Type: Improvement
> Components: Rust - Compiler
>Affects Versions: 0.11.0
>Reporter: Joshua
>Assignee: Allen George
>Priority: Minor
>
> Without this, building a project using a thrift file meant for multiple
> languages may end up with many compiler warnings similar to the following:
> {code:sh}
> warning: variant `EXAMPLE_NAME` should have a camel case name such as
> `ExampleName`
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[
https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16682470#comment-16682470
]
ASF GitHub Bot commented on THRIFT-4662:
allengeorge commented on a change in pull request #1624: THRIFT-4662: Rust
const string calls function at compile time
URL: https://github.com/apache/thrift/pull/1624#discussion_r232455479
##
File path: compiler/cpp/src/thrift/generate/t_rs_generator.cc
##
@@ -3076,6 +3086,21 @@ string t_rs_generator::to_rust_field_type_enum(t_type*
ttype) {
throw "cannot find TType for " + ttype->get_name();
}
+string t_rs_generator::to_rust_const_type(t_type* ttype, bool ordered_float) {
Review comment:
Please reorder implementation just before `to_rust_type`
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Rust const string calls function at compile time
>
>
> Key: THRIFT-4662
> URL: https://issues.apache.org/jira/browse/THRIFT-4662
> Project: Thrift
> Issue Type: Bug
> Components: Rust - Compiler
>Affects Versions: 0.11.0
> Environment: C:\Users\jake>rustup show
> Default host: x86_64-pc-windows-msvc
> stable-x86_64-pc-windows-msvc (default)
> rustc 1.30.0 (da5f414c2 2018-10-24)
>Reporter: J W
>Assignee: Allen George
>Priority: Major
>
> *For this thrift:*
> const string broker_playback_message = "mmi.developer.playback"
> *Generates:*
> // thrift -gen rs -out ../rust/thrift/src const_string.thrift
> pub const BROKER_PLAYBACK_MESSAGE: String =
> "mmi.developer.playback".to_owned();
> *Fails to compile:*
> error[E0015]: calls in constants are limited to tuple structs and tuple
> variants
> note: a limited form of compile-time function evaluation is available on a
> nightly compiler via `const fn`
> *Fix:*
> Probably want to output:
> pub const BROKER_PLAYBACK_MESSAGE: &str = "mmi.developer.playback";
>
> Looking at render_const_value() it looks like byte arrays will have the same
> issue.
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16682471#comment-16682471 ] ASF GitHub Bot commented on THRIFT-4662: allengeorge commented on issue #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624#issuecomment-437592044 @jake-ruyi Thank you for the PR. I've some minor comments, but overall this looks good. If you can address them, I'll approve and this can be merged in. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust const string calls function at compile time > > > Key: THRIFT-4662 > URL: https://issues.apache.org/jira/browse/THRIFT-4662 > Project: Thrift > Issue Type: Bug > Components: Rust - Compiler >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Major > > *For this thrift:* > const string broker_playback_message = "mmi.developer.playback" > *Generates:* > // thrift -gen rs -out ../rust/thrift/src const_string.thrift > pub const BROKER_PLAYBACK_MESSAGE: String = > "mmi.developer.playback".to_owned(); > *Fails to compile:* > error[E0015]: calls in constants are limited to tuple structs and tuple > variants > note: a limited form of compile-time function evaluation is available on a > nightly compiler via `const fn` > *Fix:* > Probably want to output: > pub const BROKER_PLAYBACK_MESSAGE: &str = "mmi.developer.playback"; > > Looking at render_const_value() it looks like byte arrays will have the same > issue. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16682469#comment-16682469 ] ASF GitHub Bot commented on THRIFT-4662: allengeorge commented on a change in pull request #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624#discussion_r232455435 ## File path: lib/rs/test/thrifts/Base_One.thrift ## @@ -37,6 +37,9 @@ const list CommonTemperatures = [300.0, 450.0] const double MealsPerDay = 2.5; +const string DefaultRecipeName = "Something" Review comment: This is incredibly minor, but, could you please choose some funny recipe names here? :) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust const string calls function at compile time > > > Key: THRIFT-4662 > URL: https://issues.apache.org/jira/browse/THRIFT-4662 > Project: Thrift > Issue Type: Bug > Components: Rust - Compiler >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Major > > *For this thrift:* > const string broker_playback_message = "mmi.developer.playback" > *Generates:* > // thrift -gen rs -out ../rust/thrift/src const_string.thrift > pub const BROKER_PLAYBACK_MESSAGE: String = > "mmi.developer.playback".to_owned(); > *Fails to compile:* > error[E0015]: calls in constants are limited to tuple structs and tuple > variants > note: a limited form of compile-time function evaluation is available on a > nightly compiler via `const fn` > *Fix:* > Probably want to output: > pub const BROKER_PLAYBACK_MESSAGE: &str = "mmi.developer.playback"; > > Looking at render_const_value() it looks like byte arrays will have the same > issue. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[
https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16682466#comment-16682466
]
ASF GitHub Bot commented on THRIFT-4662:
allengeorge commented on a change in pull request #1624: THRIFT-4662: Rust
const string calls function at compile time
URL: https://github.com/apache/thrift/pull/1624#discussion_r232455248
##
File path: compiler/cpp/src/thrift/generate/t_rs_generator.cc
##
@@ -127,7 +127,7 @@ class t_rs_generator : public t_generator {
void render_const_value_holder(const string& name, t_type* ttype,
t_const_value* tvalue);
// Write the actual const value - the right side of a const definition.
- void render_const_value(t_type* ttype, t_const_value* tvalue);
+ void render_const_value(t_type* ttype, t_const_value* tvalue, bool isowned =
true);
Review comment:
Could you change parameter name to `owned` or `is_owned`?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Rust const string calls function at compile time
>
>
> Key: THRIFT-4662
> URL: https://issues.apache.org/jira/browse/THRIFT-4662
> Project: Thrift
> Issue Type: Bug
> Components: Rust - Compiler
>Affects Versions: 0.11.0
> Environment: C:\Users\jake>rustup show
> Default host: x86_64-pc-windows-msvc
> stable-x86_64-pc-windows-msvc (default)
> rustc 1.30.0 (da5f414c2 2018-10-24)
>Reporter: J W
>Assignee: Allen George
>Priority: Major
>
> *For this thrift:*
> const string broker_playback_message = "mmi.developer.playback"
> *Generates:*
> // thrift -gen rs -out ../rust/thrift/src const_string.thrift
> pub const BROKER_PLAYBACK_MESSAGE: String =
> "mmi.developer.playback".to_owned();
> *Fails to compile:*
> error[E0015]: calls in constants are limited to tuple structs and tuple
> variants
> note: a limited form of compile-time function evaluation is available on a
> nightly compiler via `const fn`
> *Fix:*
> Probably want to output:
> pub const BROKER_PLAYBACK_MESSAGE: &str = "mmi.developer.playback";
>
> Looking at render_const_value() it looks like byte arrays will have the same
> issue.
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[
https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16682467#comment-16682467
]
ASF GitHub Bot commented on THRIFT-4662:
allengeorge commented on a change in pull request #1624: THRIFT-4662: Rust
const string calls function at compile time
URL: https://github.com/apache/thrift/pull/1624#discussion_r232455421
##
File path: compiler/cpp/src/thrift/generate/t_rs_generator.cc
##
@@ -673,15 +676,22 @@ void t_rs_generator::render_const_value_holder(const
string& name, t_type* ttype
f_gen_ << endl;
}
-void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue) {
+void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue,
bool isowned) {
Review comment:
Please rename parameter to `owned` or `is_owned`.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Rust const string calls function at compile time
>
>
> Key: THRIFT-4662
> URL: https://issues.apache.org/jira/browse/THRIFT-4662
> Project: Thrift
> Issue Type: Bug
> Components: Rust - Compiler
>Affects Versions: 0.11.0
> Environment: C:\Users\jake>rustup show
> Default host: x86_64-pc-windows-msvc
> stable-x86_64-pc-windows-msvc (default)
> rustc 1.30.0 (da5f414c2 2018-10-24)
>Reporter: J W
>Assignee: Allen George
>Priority: Major
>
> *For this thrift:*
> const string broker_playback_message = "mmi.developer.playback"
> *Generates:*
> // thrift -gen rs -out ../rust/thrift/src const_string.thrift
> pub const BROKER_PLAYBACK_MESSAGE: String =
> "mmi.developer.playback".to_owned();
> *Fails to compile:*
> error[E0015]: calls in constants are limited to tuple structs and tuple
> variants
> note: a limited form of compile-time function evaluation is available on a
> nightly compiler via `const fn`
> *Fix:*
> Probably want to output:
> pub const BROKER_PLAYBACK_MESSAGE: &str = "mmi.developer.playback";
>
> Looking at render_const_value() it looks like byte arrays will have the same
> issue.
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[ https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16682468#comment-16682468 ] ASF GitHub Bot commented on THRIFT-4662: allengeorge commented on a change in pull request #1624: THRIFT-4662: Rust const string calls function at compile time URL: https://github.com/apache/thrift/pull/1624#discussion_r232455395 ## File path: compiler/cpp/src/thrift/generate/t_rs_generator.cc ## @@ -645,8 +648,8 @@ void t_rs_generator::render_const_value(const string& name, t_type* ttype, t_con throw "cannot generate simple rust constant for " + ttype->get_name(); } - f_gen_ << "pub const " << rust_upper_case(name) << ": " << to_rust_type(ttype) << " = "; - render_const_value(ttype, tvalue); + f_gen_ << "pub const " << rust_upper_case(name) << ": " << to_rust_const_type(ttype) << " = "; + render_const_value(ttype, tvalue, false); Review comment: Interesting. This works because the call isn't recursive. Only the top-level type will be owned/static. The types in containers will all be owned because of the internal call to `to_rust_type`. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rust const string calls function at compile time > > > Key: THRIFT-4662 > URL: https://issues.apache.org/jira/browse/THRIFT-4662 > Project: Thrift > Issue Type: Bug > Components: Rust - Compiler >Affects Versions: 0.11.0 > Environment: C:\Users\jake>rustup show > Default host: x86_64-pc-windows-msvc > stable-x86_64-pc-windows-msvc (default) > rustc 1.30.0 (da5f414c2 2018-10-24) >Reporter: J W >Assignee: Allen George >Priority: Major > > *For this thrift:* > const string broker_playback_message = "mmi.developer.playback" > *Generates:* > // thrift -gen rs -out ../rust/thrift/src const_string.thrift > pub const BROKER_PLAYBACK_MESSAGE: String = > "mmi.developer.playback".to_owned(); > *Fails to compile:* > error[E0015]: calls in constants are limited to tuple structs and tuple > variants > note: a limited form of compile-time function evaluation is available on a > nightly compiler via `const fn` > *Fix:* > Probably want to output: > pub const BROKER_PLAYBACK_MESSAGE: &str = "mmi.developer.playback"; > > Looking at render_const_value() it looks like byte arrays will have the same > issue. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[
https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16682465#comment-16682465
]
ASF GitHub Bot commented on THRIFT-4662:
allengeorge commented on a change in pull request #1624: THRIFT-4662: Rust
const string calls function at compile time
URL: https://github.com/apache/thrift/pull/1624#discussion_r232455341
##
File path: compiler/cpp/src/thrift/generate/t_rs_generator.cc
##
@@ -414,6 +414,9 @@ class t_rs_generator : public t_generator {
// Return a string representing the rift `protocol::TType` given a `t_type`.
string to_rust_field_type_enum(t_type* ttype);
+ // Return a string representing the `const` rust type given a `t_type`
+ string to_rust_const_type(t_type* ttype, bool ordered_float = true);
Review comment:
Good call. Makes sense to split this out from the `to_rust_type`. Could you
please put this function declaration right before `to_rust_type`? And do the
same for implementation please? It just ensures that code that has similar
functionality is in the same part of the file.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Rust const string calls function at compile time
>
>
> Key: THRIFT-4662
> URL: https://issues.apache.org/jira/browse/THRIFT-4662
> Project: Thrift
> Issue Type: Bug
> Components: Rust - Compiler
>Affects Versions: 0.11.0
> Environment: C:\Users\jake>rustup show
> Default host: x86_64-pc-windows-msvc
> stable-x86_64-pc-windows-msvc (default)
> rustc 1.30.0 (da5f414c2 2018-10-24)
>Reporter: J W
>Assignee: Allen George
>Priority: Major
>
> *For this thrift:*
> const string broker_playback_message = "mmi.developer.playback"
> *Generates:*
> // thrift -gen rs -out ../rust/thrift/src const_string.thrift
> pub const BROKER_PLAYBACK_MESSAGE: String =
> "mmi.developer.playback".to_owned();
> *Fails to compile:*
> error[E0015]: calls in constants are limited to tuple structs and tuple
> variants
> note: a limited form of compile-time function evaluation is available on a
> nightly compiler via `const fn`
> *Fix:*
> Probably want to output:
> pub const BROKER_PLAYBACK_MESSAGE: &str = "mmi.developer.playback";
>
> Looking at render_const_value() it looks like byte arrays will have the same
> issue.
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (THRIFT-4662) Rust const string calls function at compile time
[
https://issues.apache.org/jira/browse/THRIFT-4662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16682454#comment-16682454
]
ASF GitHub Bot commented on THRIFT-4662:
allengeorge commented on a change in pull request #1624: THRIFT-4662: Rust
const string calls function at compile time
URL: https://github.com/apache/thrift/pull/1624#discussion_r232454772
##
File path: compiler/cpp/src/thrift/generate/t_rs_generator.cc
##
@@ -673,15 +676,22 @@ void t_rs_generator::render_const_value_holder(const
string& name, t_type* ttype
f_gen_ << endl;
}
-void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue) {
+void t_rs_generator::render_const_value(t_type* ttype, t_const_value* tvalue,
bool isowned) {
if (ttype->is_base_type()) {
t_base_type* tbase_type = (t_base_type*)ttype;
switch (tbase_type->get_base()) {
case t_base_type::TYPE_STRING:
if (tbase_type->is_binary()) {
-f_gen_ << "\"" << tvalue->get_string() << "\""<<
".to_owned().into_bytes()";
+if (isowned) {
Review comment:
No - the PR isn't being held up because of const binary. I just didn't have
time to look at it yet. I usually pull the branch locally and test in a
container to ensure everything works. For one thing, I'm unsure that
```
const FOO: &str = "ladeda";
```
works in rust 1.28.
EDIT. Oh, nm - I see it was [enabled in
1.17](https://rust-lang-nursery.github.io/edition-guide/rust-2018/ownership-and-lifetimes/simpler-lifetimes-in-static-and-const.html)
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Rust const string calls function at compile time
>
>
> Key: THRIFT-4662
> URL: https://issues.apache.org/jira/browse/THRIFT-4662
> Project: Thrift
> Issue Type: Bug
> Components: Rust - Compiler
>Affects Versions: 0.11.0
> Environment: C:\Users\jake>rustup show
> Default host: x86_64-pc-windows-msvc
> stable-x86_64-pc-windows-msvc (default)
> rustc 1.30.0 (da5f414c2 2018-10-24)
>Reporter: J W
>Assignee: Allen George
>Priority: Major
>
> *For this thrift:*
> const string broker_playback_message = "mmi.developer.playback"
> *Generates:*
> // thrift -gen rs -out ../rust/thrift/src const_string.thrift
> pub const BROKER_PLAYBACK_MESSAGE: String =
> "mmi.developer.playback".to_owned();
> *Fails to compile:*
> error[E0015]: calls in constants are limited to tuple structs and tuple
> variants
> note: a limited form of compile-time function evaluation is available on a
> nightly compiler via `const fn`
> *Fix:*
> Probably want to output:
> pub const BROKER_PLAYBACK_MESSAGE: &str = "mmi.developer.playback";
>
> Looking at render_const_value() it looks like byte arrays will have the same
> issue.
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
