aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2021-06-08 15:30:35 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2021-06-08 15:43:08 +0200
commitf8e6ed173b9b77ec7ac1b0d39ae83f29fb9468a9 (patch)
treec8f22a50c6df6070105fda88a7800c2b51332d23
parent5900f155a4a0da88cfd56bedccef0c74cc19c9f7 (diff)
Redo low verbosity diagnostic deduction to use scope instead of target
-rw-r--r--libbuild2/adhoc-rule-buildscript.cxx10
-rw-r--r--libbuild2/adhoc-rule-buildscript.hxx3
-rw-r--r--libbuild2/adhoc-rule-cxx.cxx2
-rw-r--r--libbuild2/adhoc-rule-cxx.hxx3
-rw-r--r--libbuild2/build/script/parser.cxx30
-rw-r--r--libbuild2/build/script/parser.hxx16
-rw-r--r--libbuild2/build/script/parser.test.cxx2
-rw-r--r--libbuild2/parser.cxx10
-rw-r--r--libbuild2/rule.hxx13
-rw-r--r--tests/dependency/recipe/testscript65
10 files changed, 58 insertions, 96 deletions
diff --git a/libbuild2/adhoc-rule-buildscript.cxx b/libbuild2/adhoc-rule-buildscript.cxx
index c4b9169..c715ff2 100644
--- a/libbuild2/adhoc-rule-buildscript.cxx
+++ b/libbuild2/adhoc-rule-buildscript.cxx
@@ -23,11 +23,7 @@ using namespace std;
namespace build2
{
bool adhoc_buildscript_rule::
- recipe_text (context& ctx,
- const scope& s,
- const target* tg,
- string&& t,
- attributes& as)
+ recipe_text (const scope& s, string&& t, attributes& as)
{
// Handle and erase recipe-specific attributes.
//
@@ -58,9 +54,9 @@ namespace build2
checksum = sha256 (t).string ();
istringstream is (move (t));
- build::script::parser p (ctx);
+ build::script::parser p (s.ctx);
- script = p.pre_parse (s, tg, actions,
+ script = p.pre_parse (s, actions,
is, loc.file, loc.line + 1,
move (diag), as.loc);
diff --git a/libbuild2/adhoc-rule-buildscript.hxx b/libbuild2/adhoc-rule-buildscript.hxx
index a04840d..748a4c4 100644
--- a/libbuild2/adhoc-rule-buildscript.hxx
+++ b/libbuild2/adhoc-rule-buildscript.hxx
@@ -42,8 +42,7 @@ namespace build2
: adhoc_rule (move (n), l, b) {}
virtual bool
- recipe_text (context&, const scope&, const target*,
- string&&, attributes&) override;
+ recipe_text (const scope&, string&&, attributes&) override;
virtual void
dump_attributes (ostream&) const override;
diff --git a/libbuild2/adhoc-rule-cxx.cxx b/libbuild2/adhoc-rule-cxx.cxx
index 5576eda..83eeee0 100644
--- a/libbuild2/adhoc-rule-cxx.cxx
+++ b/libbuild2/adhoc-rule-cxx.cxx
@@ -40,7 +40,7 @@ namespace build2
}
bool adhoc_cxx_rule::
- recipe_text (context&, const scope&, const target*, string&& t, attributes&)
+ recipe_text (const scope&, string&& t, attributes&)
{
code = move (t);
return true;
diff --git a/libbuild2/adhoc-rule-cxx.hxx b/libbuild2/adhoc-rule-cxx.hxx
index 8254906..0ca6038 100644
--- a/libbuild2/adhoc-rule-cxx.hxx
+++ b/libbuild2/adhoc-rule-cxx.hxx
@@ -72,8 +72,7 @@ namespace build2
optional<string> sep);
virtual bool
- recipe_text (context&, const scope&, const target*,
- string&&, attributes&) override;
+ recipe_text (const scope&, string&&, attributes&) override;
virtual
~adhoc_cxx_rule () override;
diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx
index 9194e30..b602880 100644
--- a/libbuild2/build/script/parser.cxx
+++ b/libbuild2/build/script/parser.cxx
@@ -28,7 +28,6 @@ namespace build2
script parser::
pre_parse (const scope& bs,
- const target* tg,
const small_vector<action, 1>& as,
istream& is, const path_name& pn, uint64_t line,
optional<string> diag, const location& diag_loc)
@@ -40,11 +39,11 @@ namespace build2
lexer l (is, *path_, line, lexer_mode::command_line);
set_lexer (&l);
- // The script shouldn't be able to modify the target/scopes.
+ // The script shouldn't be able to modify the scopes.
//
- target_ = const_cast<target*> (tg);
+ target_ = nullptr;
actions_ = &as;
- scope_ = const_cast<scope*> (tg != nullptr ? &tg->base_scope () : &bs);
+ scope_ = const_cast<scope*> (&bs);
root_ = scope_->root_scope ();
pbase_ = scope_->src_path_;
@@ -585,18 +584,12 @@ namespace build2
parse_names_result pr;
{
- // During pre-parse, if the script name is not set manually and we
- // have the target, we suspend pre-parse, parse the command names
- // for real and try to deduce the script name from the result.
- // Otherwise, we continue to pre-parse and bail out after parsing
- // the names.
- //
- // @@ TODO: maybe we could recognize literal names even if target
- // is NULL (see the tests for some ugly recipes). But will need
- // to be careful to still pick up ambiguity between literal and
- // skipped due to target being NULL.
+ // During pre-parse, if the script name is not set manually we
+ // suspend pre-parse, parse the command names for real and try to
+ // deduce the script name from the result. Otherwise, we continue
+ // to pre-parse and bail out after parsing the names.
//
- // Note that the later is not just an optimization since expansion
+ // Note that the latter is not just an optimization since expansion
// that wouldn't fail during execution may fail in this special
// mode, for example:
//
@@ -615,7 +608,7 @@ namespace build2
//
// This is also the reason why we add a diag frame.
//
- if (pre_parse_ && (diag_weight_ != 4 && target_ != nullptr))
+ if (pre_parse_ && diag_weight_ != 4)
{
pre_parse_ = false; // Make parse_names() perform expansions.
pre_parse_suspended_ = true;
@@ -646,7 +639,7 @@ namespace build2
pre_parse_ = true;
}
- if (pre_parse_ && (diag_weight_ == 4 || target_ == nullptr))
+ if (pre_parse_ && diag_weight_ == 4)
return nullopt;
}
@@ -1041,6 +1034,7 @@ namespace build2
// it could be used for (we need scope_ for calling functions such as
// $target.path()).
//
+ target_ = nullptr;
root_ = const_cast<scope*> (&rs);
scope_ = const_cast<scope*> (&bs);
pbase_ = scope_->src_path_;
@@ -1170,7 +1164,7 @@ namespace build2
const variable* pvar (scope_->ctx.var_pool.find (name));
if (pvar != nullptr)
- r = (*target_)[*pvar];
+ r = (*scope_)[*pvar];
}
if (!depdb_clear_)
diff --git a/libbuild2/build/script/parser.hxx b/libbuild2/build/script/parser.hxx
index 8c787f9..948c381 100644
--- a/libbuild2/build/script/parser.hxx
+++ b/libbuild2/build/script/parser.hxx
@@ -32,11 +32,21 @@ namespace build2
parser (context& c): build2::script::parser (c) {}
// Note that the returned script object references the passed path
- // name. Target is NULL if this recipe is shared among multiple
- // targets.
+ // name.
+ //
+ // Note also that we use the scope to lookup variable values while
+ // trying to deduce the low verbosity diagnostics name (see code
+ // around pre_parse_suspended for details). But that means we may
+ // derive such a name based on the wrong value. This can happen if the
+ // expanded variable value is reset after the recipe has been
+ // pre-parsed or if such a value is set on the target (which is where
+ // we start when looking up variables during the real parse). The
+ // current thinking is that a remote possibility of this happening is
+ // acceptable in this situation -- the worst that can happen is that
+ // we will end up with mismatching diagnostics.
//
script
- pre_parse (const scope&, const target*, const small_vector<action, 1>&,
+ pre_parse (const scope&, const small_vector<action, 1>&,
istream&, const path_name&, uint64_t line,
optional<string> diag_name, const location& diag_loc);
diff --git a/libbuild2/build/script/parser.test.cxx b/libbuild2/build/script/parser.test.cxx
index 0bc1c5d..c9356a8 100644
--- a/libbuild2/build/script/parser.test.cxx
+++ b/libbuild2/build/script/parser.test.cxx
@@ -208,7 +208,7 @@ namespace build2
parser p (ctx);
path_name nm ("buildfile");
- script s (p.pre_parse (tt.base_scope (), &tt, acts,
+ script s (p.pre_parse (tt.base_scope (), acts,
cin, nm,
11 /* line */,
(m != mode::diag
diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx
index 6f59b30..d0661a9 100644
--- a/libbuild2/parser.cxx
+++ b/libbuild2/parser.cxx
@@ -1567,7 +1567,6 @@ namespace build2
}
}
- bool multi (replay_ != replay::stop); // Multiple targets.
bool first (replay_ != replay::play); // First target.
bool clean (false); // Seen recipe that requires cleanup.
@@ -1584,14 +1583,13 @@ namespace build2
{
const string& name;
small_vector<shared_ptr<adhoc_rule>, 1>& recipes;
- bool multi;
bool first;
bool& clean;
size_t i;
attributes& as;
buildspec& bs;
const location& bsloc;
- } d {name, recipes, multi, first, clean, i, as, bs, bsloc};
+ } d {name, recipes, first, clean, i, as, bs, bsloc};
// Note that this function must be called at most once per iteration.
//
@@ -1799,11 +1797,7 @@ namespace build2
// Set the recipe text.
//
- if (ar.recipe_text (ctx,
- *scope_,
- d.multi ? nullptr : target_,
- move (t.value),
- d.as))
+ if (ar.recipe_text (*scope_, move (t.value), d.as))
d.clean = true;
// Verify we have no unhandled attributes.
diff --git a/libbuild2/rule.hxx b/libbuild2/rule.hxx
index 67c0f6d..e46c402 100644
--- a/libbuild2/rule.hxx
+++ b/libbuild2/rule.hxx
@@ -172,18 +172,13 @@ namespace build2
// Set the rule text, handle any recipe-specific attributes, and return
// true if the recipe builds anything in the build/recipes/ directory and
- // therefore requires cleanup.
+ // therefore requires cleanup. Scope is the scope of the recipe.
//
- // Target is not NULL only if this recipe is for a single target. Scope is
- // the scope of the recipe (not necessarily the same as the target's base
- // scope).
- //
- // Note that this function is called after the actions member has been
- // populated.
+ // Note also that this function is called after the actions member has
+ // been populated.
//
virtual bool
- recipe_text (context&, const scope&, const target*,
- string&&, attributes&) = 0;
+ recipe_text (const scope&, string&&, attributes&) = 0;
public:
// Some of the operations come in compensating pairs, such as update and
diff --git a/tests/dependency/recipe/testscript b/tests/dependency/recipe/testscript
index 3126691..ebcdfdb 100644
--- a/tests/dependency/recipe/testscript
+++ b/tests/dependency/recipe/testscript
@@ -28,16 +28,14 @@ EOE
$* <<EOI 2>>/~%EOE%
alias{x y}: alias{z}
{{
- diag echo
echo
}}
dump alias{y}
EOI
-<stdin>:6:1: dump:
+<stdin>:5:1: dump:
% .+/alias\{y\}: .+/:alias\{z\}%
- % perform(update)
+ % [diag=echo] perform(update)
{{
- diag echo
echo
}}
EOE
@@ -66,16 +64,14 @@ $* <<EOI 2>>/~%EOE%
alias{x y}:
% perform(update clean)
{{
- diag echo
echo
}}
dump alias{y}
EOI
-<stdin>:7:1: dump:
+<stdin>:6:1: dump:
% .+/alias\{y\}:%
- % perform(update) perform(clean)
+ % [diag=echo] perform(update) perform(clean)
{{
- diag echo
echo
}}
EOE
@@ -128,19 +124,17 @@ alias{x y}: alias{z}
var = x
}
{{
- diag echo
echo
}}
dump alias{y}
EOI
-<stdin>:9:1: dump:
+<stdin>:8:1: dump:
% .+/alias\{y\}: .+/:alias\{z\}%
{
var = x
}
- % perform(update)
+ % [diag=echo] perform(update)
{{
- diag echo
echo
}}
EOE
@@ -176,19 +170,17 @@ $* <<EOI 2>>/~%EOE%
var = x
}
{{
- diag echo
echo
}}
dump alias{y}
EOI
-<stdin>:9:1: dump:
+<stdin>:8:1: dump:
% .+/alias\{y\}: .+/:alias\{z\}%
{
var = x
}
- % perform(update)
+ % [diag=echo] perform(update)
{{
- diag echo
echo
}}
EOE
@@ -226,19 +218,17 @@ alias{x y}:
}
% perform(update)
{{
- diag echo
echo
}}
dump alias{y}
EOI
-<stdin>:10:1: dump:
+<stdin>:9:1: dump:
% .+/alias\{y\}:%
{
var = x
}
- % perform(update)
+ % [diag=echo] perform(update)
{{
- diag echo
echo
}}
EOE
@@ -273,26 +263,22 @@ EOE
$* <<EOI 2>>/~%EOE%
alias{x y}: alias{z}
{{
- diag echo
echo
}}
% clean
{{{
- diag cat
cat
}}}
dump alias{y}
EOI
-<stdin>:11:1: dump:
+<stdin>:9:1: dump:
% .+/alias\{y\}: .+/:alias\{z\}%
- % perform(update)
+ % [diag=echo] perform(update)
{{
- diag echo
echo
}}
- % perform(clean)
+ % [diag=cat] perform(clean)
{{{
- diag cat
cat
}}}
EOE
@@ -331,27 +317,23 @@ alias{x y}:
% clean
{{
- diag echo
echo
}}
% update
{{{
- diag cat
cat
}}}
dump alias{y}
EOI
-<stdin>:14:1: dump:
+<stdin>:12:1: dump:
% .+/alias\{y\}:%
- % perform(clean)
+ % [diag=echo] perform(clean)
{{
- diag echo
echo
}}
- % perform(update)
+ % [diag=cat] perform(update)
{{{
- diag cat
cat
}}}
EOE
@@ -504,21 +486,18 @@ alias{x y}: alias{z}
%
if $f
{{
- diag false
false
}}
else
{{
- diag echo
echo
}}
dump alias{y}
EOI
-<stdin>:14:1: dump:
+<stdin>:12:1: dump:
% .+/alias\{y\}: .+/:alias\{z\}%
- % perform(update)
+ % [diag=echo] perform(update)
{{
- diag echo
echo
}}
EOE
@@ -580,27 +559,23 @@ switch $f
{
case 1
{{
- diag false
false
}}
case 2
{{
- diag echo
echo
}}
default
{{
- diag false
false
}}
}
dump alias{y}
EOI
-<stdin>:22:1: dump:
+<stdin>:19:1: dump:
% .+/alias\{y\}: .+/:alias\{z\}%
- % perform(update)
+ % [diag=echo] perform(update)
{{
- diag echo
echo
}}
EOE