[PATCH] D10713: Let cmake infer source file language by the file extension.

Douglas Katzman dougk at google.com
Mon Jul 27 15:15:58 PDT 2015


dougk added a comment.

Here's my understanding of the history of the changes -

r214013 contained contradictory directives - on one hand, it informed cmake that the project contained ASM source, and on the other, it said that **all** files are C source.  You shouldn't need to tell cmake that '.s' is to be compiled as '.c' once you tell it that the project contains ASM source.  Because the CMakeLists.txt prior to this revision did **not** say that the project contained ASM, it would have been the case that a standalone build of compiler-rt failed to compile any '.s' file (resulting in PR20360), while a non-standalone build worked correctly because of an enable_language() command inherited from the parent scope (as far as I can tell).

r213684 (somewhat correctly) added the 'set_source_file_properties' which was removed in r213724 (incorrectly) because at that time, there was no ASM specified in the project() line.

At no point was there a project() line that said that ASM source was present without also overriding the language of all source files. So while adding the ASM into project() was correct, doing both - overriding the language *and* saying that ASM is present - was incorrect. There was a bit of flailing on account of either inability to test all combinations of build and/or misunderstanding of cmake, during which time someone concluded that all the changes were incorrect, and thus the final state was exactly as the beginning state.

I believe that the current patch is actually different and an improvement: it declares that ASM should be compiled with however cmake would normally compile ASM.


http://reviews.llvm.org/D10713







More information about the llvm-commits mailing list