can cxx_files support a .cc extension?

[andrewray]

Currently assumes the file has a .cpp extension, though .cc is also pretty common.

[ghost]

Sure, the fields c_names/cxx_names/... comes from our jenga rules. I'm assuming they take names rather than filenames to match (modules (...)) (/cc @sliquister). However, given that there are many more possible extensions for C/C++ files, it'd probably be simplier to replace them by c_files/cxx_files and take full filenames. I'll look at adding these in jbuilder.

[rauanmayemir]

Any progress on this? Having c_names take full filenames would be really helpful as there's currently no way of including .m/.mm stubs.

[rgrinberg]

@rauanmayemir would you like to give this issue a try? It's not particularly difficult and I can give you a bit of info to get started.

[rauanmayemir]

@rgrinberg Hey, why not? I'd love to try!

[rgrinberg]

Thinking about this a little more, I think that we could consider breaking compatibility here for dune 1.0. That is, we should reject c_names and cxx_names in dune files and only allow for c_files and cxx_files.

@rauanmayemir These details regarding compatibility are not so important and you can get started without worrying about it. We'll help you polish this part if it's required for 1.0. For now you can just use add c_files and cxx_files and ignore compat (it will be easy to add at the end).

The change consists of doing 3 things:

• Renaming c_names and cxx_names. These fields (and all others) are defined in jbuild.ml. Changing this should be straight forward.

• Change the rules (in gen_rules.ml) to stop appending the default extensions. Make sure to change variable and param names along the way.

• Append the hard coded list of extensions here https://github.com/ocaml/dune/blob/master/src/action.ml#L816. I suppose it shouldn't be hard coded at all anymore given that we don't have a fixed set of extensions. But it's good enough for now. cc @dra27 for

• Bonus: Add compatibility for c_names and cxx_names in jbuild files. In jbuild files, we'll need to read the c_names and cxx_names sexp fields and then convert them to c_files and cxx_files by appending the appropriate extensions. Familiarize yourself with the Syntax.renamed_in and Syntax.since functions by reading how they're used with fields such as source_tree.

• Add tests for the feature. Have a look at c-stubs blackbox tests for inspiration. Familiarize yourself with our expect test workflow $ make test to run the tests and $ make accept-corrections to promote results. See run.t files for how to write test cases.

This is a very rough guide so don't hesitate to ask more questions.

Some open questions for our maintainers @diml @bobot @emillon @dra27 @avsm @Chris00 with c stubs experience:

• Should we apply the same changes to header files? I know that header files don't really have the same variation in extension names. But it seems like this will make sense for consistency's sake.

• Do we take this opportunity to change the meaning of :standard for c sources? I don't really like how we have the opposite default for C and OCaml sources. I think that :standard should include all C sources by default. This is more consistent and will reduce boilerplate.

[rauanmayemir]

I know that header files don't really have the same variation in extension names.

There could be .h, .hh, .hpp.

[rgrinberg]

Okay. Even more of a reason to just specify extensions everywhere consistently.

[ghost]

Replacing c_names by c_files is not compatible with changing the meaning of :standard. If we don't know the extensions for C files, we can't know what are the C files in the directory.

BTW, I'm not against adding support of c_files, but I don't think we should get rid of c_names yet.

[rgrinberg]

Jérémie Dimino <notifications@github.com> writes:

Allowing arbitrary extensions seem incompatible with changing the meaning of `:standard`. If we don't know the extensions for C files, we can't know what are the C files in the directory. BTW, I'm not against adding support of `c_files`, but I don't think we should get rid of `c_names` yet.

Hmm, good point. I suppose we could allow for globs in the specification of c_files however, but this seems complicated. I suppose we should allow both fields and take their union when both are specified. Being careful to do some error checking that we don't allow to list overlapping sources in both fields. What do you think about the :standard proposal on its own however? I think it still makes sense to reduce boilerplate here. Also, if we do end up going down the glob route, :standard could simply mean *.c. This would work nicely with env overridng standard, as it would allow people to set their default list of extensions.

[ghost]

What is the use case BTW? c_names and cxx_names are used only for C stubs for OCaml libraries. Why not simply rename the C files? It's better to use the same convention everywhere.

[rauanmayemir]

After looking into relevant code and working on this for a bit, I've come to realise that simply adding c_files might not work unless we choose one of c/cxx approaches to handle those files. (C outputs its files in the same place, Cxx follows '-o dest', MSVS specifies '-llib' flags differently, install_c_headers only accepts '.h', etc)

If we'd specify all commonly used extensions (which, I'm guessing, there are not that much of) and split the c_files behaviour to use Gen.build_c_file/build_cxx_file, then it would be simpler to add those extension options to each appropriate c_names/cxx_names stanza (i.e automatically choose extension based on what we found in the directory).

Personally, I solved my problem by just renaming stubs.m to stubs.c and adding -x objective-c to c_flags. 😅
I'd argue that anything more complicated would be better solved with a custom rule.

Unless I'm missing something here?

[rgrinberg]

We can also add an objc_names field if it's a common enough use case. -x objective-c is clang only I assume?

My thinking is that renaming here is quite pesky, because now your editor won't know it's objective C anymore for example.

[emillon]

As a suggestion, would it make sense to make these extensions configurable in the dune file ? There's some variety, especially in the C++ world, and I don't think we want to ask users to rename their files. This gives a nice escape hatch for "the C compiler will know how to handle this".

[rauanmayemir]

@emillon I like this much better than adding a separate field per lang/'dialect'.

@diml How do you like c_extension and cxx_extension fields? With .c and .cpp defaults changes will be minimal.

It will also be sort of consistent with c_name being file's name and c_extension - its extension. (It could be named differently to clearly indicate it's a file extension, not a 'plugin')

[rauanmayemir]

I'm not very familiar with sexpr's, but would this fly?

(c_names (myc_stubs (mym_stubs m)))

[ghost]

Yes, that makes sense. To make this more future proof, let's call these c_extensions and cxx_extensions. Additionally, we should allow these to be set once and for all for a given project via the env stanza, so that one may write in their dune-project file:

(env
 (_ (cxx_extensions .m .c++ .cpp)))

[emillon]

I'm surprised that dune knows about the differences between C and C++ (I'd expect it to just pass these to the system compiler). What kind of handling does it do? (because there might be something to do to support Objective-C).

[rgrinberg]

I'm fine with either feature, but IMO it's a bit weird to make the default extension customizable rather than simply let users put full filenames. This seems to me like adding more code to fix a problem that we've created ourselves in the first place.

[ghost]

Make the extensions customizable would allow to make :standard for c_names be all the C files for instance. Although, I don't know how important that is.

@emillon the flags are a bit different between the two. See build_cxx_file in gen_rules.ml for instance of cxx_flags in super_context.ml. But, yh maybe we need a more generic system allowing arbitrary foreign code and where C and C++ are just two particular instances.

[rgrinberg]

Make the extensions customizable would allow to make :standard for c_names be all the C files for instance. Although, I don't know how important that is.

I think this can also be done by making :standard be a glob: *.c and allowing globs in the osl for these fields.

[ghost]

The question is more what happens when there is no c_names field at all. If the default is all c files, then you don't even need a c_names field.

[rgrinberg]

Yeah, the default would be all files with a .c extension.

[ghost]

yes, but then we have the same question for C++ files. There are many different questions in this PR, I think at this point someone should write a concrete proposal that we can reason about. For instance even if we allow more extensions, it doesn't solve the problem regarding objective-C files that might require specific treatment.

[rauanmayemir]

The question is more what happens when there is no c_names field at all. If the default is all c files, then you don't even need a c_names field.

I don't quite understand this part. Does dune automatically scan the directory and compile whatever C files it finds in current directory? Or you want it to be that way?

[rgrinberg]

I don't quite understand this part. Does dune automatically scan the directory and compile whatever C files it finds in current directory? Or you want it to be that way?

The current situation is that dune requires the stubs to be listed. What we'd like is a situation that is symmetric with the modules field. Specifying no sources would mean dune automatically discover them using some default extensions.

[rauanmayemir]

Sounds great. I'll start with basic c_extensions/cxx_extensions fields and ask for guidance from there.

[rgrinberg]

I think I agree with @diml that we need a concrete proposal before committing to anything. You could help us by clarifying what kind of special treatment objective C needs here. I'm not really convinced that adding fields for default extensions is a good approach (see my previous comments).

Further more, we should perhaps think about other languages as well here. I recall @Chris00 wanting some basic fortran support. Are there are other languages that are useful for adding fast support like this?

Finally, what about jsoo? If we have auto discovery then it should be consistently be applied for jsoo stubs as well.

[rauanmayemir]

My use case for Objective-C involves writing bindings for Cocoa to draw UI on OCaml side. It's a library that gets linked into a complete 'exe.o' and embedded as a dependency(along with libasmrun.a) into an xcode project target. (in main.m I'm calling caml_main and caml_callback to call a function from my 'exe.o')

If I were to run it outside of xcodebuild flow, all I'd need to do is to add -framework Cocoa to a linking step (or -framework Foundation if it didn't involve UI). Since I'm not doing that, I don't need any 'special treatment'.

So, practically, there are no problems except for dune not being able to understand '.m' extension out of the box.

[rgrinberg]

@rauanmayemir Why can't you write a main.c file? It has to be a C source, or does CC understand objective C on OSX?

Anyway, I've went ahead and just fixed the original issue in #2195. I'm thinking that having all this generality for file extensions is a bit of overkill and we have bigger fish to fry. However, if you have an idea of how to better support this, feel free to come up with a proposal.

[rauanmayemir]

I have to specify clang option to choose objc for .c files, plus there’s an extra hassle making an editor treat .c files as ObjC.

It’s not a big deal and I agree that at the moment dune has other important things to deal with.

We can circle back to this at some point in future.