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

Mangle rule names in generated code to avoid naming conflicts #107

Open
david-pfx opened this issue Jan 13, 2018 · 8 comments
Open

Mangle rule names in generated code to avoid naming conflicts #107

david-pfx opened this issue Jan 13, 2018 · 8 comments

Comments

@david-pfx
Copy link

Maybe just add a prefix in the generated code, so the names never conflict with other uses (such as system identifiers, enums, functions called by rules, etc).

@otac0n
Copy link
Owner

otac0n commented Jan 18, 2018

Can you please include an example grammar?

@otac0n otac0n added the bug label Jan 18, 2018
@david-pfx
Copy link
Author

Anything where the rule name conflicts with some other use of the name.

SomeRule = v:AnotherRule { SomeRule(v) }
Tracer = SomeRule

You already handle use of reserved words with an '@' prefix. I would suggest an option

@prefix xyz

which would instead prefix all rulenames in the generated code by the prefix.

@otac0n
Copy link
Owner

otac0n commented Apr 16, 2018

I'm afraid it still isn't clear to me the situations in which this would pose an issue, and this example doesn't really provide me with enough context.

Internally, the parser qualifies all references to rules with the this specifier, so I don't understand where the conflict arises.

Would you be willing to provide a non-trivial example where this poses a problem?

@david-pfx
Copy link
Author

The problem only arises when the parser class is extended by adding custom methods, as per the Wiki. Rule names and custom methods share the same namespace, and this qualification doesn't help.

Do you have any pre-built samples using custom methods, that I could add to? I think the following would reproduce the problem:

@members
{
    private bool IsCharAwesome(string c)
    {
        return c == "a";
    }
}

IsCharAwesome = c:. &{ this.IsCharAwesome(c) }

I use a lot of custom methods and quite often run into clashes. So I'm suggesting an optional prefix on rule names so they don't conflict with custom methods.

@otac0n
Copy link
Owner

otac0n commented Apr 26, 2018

Typically, I recommend using lowercase rule names and uppercase method names.

@david-pfx
Copy link
Author

A casing convention is a good idea if you know about the problem before you start. Which of course I didn't. Still it's your call: fix it or document it.

@otac0n
Copy link
Owner

otac0n commented Apr 27, 2018

I would like to support C# keywords for rule names as well, so since I need to prefix them with "@", I might as well fix this at the same time.

How would you expect this to behave if the rule were marked with "-public" or "-export"?

I think the "-export" option should continue to export the rule name as it is in the grammar, and I'm not super excited by the prospect of exposing these prefixes with the "-publc" flag, which is what would happen with the least effort. For context, public is meant to allow your parser to expose more than one entry point, whereas export is intended to allow modularizing/extending your grammar. In both cases, the original rule name makes the most sense to me.

@david-pfx
Copy link
Author

I think that's probably right, although it's hard to see all the consequences. I've used a grammar with an extra entry point (for recovery after error) and it made sense to specify exactly the intended name, but my grammars are not big enough to need splitting up. Just so long as there are no breaking changes...

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

2 participants