dlang-community / Pegged

A Parsing Expression Grammar (PEG) module, using the D programming language.
534 stars 66 forks source link

leftRecursionStoppers() imports pegged.dev.introspection; breaks build #172

Closed Llammissar closed 8 years ago

Llammissar commented 8 years ago

Using the following code:

import pegged.grammar;

void main()
{
        asModule("arithmetic","arithmetic",
        "Arithmetic:
            Expr     <- Factor AddExpr*
            AddExpr  <- ('+'/'-') Factor
            Factor   <- Primary MulExpr*
            MulExpr  <- ('*'/'/') Primary
            Primary  <- Parens / Number / Variable / '-' Primary

            Parens   <- '(' Expr ')'
            Number   <~ [0-9]+
            Variable <- identifier
        ");
}

Compilation seems to be fine, but the link fails:

[0s] wyatt@Yue ~/Source_Trees/Pegged $ dmd arithmeticPeg.d -L-L. -L-lpegged
arithmeticPeg.o: In function `_D6pegged7grammar44__T7grammarVE6pegged7grammar11Memoizationi1Z7grammarFAyaZ21leftRecursionStoppersMFNaZAAya':
arithmeticPeg.d:(.text._D6pegged7grammar44__T7grammarVE6pegged7grammar11Memoizationi1Z7grammarFAyaZ21leftRecursionStoppersMFNaZAAya+0x92): undefined reference to `_D6pegged3dev13introspection8ruleInfoFNaS6pegged3peg9ParseTreeZHAyaS6pegged3dev13introspection8RuleInfo'
collect2: error: ld returned 1 exit status
--- errorlevel 1

If I add the module in question explicitly like so, it works: dmd arithmeticPeg.d pegged/dev/introspection.d -L-L. -L-lpegged

...As do adding pegged/dev/introspection.d to the Pegged makefile and whatever magic is in rdmd arithmeticPeg.d.

My impression from the README is pegged/dev files shouldn't be part of the release build, so I'm not sure what's the right thing to do here.

veelo commented 8 years ago

Hi,

Thanks reporting. Recently, Pegged has started using the introspection module, in order to deal with any left-recursion in a grammar. So we should possibly change things a bit: either move introspection.d out of the dev dir or change the README and Makefile.

However, I have discovered that the analysis of left-recursion is sub-optimal for memoizing parsers, and I have to find a way to improve the introspection. I am not yet sure if I will keep using the existing module in dev or write a tailored analysis procedure.

I have not encountered the linking problem myself as I have been using

rdmd -I"path_to\Pegged" my_file.d

so all sources including relevant files from Pegged are compiled at once.

I suppose it will be best to adjust the README, but things might change in the not-so-distant future.

Best, Bastiaan.

veelo commented 8 years ago

Philippe,

In case I decide to continue using introspection.d, do you want it moved out of the dev dir?

Bastiaan.

PhilippeSigaud commented 8 years ago

Philippe,

In case I decide to continue using introspection.d, do you want it moved out of the dev dir?

Yes, that is cleaner that way. Could you move it, please?

veelo commented 8 years ago

Done: #173.

PhilippeSigaud commented 8 years ago

On Fri, Jan 29, 2016 at 10:31 PM, Bastiaan Veelo notifications@github.com wrote:

Done: #173 https://github.com/PhilippeSigaud/Pegged/pull/173.

Thanks!

PhilippeSigaud commented 8 years ago

There, I merged it. Thanks a lot, Bastiaan.

Wyatt, if you fetch it anew, does it work OK now?

veelo commented 8 years ago

Welcome! Note I didn't even look at the Makefile. Should it be mentioned there explicitly to resolve the link error?

PhilippeSigaud commented 8 years ago

Sorry, what should be mentioned in the makefile?

veelo commented 8 years ago

I mean that I am unsure whether this resolves Whyatt's link problem.

Llammissar commented 8 years ago

Ah, sorry, weekend came and I didn't check my mail at all.

Yeah, the move was probably a good idea, but putting it in the makefile was necessary, or you'd have to build/link the introspection module explicitly; i.e. the library was incomplete. Thanks for addressing this!

PhilippeSigaud commented 8 years ago

Can I close the issue?

veelo commented 8 years ago

I think it's closed already. Isn't it?

PhilippeSigaud commented 8 years ago

On Mon, Feb 1, 2016 at 9:44 PM, Bastiaan Veelo notifications@github.com wrote:

I think it's closed already. Isn't it?

Indeed it is. Move along, nothing to see here. :-)