From f3e193b2651b2589daecaf181b96c5622acc51e9 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Fri, 16 Jun 2017 14:57:47 +0200 Subject: Clean up module-related diagnostics --- build2/cc/compile.cxx | 30 ++++++++++++++++++++++++------ build2/cc/compile.hxx | 2 +- build2/diagnostics.hxx | 23 ++++++++++++++++++++++- build2/utility.cxx | 9 +++++++++ build2/utility.hxx | 5 +++++ tests/cc/modules/testscript | 4 ++-- tests/cc/preprocessed/testscript | 2 +- 7 files changed, 64 insertions(+), 11 deletions(-) diff --git a/build2/cc/compile.cxx b/build2/cc/compile.cxx index a7ad2fa..265b1a9 100644 --- a/build2/cc/compile.cxx +++ b/build2/cc/compile.cxx @@ -2451,7 +2451,7 @@ namespace build2 if (!modules) { if (!mi.name.empty () || !mi.imports.empty ()) - fail << "modules support not enabled or unavailable"; + fail (relative (src)) << "modules support not enabled/available"; return; } @@ -2482,7 +2482,7 @@ namespace build2 if (!mi.imports.empty ()) md.mod_pos = search_modules ( - act, t, lo, tt.bmi, move (mi.imports), cs); + act, t, lo, tt.bmi, src, move (mi.imports), cs); if (dd.expect (cs.string ()) != nullptr) updating = true; @@ -2508,6 +2508,7 @@ namespace build2 file& t, lorder lo, const target_type& mtt, + const file& src, module_imports&& imports, sha256& cs) const { @@ -2769,7 +2770,7 @@ namespace build2 // for bmi{} to have a different name). // if (p.is_a ()) - pt = &search (t, mtt, p.key ()); //@@ MOD: fuzzy... + pt = &search (t, mtt, p.key ()); // Same logic as in picking obj*{}. else if (p.is_a (mtt)) { if (pt == nullptr) @@ -2821,9 +2822,25 @@ namespace build2 { if (pts[start + i] == nullptr) { - // @@ MOD: keep import location for diagnostics? + // It would have been nice to print the location of the import + // declaration. And we could save it during parsing at the expense + // of a few paths (that can be pooled). The question is what to do + // when we re-create this information from depdb? We could have + // saved the location information there but the relative paths + // (e.g., from the #line directives) could end up being wrong if + // the we re-run from a different working directory. // - fail << "unresolved import for module " << imports[i].name; + // It seems the only workable approach is to extract full location + // info during parse, not save it in depdb, when re-creating, + // fallback to just src path without any line/column information. + // This will probably cover the majority of case (most of the time + // it will be a misspelled module name, not a removal of module + // from buildfile). + // + // But at this stage this doesn't seem worth the trouble. + // + fail (relative (src)) << "unable to resolve module " + << imports[i].name; } } } @@ -2858,7 +2875,8 @@ namespace build2 { if (p.is_a (*x_mod)) // Got to be there. { - fail << "failed to correctly guess module name from " << p << + fail (relative (src)) << "failed to correctly guess module " + << "name from " << p << info << "guessed: " << in << info << "actual: " << mn << info << "consider adjusting module interface file names or" << diff --git a/build2/cc/compile.hxx b/build2/cc/compile.hxx index d1b14bc..be4a2e2 100644 --- a/build2/cc/compile.hxx +++ b/build2/cc/compile.hxx @@ -113,7 +113,7 @@ namespace build2 modules_positions search_modules (action, file&, lorder, const target_type&, - module_imports&&, sha256&) const; + const file&, module_imports&&, sha256&) const; void append_modules (cstrings&, strings&, const file&) const; diff --git a/build2/diagnostics.hxx b/build2/diagnostics.hxx index ce5f996..1e76099 100644 --- a/build2/diagnostics.hxx +++ b/build2/diagnostics.hxx @@ -224,7 +224,18 @@ namespace build2 const char* name, const location& l, uint16_t sverb) - : type_ (type), mod_ (mod), name_ (name), loc_ (l), sverb_ (sverb) {} + : type_ (type), mod_ (mod), name_ (name), + loc_ (l), + sverb_ (sverb) {} + + location_prologue_base (const char* type, + const char* mod, + const char* name, + path&& f, + uint16_t sverb) + : type_ (type), mod_ (mod), name_ (name), + file_ (move (f)), loc_ (&file_), + sverb_ (sverb) {} void operator() (const diag_record& r) const; @@ -233,6 +244,7 @@ namespace build2 const char* type_; const char* mod_; const char* name_; + const path file_; const location loc_; const uint16_t sverb_; }; @@ -265,6 +277,15 @@ namespace build2 return location_prologue (epilogue_, type_, mod_, name_, l, sverb_ ()); } + // fail (relative (src)) << ... + // + location_prologue + operator() (path&& f) const + { + return location_prologue ( + epilogue_, type_, mod_, name_, move (f), sverb_ ()); + } + template location_prologue operator() (const L& l) const diff --git a/build2/utility.cxx b/build2/utility.cxx index 2b3f785..f2d9155 100644 --- a/build2/utility.cxx +++ b/build2/utility.cxx @@ -10,6 +10,7 @@ #include // strtol() #include // cerr +#include #include #include @@ -125,6 +126,14 @@ namespace build2 dir_path home; const dir_path* relative_base = &work; + path + relative (const path_target& t) + { + const path& p (t.path ()); + assert (!p.empty ()); + return relative (p); + } + string diag_relative (const path& p, bool cur) { diff --git a/build2/utility.hxx b/build2/utility.hxx index 4ce3407..bb0eb8e 100644 --- a/build2/utility.hxx +++ b/build2/utility.hxx @@ -135,6 +135,11 @@ namespace build2 basic_path relative (const basic_path&); + class path_target; + + path + relative (const path_target&); + // In addition to calling relative(), this function also uses shorter // notations such as '~/'. For directories the result includes the trailing // slash. If the path is the same as base, returns "./" if current is true diff --git a/tests/cc/modules/testscript b/tests/cc/modules/testscript index c259014..50dc9b2 100644 --- a/tests/cc/modules/testscript +++ b/tests/cc/modules/testscript @@ -160,7 +160,7 @@ $* test clean <>EOE != 0 - error: unresolved import for module foo.core + driver.cxx: error: unable to resolve module foo.core EOE : misguessed @@ -168,7 +168,7 @@ $* test &*.d <'exe{test}: cxx{driver}' 2>>EOE != 0 ln -s ../core.mxx ./; cat <'import bar.core;' >=driver.cxx; $* test &*.d <'exe{test}: cxx{driver} mxx{core}' 2>>EOE != 0 - error: failed to correctly guess module name from mxx{core} + driver.cxx: error: failed to correctly guess module name from mxx{core} info: guessed: bar.core info: actual: foo.core info: consider adjusting module interface file names or diff --git a/tests/cc/preprocessed/testscript b/tests/cc/preprocessed/testscript index 8875f7a..b429853 100644 --- a/tests/cc/preprocessed/testscript +++ b/tests/cc/preprocessed/testscript @@ -98,7 +98,7 @@ $* &test* <>EOE != 0 cc.preprocessed = modules exe{test}: cxx{test} EOI - error: modules support not enabled or unavailable + test.cxx: error: modules support not enabled/available EOE : all -- cgit v1.1