Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #530 +/- ##
=======================================
Coverage 84.20% 84.20%
=======================================
Files 27 27
Lines 2747 2747
=======================================
Hits 2313 2313
Misses 391 391
Partials 43 43 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| ###### Anonymous Nested Enum | ||
|
|
||
| Anonymous nested enums are converted to inline enum constants within the parent struct context, with the enum values defaulting to `c.Int` type. | ||
|
|
||
| ```c | ||
| typedef struct Config { | ||
| int version; | ||
| enum { | ||
| MODE_DEBUG = 0, | ||
| MODE_RELEASE = 1 | ||
| } mode; | ||
| } Config; | ||
| ``` | ||
|
|
||
| **Generated Go code**: | ||
| ```go | ||
| type Config struct { | ||
| Version c.Int | ||
| Mode c.Int | ||
| } | ||
|
|
||
| const ( | ||
| MODE_DEBUG c.Int = 0 | ||
| MODE_RELEASE c.Int = 1 | ||
| ) | ||
| ``` |
There was a problem hiding this comment.
I think this binding actually loss some semantics.
in c,we can know the mode field at struct Config only support twe method for input (eg MODE_DEBUG , MODE_RELEASE ), but this llgo binding lost this context , this Config type will mean support every int 's input.will cause confuse and not safe operate.
so i think the better way to keep the context is like the anonmousy struct
Lines 155 to 175 in 7abb9e8
but unfortunately go have not enum,so i think here need a type to keep this context
There was a problem hiding this comment.
Should not add a type, because in common case, we don't use a type for an anonymous enum.
We should respect the source, not add an Intermediate type. If we add an Intermediate type, that will cause user confused instead of the words you said.
For example,
struct Config {
int version;
enum {
MODE_DEBUG = 0,
MODE_RELEASE = 1
} mode;
};in C, we just use it like,
struct Config cfg;
cfg.mode = MODE_DEBUG;If we add a type, just we call it NestedMode,
type Config struct {
version int
mode NestedMode
}
type NestedMode c.Int
const (
MODE_DEBUG NestedMode = 0,
MODE_RELEASE NestedMode = 1
)
var cfg Config
cfg.mode = MODE_DEBUGIt looks great, but user may think it "What's NestedMode, what's that?"
If this user wrote C but gonna to rewrite using LLGo, he will assert MODE_DEBUG is c.Int not NestedMode. When he's going to use this enum, he found the type is totally wrong and might think it's a bug for llcppg, because in his mind, he only think c.Int is the correct type of MODE_DEBUG. This situation, we call it subconsciousness psychologically.
There was a problem hiding this comment.
And to add an Intermediate type, we need to consider more, like duplicated type name or how to rename it, it's meaningless that we do that "convert" thing.
There was a problem hiding this comment.
And enum value is unique, usually. The enum itself is a unique symbol, just like the type.
In rust, you can see that enum code
enum IpAddr {
V4(String),
V6(String),
}
let home = IpAddr::V4(String::from("127.0.0.1"));
let loopback = IpAddr::V6(String::from("::1"));There was a problem hiding this comment.
And this code shows that, in C, the type of a enum field is c.Int
#include <stdio.h>
struct a {
enum b {A} k;
};
int main()
{
struct a aa;
aa.k = 2;
printf("%d", aa.k);
return 0;
}Output 2, but 2 is not the member of enum b.
And sizeof(aa.k) is 4, same as c.Int
There was a problem hiding this comment.
I think for process these anonymous type need with consist process . at #449 we confirm process the anonymous func type with a intermediate type, this decision also base the same problem.
- llgo can't direct use anonymous func type.
- need throw type binding info to user.
so i think we need make them consistent.
additional gogen current not support field doc. goplus/gogen#455
There was a problem hiding this comment.
#449 hasn't been merged, so it can't be a conclusion.
There was a problem hiding this comment.
but it's also a consensus for anonymous type
There was a problem hiding this comment.
Based on the meeting discussion, both methods we discussed are expected, and we reached a consensus that llcppg ultimately aims to convert C libraries into a user-friendly interface presented as LLGo Bindings, providing as much original information as possible. However, considering the current input-output ratio, we will first implement the current design version to provide basic conversion capabilities, and then separately consider providing this unified intermediate type to make the user interface friendly.
There was a problem hiding this comment.
So i think we need note this design in the future will have a alias type is better
Related issue: #75