llama.cpp

llama : return nullptr from llama_grammar_init

#8093

Merged

This commit updates llama_grammar_init to return nullptr instead of throwing an exception.

The motivation for this is that this function is declared inside an extern "C" block and is intended/may be used from C code which will not be able to handle exceptions thrown, and results in undefined behavior.

On Windows and using MSVC the following warning is currently generated:

C:\llama.cpp\llama.cpp(13998,1): warning C4297: 'llama_grammar_init': function assumed not to throw an exception but does
C:\llama.cpp\llama.cpp(13998,1): message :__declspec(nothrow), throw(), noexcept(true), or noexcept was specified on the function

• I have read the contributing guidelines
• Self-reported review complexity:
  • Low
  • Medium
  • High

Overall this is a good change, but I'm curious how this changes the stack trace returned with left-recursive grammars. I think we should do an audit for everything that calls llama_grammar_init() to ensure that we are checking for null ptrs. Examples include sampling.cpp:31 and (less-importantly) gbnf-validator.cpp:101.

Overall I think this is a good change, though there may be some cleanup spots to do later.

I think we should do an audit for everything that calls llama_grammar_init() to ensure that we are checking for null ptrs.

Good point, I'll take a look at this! Thanks

I think we should do an audit for everything that calls llama_grammar_init() to ensure that we are checking for null ptrs.

Good point, I'll take a look at this! Thanks

Did you resolve everything you wanted to in 35d9680 or should we wait for more? If you're happy, I think you're good to click "Squash and Merge".

Did you resolve everything you wanted to in 35d9680 or should we wait for more? If you're happy, I think you're good to click "Squash and Merge".

All good from my side 👍 I can rebase and squash the commits to make it easier to merge, but I don't I have write access so I don't think I can merge.

All good from my side 👍 I can rebase and squash the commits to make it easier to merge, but I don't I have write access so I don't think I can merge.

Aah, right. Thanks for the PR!