update linenoise

[krux02]

just a new verison of linenoise. Nothing explicit that gets fixed.

[Araq]

Please test this still works with the C++ target. This "extern C" plus the fact that it uses the C++ keyword "new" now make me nervous.

[krux02]

Ok I get what you mean. I could fix what you also did in the original file, but I simply think it is bad practice, that every time this c file needs to be updated, I need to reapply c++ compatibility patches. wouldn't it be possible to just add the c file to the project as a c file, no matter, what the compiler target is?

To name it with a line, I think the problem is, that a c (not c++) file is included here:

Nim/lib/wrappers/linenoise/linenoise.nim

Line 18 in 078c58b

{.emit: staticRead"clinenoise.c".}

When Nim would just link against that c file, it should also be compatible with the LLVM backend (in general, not necessary speaking that the current implementation can handle it)

[Araq]

Ok I get what you mean. I could fix what you also did in the original file, but I simply think it is bad practice, that every time this c file needs to be updated, I need to reapply c++ compatibility patches.

Well "every time" is as rare as we want it to be. Your patch for instance doesn't even fix a single bug. And this patch should make it to linenoise because it's generally useful to have C++ compat.

[krux02]

Yes maybe this wrapper is updated rarely, but I don't think that this alone justifies a pattern that makes future maintenance cumbersome.

And I do disagree, that these changes should be merged into linenoise. Linenoise is already C++ compatible. The header can be compiled by both compilers, the c file can be compiled by a C compiter. C++ and C object files can the be linked together without problems. I don't see why the linenoise developer would see any reason to also make his c file c++ compatible, when it just doesn't need to be.

I honestly think the problem is that you include explicitly C source file, and then want it to be valid c++ code. And that is not an assumption you can ever safely do. Even if you could do it in this case, it could teach other people to do the same thing, and it would introduce problems in their code. No I think this should be solved in a clean way, that could also be used as an inspiration to write other wrappers for C libraries.

I made a change, on how the c library in included in the project here:
https://github.com/nim-lang/Nim/pull/5357/files#diff-df3998ce62495dbb7b1720dcb75deb88R17

The idea is, that the compiler should compile linenoise as as a c file, no matter what the target language is defined to be. But apparently that is not how the compile pragma works. It tries to compile the C file as the target language, even though it is a C file, and should be compiled as a c file.

This brings up a question I have in mind right now. How do you use C library and a C++ library (with individual wrappers) at the same time in the same project? All I know is how to declare the target language globally as a compiler flag for the project, but I don't know how to control it for individual modules.

[Araq]

Yes maybe this wrapper is updated rarely, but I don't think that this alone justifies a pattern that makes future maintenance cumbersome.

No, it doesn't justify a pattern. Usually we don't include C code at all anyway.

[Araq]

I don't see why the linenoise developer would see any reason to also make his c file c++ compatible, when it just doesn't need to be.

But it needs to be. For Nim, for better C++ interop (does the C code work with C++ exceptions?) in general. Why do you think Lua compiles as C++ out of the box if there is no merit in this approach?

[krux02]

But it needs to be. For Nim, for better C++ interop (does the C code work with C++ exceptions?) in general. Why do you think Lua compiles as C++ out of the box if there is no merit in this approach?

I don't fully understand this. A C library can't throw exceptions, and it can't catch exceptions no matter how I compile it. So what do you mean with "does the C code work with c++ exceptions?". And I had no Idea that Lua compiles as C++ out of the box, mostly because I never compiled lua, or actually used it in something bigger than a toy project. So it would be super nice from you when tell me.

No, it doesn't justify a pattern. Usually we don't include C code at all anyway.

Yes and my argument is, that if we actually have C code included, it should also be compiled with a C compiler, and the currently configured backend compiler. Just because it is C code, and not backend compatible code. It would also allow compatibility with the LLVM backend. An emit statement is definitively not LLVM compatible.

[Araq]

So leave it as it is now and fix the compiler so that

a) foo.c is compiled with gcc, not with g++. for other C compilers that should work out of the box.
b) the path in .compile is interpreted relatively to the current module.

[krux02]

I rebased on the current development branch, and tested if I can use linenoise with the c++ target language. Yes indeed, It does work. This should be able to merge now.