From 6051c8027bf812ad350119a4bb24fd742d8568e3 Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Tue, 12 Feb 2019 10:13:16 -0800 Subject: [PATCH 1/8] Allow osx tooling to understand c sharp After this change I am able to load foundationdb.sln with vscode and get features like jump to definition and autocomplete. IntermediateOutputPath will now use a default value. --- flow/actorcompiler/actorcompiler.csproj | 1 - flow/coveragetool/coveragetool.csproj | 1 - 2 files changed, 2 deletions(-) diff --git a/flow/actorcompiler/actorcompiler.csproj b/flow/actorcompiler/actorcompiler.csproj index b9af108b57..e737adabd2 100644 --- a/flow/actorcompiler/actorcompiler.csproj +++ b/flow/actorcompiler/actorcompiler.csproj @@ -12,7 +12,6 @@ v4.0 512 $(SolutionDir)bin\$(Configuration)\ - $(SystemDrive)\temp\msvcfdb\$(Configuration)\actorcompiler\ publish\ true Disk diff --git a/flow/coveragetool/coveragetool.csproj b/flow/coveragetool/coveragetool.csproj index 485f2fac50..69568c3867 100644 --- a/flow/coveragetool/coveragetool.csproj +++ b/flow/coveragetool/coveragetool.csproj @@ -12,7 +12,6 @@ v4.0 512 $(SolutionDir)bin\$(Configuration)\ - $(SystemDrive)\temp\msvcfdb\$(Configuration)\coveragetool\ true From 0116f6cf8ccb522041d47d06d56a5eb23e3a3102 Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Tue, 12 Feb 2019 10:38:12 -0800 Subject: [PATCH 2/8] Add editor-generated files to .gitignore --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index f510ee3941..979a625f2e 100644 --- a/.gitignore +++ b/.gitignore @@ -76,6 +76,8 @@ foundationdb.VC.db foundationdb.VC.VC.opendb ipch/ compile_commands.json +flow/actorcompiler/obj +flow/coveragetool/obj # Temporary and user configuration files *~ From d348846a10736d970a263aeb4637dca9a044a504 Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Tue, 12 Feb 2019 10:32:45 -0800 Subject: [PATCH 3/8] Introduce ErrorMessagePolicy class This encapsulates logic for how to report particular error messages. So far only used for reporting an actor not containing a wait statement, as this warning should be suppressed in certain contexts. We could change other error messages to use this class, but since they don't have any logic I don't think the extra indirection is appropriate. --- flow/actorcompiler/ActorParser.cs | 26 ++++++++++++++++++++------ flow/actorcompiler/ParseTree.cs | 1 + flow/actorcompiler/Program.cs | 3 ++- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/flow/actorcompiler/ActorParser.cs b/flow/actorcompiler/ActorParser.cs index b2ada01e45..69077728e2 100644 --- a/flow/actorcompiler/ActorParser.cs +++ b/flow/actorcompiler/ActorParser.cs @@ -36,6 +36,18 @@ namespace actorcompiler } }; + class ErrorMessagePolicy + { + public bool ActorWithoutWaitEnabled = true; + public void HandleActorWithoutWait(String sourceFile, Actor actor) + { + if (ActorWithoutWaitEnabled && !actor.isTestCase) + { + Console.Error.WriteLine("{0}:{1}: warning: ACTOR {2} does not contain a wait() statement", sourceFile, actor.SourceLine, actor.name); + } + } + } + class Token { public string Value; @@ -200,10 +212,12 @@ namespace actorcompiler Token[] tokens; string sourceFile; + ErrorMessagePolicy errorMessagePolicy; - public ActorParser(string text, string sourceFile) + public ActorParser(string text, string sourceFile, ErrorMessagePolicy errorMessagePolicy) { this.sourceFile = sourceFile; + this.errorMessagePolicy = errorMessagePolicy; tokens = Tokenize(text).Select(t=>new Token{ Value=t }).ToArray(); CountParens(); //if (sourceFile.EndsWith(".h")) LineNumbersEnabled = false; @@ -872,21 +886,21 @@ namespace actorcompiler var body = range(heading.End+1, tokens.Length) .TakeWhile(t => t.BraceDepth > toks.First().BraceDepth); - bool warnOnNoWait = false; if (head_token.Value == "ACTOR") { ParseActorHeading(actor, heading); - warnOnNoWait = true; } - else if (head_token.Value == "TEST_CASE") + else if (head_token.Value == "TEST_CASE") { ParseTestCaseHeading(actor, heading); + actor.isTestCase = true; + } else head_token.Assert("ACTOR or TEST_CASE expected!", t => false); actor.body = ParseCodeBlock(body); - if (!actor.body.containsWait() && warnOnNoWait) - Console.Error.WriteLine("{0}:{1}: warning: ACTOR {2} does not contain a wait() statement", sourceFile, actor.SourceLine, actor.name); + if (!actor.body.containsWait()) + this.errorMessagePolicy.HandleActorWithoutWait(sourceFile, actor); end = body.End + 1; } diff --git a/flow/actorcompiler/ParseTree.cs b/flow/actorcompiler/ParseTree.cs index 26aad6f827..f161008ce2 100644 --- a/flow/actorcompiler/ParseTree.cs +++ b/flow/actorcompiler/ParseTree.cs @@ -234,6 +234,7 @@ namespace actorcompiler public string testCaseParameters = null; public string nameSpace = null; public bool isForwardDeclaration = false; + public bool isTestCase = false; }; class Descr diff --git a/flow/actorcompiler/Program.cs b/flow/actorcompiler/Program.cs index 57fb1da85f..8ecb2d125a 100644 --- a/flow/actorcompiler/Program.cs +++ b/flow/actorcompiler/Program.cs @@ -38,11 +38,12 @@ namespace actorcompiler } Console.WriteLine("actorcompiler {0}", string.Join(" ", args)); string input = args[0], output = args[1], outputtmp = args[1] + ".tmp"; + ErrorMessagePolicy errorMessagePolicy = new ErrorMessagePolicy(); try { var inputData = File.ReadAllText(input); using (var outputStream = new StreamWriter(outputtmp)) - new ActorParser(inputData, input.Replace('\\', '/')).Write(outputStream, output.Replace('\\', '/')); + new ActorParser(inputData, input.Replace('\\', '/'), errorMessagePolicy).Write(outputStream, output.Replace('\\', '/')); if (File.Exists(output)) { File.SetAttributes(output, FileAttributes.Normal); From 8b0e593f83bb2903c267c619cd61a927fde15883 Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Tue, 12 Feb 2019 10:37:08 -0800 Subject: [PATCH 4/8] Add --disable-actor-without-wait-error flag to actorcompiler --- flow/actorcompiler/Program.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/flow/actorcompiler/Program.cs b/flow/actorcompiler/Program.cs index 8ecb2d125a..dfce4ad014 100644 --- a/flow/actorcompiler/Program.cs +++ b/flow/actorcompiler/Program.cs @@ -33,12 +33,16 @@ namespace actorcompiler if (args.Length < 2) { Console.WriteLine("Usage:"); - Console.WriteLine(" actorcompiler [input] [output]"); + Console.WriteLine(" actorcompiler [--disable-actor-without-wait-error]"); return 100; } Console.WriteLine("actorcompiler {0}", string.Join(" ", args)); string input = args[0], output = args[1], outputtmp = args[1] + ".tmp"; ErrorMessagePolicy errorMessagePolicy = new ErrorMessagePolicy(); + if (args.Contains("--disable-actor-without-wait-error")) + { + errorMessagePolicy.ActorWithoutWaitEnabled = false; + } try { var inputData = File.ReadAllText(input); From 874a58cb4f631f1fe21b5b16ec699c1994efb06b Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Tue, 12 Feb 2019 10:55:57 -0800 Subject: [PATCH 5/8] Suppress actor without wait for tests in cmake --- fdbrpc/CMakeLists.txt | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/fdbrpc/CMakeLists.txt b/fdbrpc/CMakeLists.txt index bd14736152..eaafd1e793 100644 --- a/fdbrpc/CMakeLists.txt +++ b/fdbrpc/CMakeLists.txt @@ -1,5 +1,4 @@ set(FDBRPC_SRCS - ActorFuzz.actor.cpp AsyncFileCached.actor.h AsyncFileEIO.actor.h AsyncFileKAIO.actor.h @@ -11,9 +10,7 @@ set(FDBRPC_SRCS AsyncFileWriteChecker.cpp batcher.actor.h crc32c.cpp - dsltest.actor.cpp FailureMonitor.actor.cpp - FlowTests.actor.cpp FlowTransport.actor.cpp genericactors.actor.h genericactors.actor.cpp @@ -55,8 +52,14 @@ if(NOT WIN32) list(APPEND FDBRPC_SRCS libcoroutine/context.c libeio/eio.c) endif() -actor_set(FDBRPC_BUILD "${FDBRPC_SRCS}") +set(FDBRPC_SRCS_ALLOW_ACTOR_WITHOUT_WAIT + ActorFuzz.actor.cpp + FlowTests.actor.cpp + dsltest.actor.cpp) + +actor_set(FDBRPC_BUILD "${FDBRPC_SRCS};${FDBRPC_SRCS_ALLOW_ACTOR_WITHOUT_WAIT}") add_library(fdbrpc STATIC ${FDBRPC_BUILD}) actor_compile(fdbrpc "${FDBRPC_SRCS}") +actor_compile(fdbrpc "${FDBRPC_SRCS_ALLOW_ACTOR_WITHOUT_WAIT}" DISABLE_ACTOR_WITHOUT_WAIT) target_include_directories(fdbrpc PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/libeio) target_link_libraries(fdbrpc PUBLIC flow) From df3454114e3f3040992f55d0d78ac75f916ab056 Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Tue, 12 Feb 2019 11:03:08 -0800 Subject: [PATCH 6/8] Add TODO --- flow/actorcompiler/ActorParser.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/flow/actorcompiler/ActorParser.cs b/flow/actorcompiler/ActorParser.cs index 69077728e2..07d8a6e0e1 100644 --- a/flow/actorcompiler/ActorParser.cs +++ b/flow/actorcompiler/ActorParser.cs @@ -43,6 +43,7 @@ namespace actorcompiler { if (ActorWithoutWaitEnabled && !actor.isTestCase) { + // TODO(atn34): Once cmake is the only build system we can make this an error instead of a warning. Console.Error.WriteLine("{0}:{1}: warning: ACTOR {2} does not contain a wait() statement", sourceFile, actor.SourceLine, actor.name); } } From 3a38bff8eef43b2e368d8c039546ff2a3885c34c Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Wed, 13 Feb 2019 10:30:35 -0800 Subject: [PATCH 7/8] Use DISABLE_ACTOR_WITHOUT_WAIT_WARNING consistently --- cmake/FlowCommands.cmake | 4 ++-- fdbrpc/CMakeLists.txt | 6 +++--- flow/actorcompiler/ActorParser.cs | 4 ++-- flow/actorcompiler/Program.cs | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cmake/FlowCommands.cmake b/cmake/FlowCommands.cmake index cd3cdea4ec..255689e7b6 100644 --- a/cmake/FlowCommands.cmake +++ b/cmake/FlowCommands.cmake @@ -14,7 +14,7 @@ endmacro() set(ACTOR_TARGET_COUNTER "0") macro(actor_compile target srcs) - set(options DISABLE_ACTOR_WITHOUT_WAIT) + set(options DISABLE_ACTOR_WITHOUT_WAIT_WARNING) set(oneValueArg) set(multiValueArgs) cmake_parse_arguments(ACTOR_COMPILE "${options}" "${oneValueArgs}" "${multiValueArgs}" "${ARGN}") @@ -27,7 +27,7 @@ macro(actor_compile target srcs) string(REPLACE ".actor.cpp" ".actor.g.cpp" tmp ${src}) endif() set(actor_compiler_flags "") - if(ACTOR_COMPILE_DISABLE_ACTOR_WITHOUT_WAIT) + if(ACTOR_COMPILE_DISABLE_ACTOR_WITHOUT_WAIT_WARNING) set(actor_compiler_flags "--disable-actor-without-wait-error") endif() if(tmp) diff --git a/fdbrpc/CMakeLists.txt b/fdbrpc/CMakeLists.txt index eaafd1e793..3447f4e066 100644 --- a/fdbrpc/CMakeLists.txt +++ b/fdbrpc/CMakeLists.txt @@ -52,14 +52,14 @@ if(NOT WIN32) list(APPEND FDBRPC_SRCS libcoroutine/context.c libeio/eio.c) endif() -set(FDBRPC_SRCS_ALLOW_ACTOR_WITHOUT_WAIT +set(FDBRPC_SRCS_DISABLE_ACTOR_WITHOUT_WAIT_WARNING ActorFuzz.actor.cpp FlowTests.actor.cpp dsltest.actor.cpp) -actor_set(FDBRPC_BUILD "${FDBRPC_SRCS};${FDBRPC_SRCS_ALLOW_ACTOR_WITHOUT_WAIT}") +actor_set(FDBRPC_BUILD "${FDBRPC_SRCS};${FDBRPC_SRCS_DISABLE_ACTOR_WITHOUT_WAIT_WARNING}") add_library(fdbrpc STATIC ${FDBRPC_BUILD}) actor_compile(fdbrpc "${FDBRPC_SRCS}") -actor_compile(fdbrpc "${FDBRPC_SRCS_ALLOW_ACTOR_WITHOUT_WAIT}" DISABLE_ACTOR_WITHOUT_WAIT) +actor_compile(fdbrpc "${FDBRPC_SRCS_DISABLE_ACTOR_WITHOUT_WAIT_WARNING}" DISABLE_ACTOR_WITHOUT_WAIT_WARNING) target_include_directories(fdbrpc PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/libeio) target_link_libraries(fdbrpc PUBLIC flow) diff --git a/flow/actorcompiler/ActorParser.cs b/flow/actorcompiler/ActorParser.cs index 07d8a6e0e1..ca6d709444 100644 --- a/flow/actorcompiler/ActorParser.cs +++ b/flow/actorcompiler/ActorParser.cs @@ -38,10 +38,10 @@ namespace actorcompiler class ErrorMessagePolicy { - public bool ActorWithoutWaitEnabled = true; + public bool DisableActorWithoutWaitWarning = false; public void HandleActorWithoutWait(String sourceFile, Actor actor) { - if (ActorWithoutWaitEnabled && !actor.isTestCase) + if (!DisableActorWithoutWaitWarning && !actor.isTestCase) { // TODO(atn34): Once cmake is the only build system we can make this an error instead of a warning. Console.Error.WriteLine("{0}:{1}: warning: ACTOR {2} does not contain a wait() statement", sourceFile, actor.SourceLine, actor.name); diff --git a/flow/actorcompiler/Program.cs b/flow/actorcompiler/Program.cs index dfce4ad014..9035998355 100644 --- a/flow/actorcompiler/Program.cs +++ b/flow/actorcompiler/Program.cs @@ -33,7 +33,7 @@ namespace actorcompiler if (args.Length < 2) { Console.WriteLine("Usage:"); - Console.WriteLine(" actorcompiler [--disable-actor-without-wait-error]"); + Console.WriteLine(" actorcompiler [--disable-actor-without-wait-warning]"); return 100; } Console.WriteLine("actorcompiler {0}", string.Join(" ", args)); @@ -41,7 +41,7 @@ namespace actorcompiler ErrorMessagePolicy errorMessagePolicy = new ErrorMessagePolicy(); if (args.Contains("--disable-actor-without-wait-error")) { - errorMessagePolicy.ActorWithoutWaitEnabled = false; + errorMessagePolicy.DisableActorWithoutWaitWarning = true; } try { From 1ea58c1e5ed40bc2b567834d2b028b496796ca1d Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Wed, 13 Feb 2019 10:44:58 -0800 Subject: [PATCH 8/8] Actually change name of flag --- cmake/FlowCommands.cmake | 2 +- flow/actorcompiler/Program.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/FlowCommands.cmake b/cmake/FlowCommands.cmake index 255689e7b6..4eb4c4735d 100644 --- a/cmake/FlowCommands.cmake +++ b/cmake/FlowCommands.cmake @@ -28,7 +28,7 @@ macro(actor_compile target srcs) endif() set(actor_compiler_flags "") if(ACTOR_COMPILE_DISABLE_ACTOR_WITHOUT_WAIT_WARNING) - set(actor_compiler_flags "--disable-actor-without-wait-error") + set(actor_compiler_flags "--disable-actor-without-wait-warning") endif() if(tmp) if(WIN32) diff --git a/flow/actorcompiler/Program.cs b/flow/actorcompiler/Program.cs index 9035998355..d483a8eacb 100644 --- a/flow/actorcompiler/Program.cs +++ b/flow/actorcompiler/Program.cs @@ -39,7 +39,7 @@ namespace actorcompiler Console.WriteLine("actorcompiler {0}", string.Join(" ", args)); string input = args[0], output = args[1], outputtmp = args[1] + ".tmp"; ErrorMessagePolicy errorMessagePolicy = new ErrorMessagePolicy(); - if (args.Contains("--disable-actor-without-wait-error")) + if (args.Contains("--disable-actor-without-wait-warning")) { errorMessagePolicy.DisableActorWithoutWaitWarning = true; }