From 26750bd14c715bb616d1779f031b8f5b0ced1bbf Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 8 Jun 2021 15:30:35 +0200 Subject: Redo low verbosity diagnostic deduction to use scope instead of target --- libbuild2/build/script/parser.cxx | 30 ++++++++++++------------------ libbuild2/build/script/parser.hxx | 16 +++++++++++++--- libbuild2/build/script/parser.test.cxx | 2 +- 3 files changed, 26 insertions(+), 22 deletions(-) (limited to 'libbuild2/build') 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& as, istream& is, const path_name& pn, uint64_t line, optional 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 (tg); + target_ = nullptr; actions_ = &as; - scope_ = const_cast (tg != nullptr ? &tg->base_scope () : &bs); + scope_ = const_cast (&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 (&rs); scope_ = const_cast (&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&, + pre_parse (const scope&, const small_vector&, istream&, const path_name&, uint64_t line, optional 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 -- cgit v1.1