From 622f0c047c736278d7f06d0d885def72aaca703a Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sun, 8 Oct 2023 20:38:45 -0400 Subject: [PATCH] Fix reporting `line at wrong spot. Rework some internal fileline parsing functions. --- src/V3FileLine.cpp | 40 +++++++++++++++++++++++--------- src/V3FileLine.h | 4 ++++ src/V3ParseImp.cpp | 2 +- src/V3ParseImp.h | 3 +-- src/verilog.l | 2 +- src/verilog.y | 4 ++-- test_regress/t/t_pp_line_bad.out | 33 ++++++++++++++++---------- test_regress/t/t_pp_line_bad.pl | 1 + 8 files changed, 60 insertions(+), 29 deletions(-) diff --git a/src/V3FileLine.cpp b/src/V3FileLine.cpp index a8bf044a6..56f5090a1 100644 --- a/src/V3FileLine.cpp +++ b/src/V3FileLine.cpp @@ -208,17 +208,30 @@ string FileLine::lineDirectiveStrg(int enterExit) const { } void FileLine::lineDirective(const char* textp, int& enterExitRef) { + string newFilename; + int newLineno = -1; + lineDirectiveParse(textp, newFilename /*ref*/, newLineno /*ref*/, enterExitRef); + if (enterExitRef != -1) { + filename(newFilename); + lineno(newLineno); + } else { // Advance line number to account for bogus `line + linenoInc(); + } +} + +void FileLine::lineDirectiveParse(const char* textp, string& filenameRef, int& linenoRef, + int& enterExitRef) { // Handle `line directive // Does not parse streamNumber/streamLineno as the next input token // will come from the same stream as the previous line. do { - int lineNo; // Skip `line while (*textp && std::isspace(*textp)) ++textp; while (*textp && !std::isspace(*textp)) ++textp; while (*textp && std::isspace(*textp)) ++textp; // Grab linenumber + int lineNo; const char* const ln = textp; while (*textp && !std::isspace(*textp)) ++textp; if (0 == strncmp(ln, "`__LINE__", textp - ln)) { @@ -229,7 +242,6 @@ void FileLine::lineDirective(const char* textp, int& enterExitRef) { } else { break; // Fail } - lineno(lineNo); while (*textp && (std::isspace(*textp))) ++textp; // Grab filename @@ -240,7 +252,6 @@ void FileLine::lineDirective(const char* textp, int& enterExitRef) { string errMsg; const string& parsedFilename = VString::unquoteSVString(string{fn, textp}, errMsg); if (!errMsg.empty()) this->v3error(errMsg.c_str()); - filename(parsedFilename); ++textp; while (*textp && std::isspace(*textp)) ++textp; @@ -248,17 +259,16 @@ void FileLine::lineDirective(const char* textp, int& enterExitRef) { if (!std::isdigit(*textp)) break; // Fail const int level = std::atoi(textp); if (level < 0 || level >= 3) break; // Fail - /// TODO: store lineno/filename only when the `line directive is valid - /// lineno(lineNo); - /// filename(filenameNew); + + linenoRef = lineNo; + filenameRef = parsedFilename; enterExitRef = level; return; } while (false); // Fail - // TODO: show correct place of the code v3error("`line was not properly formed with '`line number \"filename\" level'\n"); - enterExitRef = 0; + enterExitRef = -1; } void FileLine::forwardToken(const char* textp, size_t size, bool trackLines) { @@ -273,14 +283,22 @@ void FileLine::forwardToken(const char* textp, size_t size, bool trackLines) { } } +void FileLine::applyIgnores() { +#ifndef V3ERROR_NO_GLOBAL_ + V3Config::applyIgnores(this); // Toggle warnings based on global config file +#endif +} + +FileLine* FileLine::copyOrSameFileLineApplied() { + applyIgnores(); + return copyOrSameFileLine(); +} + FileLine* FileLine::copyOrSameFileLine() { // When a fileline is "used" to produce a node, calls this function. // Return this, or a copy of this // There are often more than one token per line, thus we use the // same pointer as long as we're on the same line, file & warn state. -#ifndef V3ERROR_NO_GLOBAL_ - V3Config::applyIgnores(this); // Toggle warnings based on global config file -#endif static FileLine* lastNewp = nullptr; if (lastNewp && *lastNewp == *this) { // Compares lineno, filename, etc return lastNewp; diff --git a/src/V3FileLine.h b/src/V3FileLine.h index 15541f418..03cf71695 100644 --- a/src/V3FileLine.h +++ b/src/V3FileLine.h @@ -195,7 +195,9 @@ public: , m_parent{fromp->m_parent} { if (m_contentp) m_contentp->refInc(); } + void applyIgnores(); FileLine* copyOrSameFileLine(); + FileLine* copyOrSameFileLineApplied(); static void deleteAllRemaining(); ~FileLine(); #ifdef VL_LEAK_CHECKS @@ -221,6 +223,8 @@ public: void filename(const string& name) { m_filenameno = singleton().nameToNumber(name); } void parent(FileLine* fileline) { m_parent = fileline; } void lineDirective(const char* textp, int& enterExitRef); + void lineDirectiveParse(const char* textp, string& filenameRef, int& linenoRef, + int& enterExitRef); void linenoInc() { m_lastLineno++; m_lastColumn = 1; diff --git a/src/V3ParseImp.cpp b/src/V3ParseImp.cpp index 1da582004..0e082d402 100644 --- a/src/V3ParseImp.cpp +++ b/src/V3ParseImp.cpp @@ -72,7 +72,7 @@ V3ParseImp::~V3ParseImp() { void V3ParseImp::lexPpline(const char* textp) { // Handle lexer `line directive - FileLine* const prevFl = copyOrSameFileLine(); + FileLine* const prevFl = lexFileline()->copyOrSameFileLineApplied(); int enterExit; lexFileline()->lineDirective(textp, enterExit /*ref*/); if (enterExit == 1) { // Enter diff --git a/src/V3ParseImp.h b/src/V3ParseImp.h index 743b4ec14..439edf510 100644 --- a/src/V3ParseImp.h +++ b/src/V3ParseImp.h @@ -176,8 +176,8 @@ public: bool precSet, double precVal) VL_MT_DISABLED; VTimescale timeLastUnit() const { return m_timeLastUnit; } + void lexFileline(FileLine* fl) { m_lexFileline = fl; } FileLine* lexFileline() const { return m_lexFileline; } - FileLine* lexCopyOrSameFileLine() { return lexFileline()->copyOrSameFileLine(); } static void lexErrorPreprocDirective(FileLine* fl, const char* textp) VL_MT_DISABLED; static string lexParseTag(const char* textp) VL_MT_DISABLED; static double lexParseTimenum(const char* text) VL_MT_DISABLED; @@ -243,7 +243,6 @@ public: // Return next token, for bison, since bison isn't class based, use a global THIS AstNetlist* rootp() const { return m_rootp; } - FileLine* copyOrSameFileLine() { return bisonLastFileline()->copyOrSameFileLine(); } bool inLibrary() const { return m_inLibrary; } VOptionBool unconnectedDrive() const { return m_unconnectedDrive; } void unconnectedDrive(const VOptionBool flag) { m_unconnectedDrive = flag; } diff --git a/src/verilog.l b/src/verilog.l index 3ac7873b3..102a43195 100644 --- a/src/verilog.l +++ b/src/verilog.l @@ -40,7 +40,7 @@ // Use this to break between tokens whereever not return'ing a token (e.g. skipping inside lexer) #define FL_BRK (PARSEP->lexFileline()->startToken()) -#define CRELINE() (PARSEP->lexCopyOrSameFileLine()) +#define CRELINE() (PARSEP->lexFileline()->copyOrSameFileLineApplied()) #define FL \ do { \ diff --git a/src/verilog.y b/src/verilog.y index 31581e891..cfc01f62a 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -309,8 +309,8 @@ int V3ParseGrammar::s_modTypeImpNum = 0; //====================================================================== // Macro functions -#define CRELINE() \ - (PARSEP->copyOrSameFileLine()) // Only use in empty rules, so lines point at beginnings +// Only use in empty rules, so lines point at beginnings +#define CRELINE() (PARSEP->bisonLastFileline()->copyOrSameFileLineApplied()) #define FILELINE_OR_CRE(nodep) ((nodep) ? (nodep)->fileline() : CRELINE()) #define VARRESET_LIST(decl) \ diff --git a/test_regress/t/t_pp_line_bad.out b/test_regress/t/t_pp_line_bad.out index 8365b61c6..fc54a045d 100644 --- a/test_regress/t/t_pp_line_bad.out +++ b/test_regress/t/t_pp_line_bad.out @@ -1,22 +1,31 @@ -%Error: t/t_pp_line_bad.v:100:1: `line was not properly formed with '`line number "filename" level' - 100 | `line 100 +%Error: t/t_pp_line_bad.v:8:1: `line was not properly formed with '`line number "filename" level' + 8 | `line 100 | ^ -%Error: t/t_pp_line_bad.v:200:1: `line was not properly formed with '`line number "filename" level' - 200 | `line 100 +%Error: t/t_pp_line_bad.v:8:1: `line was not properly formed with '`line number "filename" level' +%Error: t/t_pp_line_bad.v:9:1: `line was not properly formed with '`line number "filename" level' + 9 | `line 200 somefile | ^ -%Error: t/t_pp_line_bad.v:300:1: `line was not properly formed with '`line number "filename" level' - 300 | `line 100 +%Error: t/t_pp_line_bad.v:9:1: `line was not properly formed with '`line number "filename" level' +%Error: t/t_pp_line_bad.v:10:1: `line was not properly formed with '`line number "filename" level' + 10 | `line 300 "somefile 1 | ^ -%Error: some file:400:1: `line was not properly formed with '`line number "filename" level' - 400 | `line 100 +%Error: t/t_pp_line_bad.v:10:1: `line was not properly formed with '`line number "filename" level' +%Error: t/t_pp_line_bad.v:11:1: `line was not properly formed with '`line number "filename" level' + 11 | `line 400 "some file" | ^ -%Error: somefile:500:1: `line was not properly formed with '`line number "filename" level' - 500 | `line 100 +%Error: t/t_pp_line_bad.v:11:1: `line was not properly formed with '`line number "filename" level' +%Error: t/t_pp_line_bad.v:12:1: `line was not properly formed with '`line number "filename" level' + 12 | `line 500 "somefile" 3 | ^ -%Error: some file:600:1: `line was not properly formed with '`line number "filename" level' - 600 | `line 100 +%Error: t/t_pp_line_bad.v:12:1: `line was not properly formed with '`line number "filename" level' +%Error: t/t_pp_line_bad.v:13:1: `line was not properly formed with '`line number "filename" level' + 13 | `line 600 "some file" 3 | ^ +%Error: t/t_pp_line_bad.v:13:1: `line was not properly formed with '`line number "filename" level' %Error: t/t_pp_line_bad.v:7:1: Define or directive not defined: '`line' 7 | `line | ^~~~~ +%Error: t/t_pp_line_bad.v:15:1: `line was not properly formed with '`line number "filename" level' +%Error: t/t_pp_line_bad.v:17:1: `line was not properly formed with '`line number "filename" level' +%Error: t/t_pp_line_bad.v:19:1: `line was not properly formed with '`line number "filename" level' %Error: Exiting due to diff --git a/test_regress/t/t_pp_line_bad.pl b/test_regress/t/t_pp_line_bad.pl index a60503a1f..887b07b64 100755 --- a/test_regress/t/t_pp_line_bad.pl +++ b/test_regress/t/t_pp_line_bad.pl @@ -11,6 +11,7 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di scenarios(linter => 1); lint( + verilator_flags2 => ['-no-std'], fails => 1, expect_filename => $Self->{golden_filename}, );