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

Invalid identifiers from build macros make it to native compilation #5002

Closed
Gama11 opened this issue Mar 31, 2016 · 7 comments
Closed

Invalid identifiers from build macros make it to native compilation #5002

Gama11 opened this issue Mar 31, 2016 · 7 comments
Assignees

Comments

@Gama11
Copy link
Member

Gama11 commented Mar 31, 2016

#if macro
import haxe.macro.Context;
import haxe.macro.Expr;
#else
@:build(Macro.invalidIdentifier())
#end
class Main {
    static function main() {}
}

class Macro 
{
    #if macro
    macro public static function invalidIdentifier():Array<Field>
    {
        var fields = Context.getBuildFields();
        fields.push({
            name: "0",
            doc: null,
            access: [Access.APublic, Access.AStatic],
            kind: FieldType.FVar(macro:String, null),
            pos: Context.currentPos()
        });
        return fields;
    }
    #end
}

Fails during cpp compilation:

include\Main.h(37): error C2059: syntax error: '-'
include\Main.h(37): error C2238: unexpected token(s) preceding ';'

Cs compilation:

src\Main.cs(30,23): error CS1519: Ung�ltiges Token '-' in Klasse, Struktur oder Schnittstellenmemberdeklaration.
Compilation error
Native compilation failed
Error: Build failed
Build halted with errors.

...and probably every other target with a native compilation step.

Not a regression.

@Simn
Copy link
Member

Simn commented Mar 31, 2016

I'm not sure if we should detect that, you could cause the same problems using @:native native. I'm inclined to file this under "Don't do stupid things".

@Gama11
Copy link
Member Author

Gama11 commented Mar 31, 2016

It's not like there's a valid use case for this though, right? It seems strange that macro-generated identifiers wouldn't go through the same checks as regular ones.

Not checking this leads to a lot of target-specific, cryptic errors that could be detected much earlier.

@Simn
Copy link
Member

Simn commented Sep 23, 2017

I still think this should be filed under "don't do stupid things". Otherwise we would have to add "is valid field name" functions for every target, and then we would probably need the same for type names, var names and whatnot. So yeah, don't do stupid things. :)

@Simn Simn closed this as completed Sep 23, 2017
@Gama11
Copy link
Member Author

Gama11 commented Sep 23, 2017

Meh, I really dislike that you can get failures during native compilation (or even at runtime on some targets) because of this...

we would have to add "is valid field name" functions for every target

Huh? Why not just require it to be a valid Haxe identifier as usual- how is that target-specific?

@Simn Simn reopened this Sep 30, 2017
@Simn Simn modified the milestones: Release 4.0, Backlog Apr 17, 2018
@Simn Simn self-assigned this Apr 19, 2018
@Gama11
Copy link
Member Author

Gama11 commented Aug 27, 2019

Does it make sense to do this check during decoding in macroApi?

@Simn
Copy link
Member

Simn commented Aug 27, 2019

I'm still not convinced this should be checked at all, but if anything this should be handled during typing. Maybe all the places where we check for that leading $.

@Gama11
Copy link
Member Author

Gama11 commented Aug 27, 2019

Well.. to me it seems like a pretty big hole in the type safety of the macro API. In a way it's almost as if identifiers are dynamically typed. Effectively this makes macro-generated code less type safe than regular Haxe code, which is a strange disparity.

Edit: maybe "type" safe is the wrong term here.. anyway you get the point. :)

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

No branches or pull requests

2 participants