You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Ying-Yi Liang (JIRA)" <ji...@apache.org> on 2009/07/02 04:18:47 UTC
[jira] Created: (THRIFT-532) Implicit enum value generation is
incorrect
Implicit enum value generation is incorrect
-------------------------------------------
Key: THRIFT-532
URL: https://issues.apache.org/jira/browse/THRIFT-532
Project: Thrift
Issue Type: Bug
Components: Compiler (General)
Affects Versions: 0.2
Reporter: Ying-Yi Liang
Priority: Minor
For languages without native enum support (e.g. Java, PHP...), the Thrift compiler automatically assigns a value to enum members without explicit values. The current algorithm assumes explicit values are in ascending order, which does not always hold...
See below for a violation case:
enum ValueCollision {
TWO,
ONE = 1,
THREE
}
The implicit value of TWO collides with ONE...A simple solution is to make the smallest implicit value greater than the largest explicit value.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-532) Implicit enum value generation is
incorrect
Posted by "Ying-Yi Liang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-532?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12728329#action_12728329 ]
Ying-Yi Liang commented on THRIFT-532:
--------------------------------------
Please ignore the patch of "enum_value_less_aggressive.diff", since it is wrong. That patch does not correctly deal with this case:
enum Numberz {
ZERO,
ONE,
TWO,
MOREZERO = 0
}
The reason is that it starts assigning implicit values without anticipation of the same explicit value. Doing a pre-scan to mark down the occupied values should fix this issue. Besides, the aggressive but less backward compatible patch "enum_value.diff" is fine.
> Implicit enum value generation is incorrect
> -------------------------------------------
>
> Key: THRIFT-532
> URL: https://issues.apache.org/jira/browse/THRIFT-532
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (General)
> Affects Versions: 0.2
> Reporter: Ying-Yi Liang
> Priority: Minor
> Attachments: enum_value.diff, enum_value_less_agressive.diff
>
>
> For languages without native enum support (e.g. Java, PHP...), the Thrift compiler automatically assigns a value to enum members without explicit values. The current algorithm assumes explicit values are in ascending order, which does not always hold...
> See below for a violation case:
> enum ValueCollision {
> TWO,
> ONE = 1,
> THREE
> }
> The implicit value of TWO collides with ONE...A simple solution is to make the smallest implicit value greater than the largest explicit value.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-532) Implicit enum value generation is
incorrect
Posted by "Ying-Yi Liang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-532?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12726672#action_12726672 ]
Ying-Yi Liang commented on THRIFT-532:
--------------------------------------
The patch will definitely change the numbering of the existing constants, which means there would be some client/server semantic inconsistency if the two sides use different versions of Thrift compiler.
Another solution more compatible with the original method is to maintain an occupation list. Implicit value assignment still goes the same way as it was except that we check whether there is a value conflict before using it as the final value. Such solution still changes the numbering if there is a value conflict in the original output, but...that was incorrect...
BTW: implicit enum value assignment is basically the same in all generators. Shouldn't there be a parse-tree resolution pass dealing with these issues before handing it down to the generators?
> Implicit enum value generation is incorrect
> -------------------------------------------
>
> Key: THRIFT-532
> URL: https://issues.apache.org/jira/browse/THRIFT-532
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (General)
> Affects Versions: 0.2
> Reporter: Ying-Yi Liang
> Priority: Minor
> Attachments: enum_value.diff
>
>
> For languages without native enum support (e.g. Java, PHP...), the Thrift compiler automatically assigns a value to enum members without explicit values. The current algorithm assumes explicit values are in ascending order, which does not always hold...
> See below for a violation case:
> enum ValueCollision {
> TWO,
> ONE = 1,
> THREE
> }
> The implicit value of TWO collides with ONE...A simple solution is to make the smallest implicit value greater than the largest explicit value.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-532) Implicit enum value generation is
incorrect
Posted by "Ying-Yi Liang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-532?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Ying-Yi Liang updated THRIFT-532:
---------------------------------
Patch Info: [Patch Available]
> Implicit enum value generation is incorrect
> -------------------------------------------
>
> Key: THRIFT-532
> URL: https://issues.apache.org/jira/browse/THRIFT-532
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (General)
> Affects Versions: 0.2
> Reporter: Ying-Yi Liang
> Priority: Minor
>
> For languages without native enum support (e.g. Java, PHP...), the Thrift compiler automatically assigns a value to enum members without explicit values. The current algorithm assumes explicit values are in ascending order, which does not always hold...
> See below for a violation case:
> enum ValueCollision {
> TWO,
> ONE = 1,
> THREE
> }
> The implicit value of TWO collides with ONE...A simple solution is to make the smallest implicit value greater than the largest explicit value.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-532) Implicit enum value generation is
incorrect
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-532?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12726665#action_12726665 ]
Bryan Duxbury commented on THRIFT-532:
--------------------------------------
Hm, this is a good one. I haven't looked at your patch in detail yet, but I am wondering, beyond fixing problems, will this change the numbering of people's existing constants? If so, we must proceed with utmost caution with whatever fix we incorporate.
> Implicit enum value generation is incorrect
> -------------------------------------------
>
> Key: THRIFT-532
> URL: https://issues.apache.org/jira/browse/THRIFT-532
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (General)
> Affects Versions: 0.2
> Reporter: Ying-Yi Liang
> Priority: Minor
> Attachments: enum_value.diff
>
>
> For languages without native enum support (e.g. Java, PHP...), the Thrift compiler automatically assigns a value to enum members without explicit values. The current algorithm assumes explicit values are in ascending order, which does not always hold...
> See below for a violation case:
> enum ValueCollision {
> TWO,
> ONE = 1,
> THREE
> }
> The implicit value of TWO collides with ONE...A simple solution is to make the smallest implicit value greater than the largest explicit value.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-532) Implicit enum value generation is
incorrect
Posted by "Ying-Yi Liang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-532?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Ying-Yi Liang updated THRIFT-532:
---------------------------------
Attachment: enum_value_less_agressive.diff
The more backward compatible method. If there was no enum value conflicts in the original output, the new output after applying this patch should be the same.
Errata to the original violation case:
enum Numberz {
ONE = 2,
TWO = 1,
THREE,
} // implicit value of THREE conflicts with TWO
> Implicit enum value generation is incorrect
> -------------------------------------------
>
> Key: THRIFT-532
> URL: https://issues.apache.org/jira/browse/THRIFT-532
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (General)
> Affects Versions: 0.2
> Reporter: Ying-Yi Liang
> Priority: Minor
> Attachments: enum_value.diff, enum_value_less_agressive.diff
>
>
> For languages without native enum support (e.g. Java, PHP...), the Thrift compiler automatically assigns a value to enum members without explicit values. The current algorithm assumes explicit values are in ascending order, which does not always hold...
> See below for a violation case:
> enum ValueCollision {
> TWO,
> ONE = 1,
> THREE
> }
> The implicit value of TWO collides with ONE...A simple solution is to make the smallest implicit value greater than the largest explicit value.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-532) Implicit enum value generation is
incorrect
Posted by "Ying-Yi Liang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-532?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Ying-Yi Liang updated THRIFT-532:
---------------------------------
Attachment: enum_value.diff
The output of test/ThriftTest.thrift now looks weird...but safer..
> Implicit enum value generation is incorrect
> -------------------------------------------
>
> Key: THRIFT-532
> URL: https://issues.apache.org/jira/browse/THRIFT-532
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (General)
> Affects Versions: 0.2
> Reporter: Ying-Yi Liang
> Priority: Minor
> Attachments: enum_value.diff
>
>
> For languages without native enum support (e.g. Java, PHP...), the Thrift compiler automatically assigns a value to enum members without explicit values. The current algorithm assumes explicit values are in ascending order, which does not always hold...
> See below for a violation case:
> enum ValueCollision {
> TWO,
> ONE = 1,
> THREE
> }
> The implicit value of TWO collides with ONE...A simple solution is to make the smallest implicit value greater than the largest explicit value.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.