CCpp target #513
CCpp target #513
Conversation
…ing marked in the Eclipse IDE in the C/CCpp target
… produced if flags is used
This reverts commit da58de2.
| super.setTargetConfig(context); | ||
| // Set defaults for the compiler after parsing the target properties | ||
| // of the main .lf file. | ||
| if(this.CCppMode && targetConfig.useCmake == false && targetConfig.compiler.isNullOrEmpty) { |
There was a problem hiding this comment.
Better not to repeat the same conditional twice. Cleaner to have something like:
if (targetConfig.useCmake == false && targetConfig.compiler.isNullOrEmpty) {
if (this.CCppMode) {
// ..
} else {
// ..
}
}
There was a problem hiding this comment.
Thanks, that's a great suggestion.
| @@ -549,7 +549,7 @@ class LFValidatorImpl extends AbstractLFValidator { | |||
|
|
|||
| @Check(FAST) | |||
| def checkDeadline(Deadline deadline) { | |||
| if (this.target == Target.C && | |||
| if ((this.target == Target.C || this.target == Target.CCPP) && | |||
There was a problem hiding this comment.
The fact that this has to be repeated everywhere is a bit inelegant, but it is an unavoidable consequence of the CCpp target approach. With the cmake-force-cpp proposal this would not have been the case...
There was a problem hiding this comment.
The conditions became really difficult to parse with the nested or operation. Maybe we could introduce a small method that does the check? For instance, isCTarget() or isCBasedTarget()
There was a problem hiding this comment.
That's a great suggestion. Thanks.
| @@ -21,11 +21,10 @@ | |||
| * | |||
| * @author Soroush Bateni <[email protected]> | |||
| */ | |||
| target C { | |||
| target CCpp { | |||
| cmake: true, // Only CMake is supported | |||
There was a problem hiding this comment.
Is it this exampke or the CCpp target that only supports cmake? In the latter case, do we need this line and should we even allow the cmake property in this target?
| * Change the target name to 'newTargetName'. | ||
| * For example, change C to CCpp. | ||
| */ | ||
| static def boolean changeTargetName(Resource resource, String newTargetName) { |
There was a problem hiding this comment.
This seems dangerous. I assume it is needed by the test that compile C tests as CCpp? I wouldn't expose this function here if we don't have a use-case for it. Maybe it would be better placed in the test package.
There was a problem hiding this comment.
I assume it is needed by the test that compile C tests as CCpp?
Correct.
Maybe it would be better placed in the test package.
I think semantically, it belongs to ASTUtils because it is an AST transformation after all.
I think in terms of being dangerous, there will be validation and compiler errors if a target name is changed to an incompatible language, so it might be relatively safe perhaps.
| @@ -549,7 +549,7 @@ class LFValidatorImpl extends AbstractLFValidator { | |||
|
|
|||
| @Check(FAST) | |||
| def checkDeadline(Deadline deadline) { | |||
| if (this.target == Target.C && | |||
| if ((this.target == Target.C || this.target == Target.CCPP) && | |||
There was a problem hiding this comment.
The conditions became really difficult to parse with the nested or operation. Maybe we could introduce a small method that does the check? For instance, isCTarget() or isCBasedTarget()
Co-authored-by: Marten Lohstroh <[email protected]>
…th their file and target configs instead of keeping a list of resources
Co-authored-by: Marten Lohstroh <[email protected]>
…imported .lf resources
`cmake-include` improvements
This (re)adds the CCpp target as a way to mix C/C++ code in the C target.
This feature has been documented here.