Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

typedef should be defined in the current scope #346

Open
yulvil opened this issue Nov 11, 2017 · 5 comments
Open

typedef should be defined in the current scope #346

yulvil opened this issue Nov 11, 2017 · 5 comments

Comments

@yulvil
Copy link
Contributor

yulvil commented Nov 11, 2017

Typedefs are always transpile into global-scope go types. We should preserve the scope in which they were defined.

I have not seen real world example of that, and I would think it is a bad practice to reuse the same name for 2 different types. For those reasons I would label this issue as a "Low". We can fix it when we encounter the problem.

P.S. This could happen when importing a library.

double f1() {
    typedef double mynum;
    mynum d = 2.2;
    return d;
}

int f2() {
    typedef int mynum;
    mynum i = 123;
    return i;
}

int main() {
    double d = f1();
    int i = f2();
    return (int)(i + d);
}

Currently transpiles to:

package main

import "os"
import "github.com/elliotchance/c2go/noarch"

type mynum float64    // <-- global scope instead of local. "type mynum int" overwritten

func f1() float64 {
...

Ref: #272

@elliotchance
Copy link
Owner

I guess a longer term solution would be to incorporate the function name into the type. However, I agree that its a pretty silly thing to do and I have never seen it done in code either. I didn't even know you could do that until you raised the issue...

@Konstantin8105
Copy link
Contributor

Konstantin8105 commented Nov 12, 2017

Could you solve that point at globally?

Preliminary steps:

  1. Move(combine) body from function func newDeclStmt(a *ast.VarDecl, p *program.Program) to function func transpileVarDecl(p *program.Program, n *ast.VarDecl)
  2. Remove function func newDeclStmt(a *ast.VarDecl, p *program.Program)
  3. Change design of function from func transpileToNode(node ast.Node, p *program.Program) error to func transpileToNode(node ast.Node, p *program.Program) (decls []goast.Decls, err error). Added returns
  4. Change design of all function inside func transpileToNode like in step 3.
  5. Remove fully body of function func transpileDeclStmt(n *ast.DeclStmt, p *program.Program) and add...(See prototype at the end of that message).

At the end:

  • we create change transpileToNode to recursive function(It is nice)
  • all structures, enums, ... are allowable inside function.
  • If we add some features (for example, another support for enum), then we can change on transpileToNode, but not in 2 places tranpileToNode and newDeclStmt.

Nice prototype of that changes see #345. Generally, prototype is work(only one FAIL in transpiling of file sqlite3.c, but I don't have time,now, for solving).
@yulvil , Could you clarify your point of view.

@Konstantin8105
Copy link
Contributor

One more note for step 4 : change p.File.Decls to decls. (see prototype)

@elliotchance
Copy link
Owner

@Konstantin8105 I think that is too much since typedefs cannot be nested in anything - only some node types. Also there is the p which managed global state like types and structures so theres no need to change this much code.

@Konstantin8105
Copy link
Contributor

@elliotchance Yes are right, if we think only about a typedef struct. The real reason for that big changes to support enum..., typedef enum name..., typedef enum...(without name), struct name, typedef struct name... everywhere - and outside of function and inside of function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants