Skip to content

Commit

Permalink
Merge master into transient-federates and Align reactor-c.
Browse files Browse the repository at this point in the history
  • Loading branch information
ChadliaJerad committed May 4, 2023
1 parent d7791fa commit e8349ce
Show file tree
Hide file tree
Showing 25 changed files with 344 additions and 268 deletions.
2 changes: 0 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ jobs:
uses: lf-lang/benchmarks-lingua-franca/.github/workflows/benchmark-tests.yml@main
with:
target: 'C'
benchmarks-ref: main
compiler-ref: master
needs: cancel

# Run language server tests.
Expand Down
15 changes: 1 addition & 14 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ Test categories are declared in the [TestCategory enum in TestRegistry.java](htt

### Workflow
All code contributions must go through a pull request (PR), pass all tests run in CI, and get an approving review before it is merged. Pushing to the `master` branch is restricted. All code review is conducted using the Github review system on PRs. Before requesting a code review, ensure that you have:
- applied the [code formatter](#code-style-and-formatting);
- [documented](#code-style-and-formatting) your code;
- documented your code;
- written [tests](#writing-tests) that cover your code; and
- accompanied any remaining `TODO`s or `FIXME`s with a link to an active [issue](#reporting-issues).

Expand Down Expand Up @@ -73,18 +72,6 @@ Perform merges to bring your feature branch up-to-date with master locally (do n
#### Addressing reviews
To address feedback from code review, implement changes and push to your feature branch. If you are certain that a reviewer's concern has been addressed by your new changes, hit the `Resolve conversation` button. If you are unsure, ask for follow up in the conversation and let the reviewer determine whether the feedback was addressed accordingly.

### Code style and formatting
The Lingua Franca compiler is implemented in Java and Kotlin. The overarching advice is to use each language's most widely used idioms and conventions, which are fortunately very similar. The code base is shipped with a [Spotless](https://github.com/diffplug/spotless) configuration to check and enforce style compliance. Lingua Franca code (e.g., tests) in this repository is also automatically formatted via Spotless.

- To check that modified files are formatted correctly, run:
```
./gradlew spotlessCheck
```
- To apply the changes recommended by the formatter, run:
```
./gradlew spotlessApply
```

#### General guidelines
- _Do not copy-paste code._ If you want to reuse code, factor it out into a method and call it.
- _Keep methods concise._ As a rule of thumb, a method should fit on your screen so that it can be read without scrolling. We impose no hard limit on method length, but anything above 40 lines should be considered for breaking up.
Expand Down
6 changes: 0 additions & 6 deletions org.lflang/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,6 @@ task buildAll() {
mainClassName = 'org.lflang.cli.Lfc'
}

// define buildLfc as an alias to buildAll
task buildLfc {
dependsOn buildAll
doFirst({{logger.warn("The buildLfc task is deprecated! Please use buildAll instead.")}})
}

task jarCliTools(type: ShadowJar) {
manifest {
attributes('Main-Class': 'org.lflang.cli.Lfc')
Expand Down
19 changes: 0 additions & 19 deletions org.lflang/src/lib/ts/babel.config.js

This file was deleted.

24 changes: 6 additions & 18 deletions org.lflang/src/lib/ts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,35 +3,23 @@
"type": "commonjs",
"dependencies": {
"@lf-lang/reactor-ts": "git://github.com/lf-lang/reactor-ts.git#master",
"@babel/cli": "^7.8.4",
"@babel/core": "^7.8.7",
"@babel/node": "^7.8.7",
"@babel/plugin-proposal-class-properties": "^7.8.3",
"@babel/plugin-proposal-object-rest-spread": "^7.8.3",
"@babel/plugin-proposal-optional-chaining": "^7.8.3",
"@babel/plugin-transform-modules-commonjs": "^7.8.3",
"@babel/preset-env": "^7.8.7",
"@babel/preset-typescript": "^7.8.3",
"command-line-usage": "^6.1.0",
"command-line-args": "^5.1.1",
"rimraf": "^3.0.2"
"command-line-usage": "^6.1.3"
},
"devDependencies": {
"@types/command-line-args": "^5.2.0",
"@types/command-line-usage": "^5.0.2",
"@types/jest": "^27.0.0",
"@types/microtime": "^2.1.0",
"@types/node": "^16.3.3",
"@types/google-protobuf": "^3.7.4",
"@types/uuid": "^8.3.4",
"@types/microtime": "^2.1.0",
"@types/node": "^18.14.2",
"@typescript-eslint/eslint-plugin": "5.33.0",
"@typescript-eslint/parser": "^5.8.1",
"eslint": "^8.5.0",
"typescript": "~4.8.2",
"ts-protoc-gen": "^0.12.0"
"ts-protoc-gen": "^0.15.0",
"rimraf": "^3.0.2"
},
"scripts": {
"check-types": "tsc",
"build": "npx rimraf dist && npx babel src --out-dir dist --extensions .ts,.js"
"build": "npx rimraf dist && npx tsc --outDir dist"
}
}
7 changes: 5 additions & 2 deletions org.lflang/src/lib/ts/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
{
"compilerOptions": {
"allowJs": true,
"noEmit": true,
"target": "esnext",
"module": "CommonJS",
"types": ["node", "@lf-lang/reactor-ts", "ulog", "microtime", "command-line-args", "command-line-usage"],
"typeRoots": ["./node_modules/@types/", "./node_modules/@lf-lang/reactor-ts/src/core/@types/"],
"esModuleInterop": true,
"isolatedModules": true,
"lib": ["esnext", "dom"],
"sourceMap": true,
"strict": true,
"moduleResolution": "node",
"skipLibCheck": true,
"strictBindCallApply": true,
"strictNullChecks": true,
"strictFunctionTypes": true,
"typeRoots": ["./node_modules/@types/", "./node_modules/@lf-lang/reactor-ts/src/core/@types/"]
"noImplicitThis": true,
"outDir": "dist"
},
"include": [
"src/**/*"
Expand Down
4 changes: 1 addition & 3 deletions org.lflang/src/org/lflang/FileConfig.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.lflang;

import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
Expand All @@ -15,7 +14,6 @@
import org.eclipse.xtext.generator.IFileSystemAccess2;


import org.lflang.federated.generator.FedFileConfig;
import org.lflang.generator.GeneratorUtils;
import org.lflang.util.FileUtil;
import org.lflang.util.LFCommand;
Expand Down Expand Up @@ -308,7 +306,7 @@ public LFCommand getCommand() {
* Return the extension used for binaries on the platform on which compilation takes place.
*/
protected String getExecutableExtension() {
return (GeneratorUtils.isHostWindows() ? ".exe" : "");
return GeneratorUtils.isHostWindows() ? ".exe" : "";
}

/**
Expand Down
16 changes: 1 addition & 15 deletions org.lflang/src/org/lflang/TargetConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -172,13 +172,6 @@ public TargetConfig(
*/
public List<String> cmakeIncludes = new ArrayList<>();

/**
* List of cmake-includes from the cmake-include target property with no path info.
* Useful for copying them to remote machines. This is needed because
* target cmake-includes can be resources with resource paths.
*/
public List<String> cmakeIncludesWithoutPath = new ArrayList<>();

/**
* The compiler to invoke, unless a build command has been specified.
*/
Expand Down Expand Up @@ -237,14 +230,7 @@ public TargetConfig(
/**
* List of files to be copied to src-gen.
*/
public List<String> fileNames = new ArrayList<>();

/**
* List of file names from the files target property with no path info.
* Useful for copying them to remote machines. This is needed because
* target files can be resources with resource paths.
*/
public List<String> filesNamesWithoutPath = new ArrayList<>();
public List<String> files = new ArrayList<>();

/**
* If true, configure the execution environment to keep executing if there
Expand Down
6 changes: 3 additions & 3 deletions org.lflang/src/org/lflang/TargetProperty.java
Original file line number Diff line number Diff line change
Expand Up @@ -276,16 +276,16 @@ public enum TargetProperty {
* processed by the code generator.
*/
FILES("files", UnionType.FILE_OR_FILE_ARRAY, List.of(Target.C, Target.CCPP, Target.Python),
(config) -> ASTUtils.toElement(config.fileNames),
(config) -> ASTUtils.toElement(config.files),
(config, value, err) -> {
config.fileNames = ASTUtils.elementToListOfStrings(value);
config.files = ASTUtils.elementToListOfStrings(value);
},
// FIXME: This merging of lists is potentially dangerous since
// the incoming list of files can belong to a .lf file that is
// located in a different location, and keeping just filename
// strings like this without absolute paths is incorrect.
(config, value, err) -> {
config.fileNames.addAll(ASTUtils.elementToListOfStrings(value));
config.files.addAll(ASTUtils.elementToListOfStrings(value));
}),

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ public String generatePreamble(
includes.pr("#include \"core/federated/federate.h\"");
includes.pr("#include \"core/federated/net_common.h\"");
includes.pr("#include \"core/federated/net_util.h\"");
includes.pr("#include \"core/federated/clock-sync.h\"");
includes.pr("#include \"core/threaded/reactor_threaded.h\"");
includes.pr("#include \"core/utils/util.h\"");
includes.pr("extern federate_instance_t _fed;");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,11 @@ public static void generateCMakeInclude(
CodeBuilder cmakeIncludeCode = new CodeBuilder();

cmakeIncludeCode.pr(generateSerializationCMakeExtension(federate));
cmakeIncludeCode.pr(
"add_compile_definitions(LF_SOURCE_DIRECTORY=\""
+ fileConfig.srcPath
+ "\")"
);

try (var srcWriter = Files.newBufferedWriter(cmakeIncludePath)) {
srcWriter.write(cmakeIncludeCode.getCode());
Expand Down
18 changes: 12 additions & 6 deletions org.lflang/src/org/lflang/federated/generator/FedFileConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;

Expand Down Expand Up @@ -110,7 +111,7 @@ public void doClean() throws IOException {
*/
public void relativizePaths(FedTargetConfig targetConfig) {
relativizePathList(targetConfig.protoFiles);
relativizePathList(targetConfig.fileNames);
relativizePathList(targetConfig.files);
relativizePathList(targetConfig.cmakeIncludes);
}

Expand All @@ -120,17 +121,22 @@ public void relativizePaths(FedTargetConfig targetConfig) {
*/
private void relativizePathList(List<String> paths) {
List<String> tempList = new ArrayList<>();
paths.forEach(f -> tempList.add(relativizePath(f)));
paths.forEach(f -> tempList.add(relativizePath(Paths.get(f))));
paths.clear();
paths.addAll(tempList);
}

/**
* Relativize a single path.
* Relativize a single path, but only if it points to a local resource in the project (i.e., not
* on the class path).
* @param path The path to relativize.
*/
private String relativizePath(String path) {
Path resolvedPath = this.srcPath.resolve(path).toAbsolutePath();
return this.getSrcPath().relativize(resolvedPath).toString();
private String relativizePath(Path path) {
if (FileUtil.findInPackage(path, this) == null) {
return String.valueOf(path);
} else {
Path resolvedPath = this.srcPath.resolve(path).toAbsolutePath();
return this.getSrcPath().relativize(resolvedPath).toString();
}
}
}
5 changes: 4 additions & 1 deletion org.lflang/src/org/lflang/generator/GeneratorBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@

import org.lflang.lf.Reaction;
import org.lflang.lf.Reactor;
import org.lflang.util.FileUtil;
import org.lflang.validation.AbstractLFValidator;

import com.google.common.base.Objects;
Expand Down Expand Up @@ -348,7 +349,9 @@ protected void setReactorsAndInstantiationGraph(LFGeneratorContext.Mode mode) {
* @param targetConfig The targetConfig to read the `files` from.
* @param fileConfig The fileConfig used to make the copy and resolve paths.
*/
protected void copyUserFiles(TargetConfig targetConfig, FileConfig fileConfig) {}
protected void copyUserFiles(TargetConfig targetConfig, FileConfig fileConfig) {
FileUtil.copyFiles(targetConfig.files, this.context.getFileConfig().getSrcGenPath(), fileConfig, errorReporter);
}

/**
* Return true if errors occurred in the last call to doGenerate().
Expand Down
5 changes: 3 additions & 2 deletions org.lflang/src/org/lflang/generator/c/CCmakeGenerator.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

package org.lflang.generator.c;

import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -313,8 +314,8 @@ CodeBuilder generateCMakeCode(
cMakeCode.newLine();

// Add the include file
for (String includeFile : targetConfig.cmakeIncludesWithoutPath) {
cMakeCode.pr("include(\""+includeFile+"\")");
for (String includeFile : targetConfig.cmakeIncludes) {
cMakeCode.pr("include(\""+ Path.of(includeFile).getFileName()+"\")");
}
cMakeCode.newLine();

Expand Down
19 changes: 18 additions & 1 deletion org.lflang/src/org/lflang/generator/c/CCompiler.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

package org.lflang.generator.c;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
Expand Down Expand Up @@ -222,6 +223,14 @@ static Stream<String> cmakeCompileDefinitions(TargetConfig targetConfig) {
private static List<String> cmakeOptions(TargetConfig targetConfig, FileConfig fileConfig) {
List<String> arguments = new ArrayList<>();
cmakeCompileDefinitions(targetConfig).forEachOrdered(arguments::add);
String separator = File.separator;
String maybeQuote = ""; // Windows seems to require extra level of quoting.
String srcPath = fileConfig.srcPath.toString(); // Windows requires escaping the backslashes.
if (separator.equals("\\")) {
separator = "\\\\\\\\";
maybeQuote = "\\\"";
srcPath = srcPath.replaceAll("\\\\", "\\\\\\\\");
}
arguments.addAll(List.of(
"-DCMAKE_BUILD_TYPE=" + ((targetConfig.cmakeBuildType!=null) ? targetConfig.cmakeBuildType.toString() : "Release"),
"-DCMAKE_INSTALL_PREFIX=" + FileUtil.toUnixString(fileConfig.getOutPath()),
Expand All @@ -230,8 +239,16 @@ private static List<String> cmakeOptions(TargetConfig targetConfig, FileConfig f
fileConfig.binPath
)
),
FileUtil.toUnixString(fileConfig.getSrcGenPath())
"-DLF_FILE_SEPARATOR=\"" + maybeQuote + separator + maybeQuote + "\""
));
// Add #define for source file directory.
// Do not do this for federated programs because for those, the definition is put
// into the cmake file (and fileConfig.srcPath is the wrong directory anyway).
if (!fileConfig.srcPath.toString().contains("fed-gen")) {
// Do not convert to Unix path
arguments.add("-DLF_SOURCE_DIRECTORY=\"" + maybeQuote + srcPath + maybeQuote + "\"");
}
arguments.add(FileUtil.toUnixString(fileConfig.getSrcGenPath()));

if (GeneratorUtils.isHostWindows()) {
arguments.add("-DCMAKE_SYSTEM_VERSION=\"10.0\"");
Expand Down
Loading

0 comments on commit e8349ce

Please sign in to comment.