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

Add ".c++" as a known C++ file extension #1192

Merged
merged 1 commit into from
Oct 14, 2019
Merged

Add ".c++" as a known C++ file extension #1192

merged 1 commit into from
Oct 14, 2019

Conversation

dsvi
Copy link
Contributor

@dsvi dsvi commented Nov 11, 2018

added *.c++ extension

added *.c++ extension
@ratzlaff
Copy link
Contributor

Is this a common extension?

@samsinsane
Copy link
Member

This was requested in #612, however, I'm fairly certain that this change won't work for gmake2 based on this snippet:

rule 'cpp'
fileExtension { ".cc", ".cpp", ".cxx", ".mm" }
buildoutputs { "$(OBJDIR)/%{premake.modules.gmake2.cpp.makeUnique(cfg, file.objname)}.o" }
buildmessage '$(notdir $<)'
buildcommands {'$(CXX) %{premake.modules.gmake2.cpp.fileFlags(cfg, file)} $(FORCE_INCLUDE) -o "$@" -MF "$(@:%.o=%.d)" -c "$<"'}

@dsvi
Copy link
Contributor Author

dsvi commented Nov 12, 2018

Is this a common extension?

Relatively. qmake/qtcreator recongnizes it as C++ file.

I'm fairly certain that this change won't work for gmake2 based on this snippet:

Nothing will, since gmake2 does not rely on the path. functions. gmake works fine with this change.

@samsinsane
Copy link
Member

Relatively. qmake/qtcreator recongnizes it as C++ file.

Do you have any examples of what uses this extension? I'm kind of curious about where this extension comes from, things seem to support it but nobody seems to use it. I find this really weird.

Nothing will, since gmake2 does not rely on the path. functions. gmake works fine with this change.

I didn't go any deeper than the rule definition, but I kind of assumed the path. functions weren't called further down. FWIW, this change will need to work in gmake2 before we can merge it.

@dsvi
Copy link
Contributor Author

dsvi commented Nov 12, 2018

Do you have any examples of what uses this extension?

https://www.google.com/search?q=.c%2B%2B+file+extension
gcc toolset recognizes it as well
https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html
Anyway, it doesn't hurt to add, does it?

FWIW, this change will need to work in gmake2 before we can merge it.

I believe the best way to achieve it, is to redesign gmake2 to rely on the detection functions from path., like gmake does.
This change as it is doesn't break anything in gmake2

@ratzlaff
Copy link
Contributor

Can you implement any new tests with the gmake and gmake2 generators for this extension?

@dsvi
Copy link
Contributor Author

dsvi commented Nov 12, 2018

Hmm...

@tdesveauxPKFX
Copy link
Contributor

Do you have any examples of what uses this extension?

https://www.google.com/search?q=.c%2B%2B+file+extension
gcc toolset recognizes it as well
https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html
Anyway, it doesn't hurt to add, does it?

FWIW, this change will need to work in gmake2 before we can merge it.

I believe the best way to achieve it, is to redesign gmake2 to rely on the detection functions from path., like gmake does.
This change as it is doesn't break anything in gmake2

Indeed, it does not hurt and recognizing the same extensions as GCC looks fine to me.
However, I agree with @samsinsane here. The gmake generator will be deprecated in favor of gmake2 in the future so we'd need to have this works for gmake2 to merge.
From a quick look, I think you only need to make changes https://github.com/premake/premake-core/blob/master/modules/gmake2/gmake2_cpp.lua#L60 and https://github.com/premake/premake-core/blob/master/modules/gmake2/gmake2_cpp.lua#L102-L113.

And as @ratzlaff mentioned, a simple unit test that confirm that the correct rule is applied would be great in case a future change break this. There don't seem to be one at the moment but it would have it's place in *_file_rules.lua file test.

@WorldofBay WorldofBay mentioned this pull request Jan 11, 2019
Copy link
Member

@starkos starkos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm approving this one. The Gmake2 issues should be addressed on their own.

@starkos starkos merged commit c3a044f into premake:master Oct 14, 2019
@starkos starkos changed the title Update path.lua Add ".c++" as a known C++ file extension Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants