diff --git a/include/llbuild/Ninja/Manifest.h b/include/llbuild/Ninja/Manifest.h index 4d3a6ac93..11911f7c8 100644 --- a/include/llbuild/Ninja/Manifest.h +++ b/include/llbuild/Ninja/Manifest.h @@ -166,6 +166,10 @@ class Pool { /// in the \see inputs array. The remaining inputs are the order-only ones. unsigned numImplicitInputs; + /// The number of implicit outputs, immediately following the explicit inputs + /// in the \see outputs array. + unsigned numImplicitOutputs; + /// The command parameters, which are used to evaluate the rule template. // // FIXME: It might be substantially better to evaluate all of these in the @@ -191,10 +195,12 @@ class Pool { ArrayRef outputs, ArrayRef inputs, unsigned numExplicitInputs, - unsigned numImplicitInputs) + unsigned numImplicitInputs, + unsigned numImplicitOutputs) : rule(rule), outputs(outputs), inputs(inputs), numExplicitInputs(numExplicitInputs), numImplicitInputs(numImplicitInputs), + numImplicitOutputs(numImplicitOutputs), executionPool(nullptr), depsStyle(unsigned(DepsStyleKind::None)), isGenerator(0), shouldRestat(0) { @@ -208,6 +214,20 @@ class Pool { const std::vector& getInputs() const { return inputs; } + const std::vector::const_iterator explicitOutputs_begin() const { + return outputs.begin(); + } + const std::vector::const_iterator explicitOutputs_end() const { + return explicitOutputs_begin() + getNumExplicitOutputs(); + } + + const std::vector::const_iterator implicitOutputs_begin() const { + return explicitOutputs_end(); + } + const std::vector::const_iterator implicitOutputs_end() const { + return outputs.end(); + } + const std::vector::const_iterator explicitInputs_begin() const { return inputs.begin(); } @@ -229,6 +249,10 @@ class Pool { return inputs.end(); } + unsigned getNumExplicitOutputs() const { + return getOutputs().size() - getNumImplicitOutputs(); + } + unsigned getNumImplicitOutputs() const { return numImplicitOutputs; } unsigned getNumExplicitInputs() const { return numExplicitInputs; } unsigned getNumImplicitInputs() const { return numImplicitInputs; } unsigned getNumOrderOnlyInputs() const { diff --git a/include/llbuild/Ninja/Parser.h b/include/llbuild/Ninja/Parser.h index 7a8f11c99..ac47d598b 100644 --- a/include/llbuild/Ninja/Parser.h +++ b/include/llbuild/Ninja/Parser.h @@ -82,13 +82,17 @@ class ParseActions { /// following the explicit inputs in the \see Inputs array. All of the /// remaining inputs past the implicit inputs are "order-only" inputs. /// + /// \param numImplicitOutputs The number of implicit outputs, listed immediately + /// following the explicit outputs in the \see Outputs array. + /// /// \returns A result object to represent this decl, which will be passed /// later to \see actOnEndBuildDecl(). virtual BuildResult actOnBeginBuildDecl(const Token& name, ArrayRef outputs, ArrayRef inputs, unsigned numExplicitInputs, - unsigned numImplicitInputs) = 0; + unsigned numImplicitInputs, + unsigned numImplicitOutputs) = 0; /// Called on a variable binding within a "build" declaration. /// diff --git a/lib/Commands/NinjaCommand.cpp b/lib/Commands/NinjaCommand.cpp index 73cd15b4c..d7e99b318 100644 --- a/lib/Commands/NinjaCommand.cpp +++ b/lib/Commands/NinjaCommand.cpp @@ -163,7 +163,8 @@ class ParseCommandActions : public ninja::ParseActions { ArrayRef outputs, ArrayRef inputs, unsigned numExplicitInputs, - unsigned numImplicitInputs) override { + unsigned numImplicitInputs, + unsigned numImplicitOutputs) override { std::cerr << __FUNCTION__ << "(/*Name=*/" << "\"" << escapedString(name) << "\"" << ", /*Outputs=*/["; @@ -183,7 +184,11 @@ class ParseCommandActions : public ninja::ParseActions { first = false; } std::cerr << "], /*NumExplicitInputs=*/" << numExplicitInputs - << ", /*NumImplicitInputs=*/" << numImplicitInputs << ")\n"; + << ", /*NumImplicitInputs=*/" << numImplicitInputs; + if (numImplicitOutputs) { + std::cerr << ", /*NumImplicitOutputs=*/" << numImplicitOutputs; + } + std::cerr << ")\n"; return 0; } @@ -278,7 +283,8 @@ class ParseOnlyCommandActions : public ninja::ParseActions { ArrayRef outputs, ArrayRef inputs, unsigned numExplicitInputs, - unsigned numImplicitInputs) override { + unsigned numImplicitInputs, + unsigned numImplicitOutputs) override { return 0; } @@ -536,11 +542,17 @@ static void dumpNinjaManifestText(StringRef file, ninja::Manifest* manifest) { for (const auto& command: commands) { // Write the command entry. std::cout << "build"; + unsigned count = 0; for (const auto& node: command->getOutputs()) { - std::cout << " \"" << util::escapedString(node->getScreenPath()) << "\""; + std::cout << " "; + if (count == command->getOutputs().size() - command->getNumImplicitOutputs()) { + std::cout << "| "; + } + std::cout << "\"" << util::escapedString(node->getScreenPath()) << "\""; + ++count; } std::cout << ": " << command->getRule()->getName(); - unsigned count = 0; + count = 0; for (const auto& node: command->getInputs()) { std::cout << " "; if (count == (command->getNumExplicitInputs() + @@ -704,12 +716,21 @@ static void dumpNinjaManifestJSON(StringRef file, ninja::Manifest* manifest) { if (it != commands.begin()) std::cout << ",\n"; std::cout << " {\n"; std::cout << " \"outputs\": ["; - for (auto it = command->getOutputs().begin(), - ie = command->getOutputs().end(); it != ie; ++it) { - if (it != command->getOutputs().begin()) std::cout << ", "; + for (auto it = command->explicitOutputs_begin(), + ie = command->explicitOutputs_end(); it != ie; ++it) { + if (it != command->explicitOutputs_begin()) std::cout << ", "; std::cout << "\"" << escapeForJSON((*it)->getScreenPath()) << "\""; } std::cout << "],\n"; + if (command->getNumImplicitOutputs()) { + std::cout << " \"implicit_outputs\": ["; + for (auto it = command->implicitOutputs_begin(), + ie = command->implicitOutputs_end(); it != ie; ++it) { + if (it != command->implicitOutputs_begin()) std::cout << ", "; + std::cout << "\"" << escapeForJSON((*it)->getScreenPath()) << "\""; + } + std::cout << "],\n"; + } std::cout << " \"rule\": \"" << command->getRule()->getName() << "\",\n"; diff --git a/lib/Ninja/ManifestLoader.cpp b/lib/Ninja/ManifestLoader.cpp index 9645279ff..c72660768 100644 --- a/lib/Ninja/ManifestLoader.cpp +++ b/lib/Ninja/ManifestLoader.cpp @@ -313,7 +313,8 @@ class ManifestLoader::ManifestLoaderImpl: public ParseActions { ArrayRef outputTokens, ArrayRef inputTokens, unsigned numExplicitInputs, - unsigned numImplicitInputs) override { + unsigned numImplicitInputs, + unsigned numImplicitOutputs) override { StringRef name(nameTok.start, nameTok.length); // Resolve the rule. @@ -351,7 +352,8 @@ class ManifestLoader::ManifestLoaderImpl: public ParseActions { } Command* decl = new (manifest->getAllocator()) - Command(rule, outputs, inputs, numExplicitInputs, numImplicitInputs); + Command(rule, outputs, inputs, numExplicitInputs, numImplicitInputs, + numImplicitOutputs); manifest->getCommands().push_back(decl); return decl; @@ -404,10 +406,10 @@ class ManifestLoader::ManifestLoaderImpl: public ParseActions { } return; } else if (name == "out") { - for (unsigned i = 0, ie = decl->getOutputs().size(); i != ie; ++i) { + for (unsigned i = 0, ie = decl->getNumExplicitOutputs(); i != ie; ++i) { if (i != 0) result << " "; - auto& path = decl->getOutputs()[i]->getScreenPath(); + auto& path = decl->explicitOutputs_begin()[i]->getScreenPath(); result << (context->shellEscapeInAndOut ? basic::shellEscaped(path) : path); } diff --git a/lib/Ninja/Parser.cpp b/lib/Ninja/Parser.cpp index 599a74c14..9391bbd6d 100644 --- a/lib/Ninja/Parser.cpp +++ b/lib/Ninja/Parser.cpp @@ -387,15 +387,31 @@ bool Parser::ParserImpl::parseBuildSpecifier( consumeExpectedToken(Token::Kind::KWBuild); // Parse the output list. - if (tok.tokenKind != Token::Kind::String) { + if (tok.tokenKind != Token::Kind::String && tok.tokenKind != Token::Kind::Pipe) { error("expected output path string"); lexer.setMode(Lexer::LexingMode::None); return false; } SmallVector outputs; - do { + while (tok.tokenKind == Token::Kind::String) { outputs.push_back(consumeExpectedToken(Token::Kind::String)); - } while (tok.tokenKind == Token::Kind::String); + } + unsigned numExplicitOutputs = outputs.size(); + + // Parse implicit outputs, if present. + if (consumeIfToken(Token::Kind::Pipe)) { + // If we have no explicit outputs, we must have at least one implicit output. + if (!numExplicitOutputs && tok.tokenKind != Token::Kind::String) { + error("expected output path string"); + lexer.setMode(Lexer::LexingMode::None); + return false; + } + + while (tok.tokenKind == Token::Kind::String) { + outputs.push_back(consumeExpectedToken(Token::Kind::String)); + } + } + unsigned numImplicitOutputs = outputs.size() - numExplicitOutputs; // Expect the string list to be terminated by a colon. if (tok.tokenKind != Token::Kind::Colon) { @@ -448,7 +464,8 @@ bool Parser::ParserImpl::parseBuildSpecifier( } *decl_out = actions.actOnBeginBuildDecl(name, outputs, inputs, - numExplicitInputs, numImplicitInputs); + numExplicitInputs, numImplicitInputs, + numImplicitOutputs); return true; } diff --git a/products/libllbuild/Ninja-C-API.cpp b/products/libllbuild/Ninja-C-API.cpp index e876cd13a..64977025c 100644 --- a/products/libllbuild/Ninja-C-API.cpp +++ b/products/libllbuild/Ninja-C-API.cpp @@ -129,7 +129,10 @@ class CAPIManifest { statement->implicitInputs_end()); auto orderOnlyInputs = copyRefs(statement->orderOnlyInputs_begin(), statement->orderOnlyInputs_end()); - auto outputs = copyRefs(statement->getOutputs()); + auto explicitOutputs = copyRefs(statement->explicitOutputs_begin(), + statement->explicitOutputs_end()); + auto implicitOutputs = copyRefs(statement->implicitOutputs_begin(), + statement->implicitOutputs_end()); auto variables = copyRefs(statement->getParameters()); return { @@ -139,7 +142,8 @@ class CAPIManifest { explicitInputs.size(), explicitInputs.data(), implicitInputs.size(), implicitInputs.data(), orderOnlyInputs.size(), orderOnlyInputs.data(), - outputs.size(), outputs.data(), + explicitOutputs.size(), explicitOutputs.data(), + implicitOutputs.size(), implicitOutputs.data(), variables.size(), variables.data(), statement->hasGeneratorFlag(), statement->hasRestatFlag() diff --git a/products/libllbuild/include/llbuild/ninja.h b/products/libllbuild/include/llbuild/ninja.h index 91998902c..2c3bd3da3 100644 --- a/products/libllbuild/include/llbuild/ninja.h +++ b/products/libllbuild/include/llbuild/ninja.h @@ -79,6 +79,8 @@ typedef struct llb_ninja_build_statement_t_ { const llb_string_ref_t *order_only_inputs; uint64_t num_outputs; const llb_string_ref_t *outputs; + uint64_t num_implicit_outputs; + const llb_string_ref_t *implicit_outputs; uint64_t num_variables; const llb_ninja_variable_t *variables; bool generator; diff --git a/tests/Ninja/Build/implicit-outputs.ninja b/tests/Ninja/Build/implicit-outputs.ninja new file mode 100644 index 000000000..ed27a1ccb --- /dev/null +++ b/tests/Ninja/Build/implicit-outputs.ninja @@ -0,0 +1,28 @@ +# We run the build in a sandbox in the temp directory to ensure we don't +# interact with the source dirs. +# +# RUN: rm -rf %t.build +# RUN: mkdir -p %t.build +# RUN: cp %s %t.build/build.ninja +# RUN: touch %t.build/dummy_in1 +# RUN: %{llbuild} ninja build --jobs 1 --chdir %t.build &> %t1.out +# RUN: %{FileCheck} --check-prefix CHECK-INITIAL < %t1.out %s +# +# RUN: %{llbuild} ninja build --jobs 1 --chdir %t.build &> %t2.out +# RUN: %{FileCheck} --check-prefix CHECK-NULL < %t2.out %s + +# CHECK-INITIAL: RULE A -- dummy_in1 -- dummy_out1 +# +# CHECK-NULL-NOT: RULE +# CHECK-NULL: Entering directory +# CHECK-NULL-NEXT: no work to do +# +# CHECK-REBUILD: RULE A -- dummy_in1 -- dummy_out1 + +# RUN: rm %t.build/dummy_out2 +# RUN: %{llbuild} ninja build --jobs 1 --chdir %t.build &> %t3.out +# RUN: %{FileCheck} --check-prefix CHECK-REBUILD < %t3.out %s +rule A + command = echo "RULE A -- ${in} -- ${out}" && touch "${out}" dummy_out2 + +build dummy_out1 | dummy_out2: A dummy_in1 diff --git a/tests/Ninja/Loader/builds.ninja b/tests/Ninja/Loader/builds.ninja index 26bb3cb15..95cfb9054 100644 --- a/tests/Ninja/Loader/builds.ninja +++ b/tests/Ninja/Loader/builds.ninja @@ -105,3 +105,12 @@ rule target12_rule command = target12_rule $in -o $out depfile = $out.d build target12$ foo: target12_rule target12$ inputA + +# Check handling of implicit build outputs (and $out) +# +# CHECK: build "target13" | "target13_outA": target13_rule +# CHECK-NEXT: command = "target13 target13_inputA -o target13" +rule target13_rule + command = target13 ${in} -o ${out} +build target13 | target13_outA: target13_rule target13_inputA + diff --git a/tests/Ninja/Parser/basic.ninja b/tests/Ninja/Parser/basic.ninja index ec88fa513..49256a6f1 100644 --- a/tests/Ninja/Parser/basic.ninja +++ b/tests/Ninja/Parser/basic.ninja @@ -51,7 +51,7 @@ build a.o: cc b.o name = value # CHECK: actOnEndBuildDecl(/*Decl=*/{{.*}}) -# Check "build" implicit and order-only parsing. +# Check "build" implicit inputs and order-only inputs parsing. # # CHECK: actOnBeginBuildDecl(/*Name=*/"cc", /*Outputs=*/["a.o"], /*Inputs=*/["b.o"], /*NumExplicitInputs=*/0, /*NumImplicitInputs=*/0) build a.o: cc | || b.o @@ -62,6 +62,19 @@ build a.o: cc b.o | c.o || d.o # CHECK: basic.ninja:[[@LINE+1]]:14: error: expected newline token build a.o: cc : +# Check "build" implict output parsing. +# +# CHECK: actOnBeginBuildDecl(/*Name=*/"cc", /*Outputs=*/["a.o", "b.d"], /*Inputs=*/[], /*NumExplicitInputs=*/0, /*NumImplicitInputs=*/0, /*NumImplicitOutputs=*/1) +build a.o | b.d: cc + +# CHECK: actOnBeginBuildDecl(/*Name=*/"cc", /*Outputs=*/["c.d"], /*Inputs=*/[], /*NumExplicitInputs=*/0, /*NumImplicitInputs=*/0, /*NumImplicitOutputs=*/1) +build | c.d: cc + +# CHECK: basic.ninja:[[@LINE+2]]:6: error: expected output path string +# CHECK: basic.ninja:[[@LINE+2]]:8: error: expected output path string +build : cc +build | : cc + # Check for handling of malformed statements. # CHECK: basic.ninja:[[@LINE+3]]:4: error: expected variable name # CHECK: basic.ninja:[[@LINE+3]]:11: error: expected '=' token