From 173efbc829585a0725b613515df3b9d8a194e78a Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 13 Jul 2019 20:30:32 -0400 Subject: [PATCH] Offer suggestions on bad identifier errors. --- Changes | 2 + src/V3Ast.h | 10 +- src/V3LanguageWords.h | 59 +++++++++ src/V3LinkDot.cpp | 112 +++++++++++++++-- src/V3ParseImp.cpp | 16 +++ src/V3ParseImp.h | 1 + src/V3PreProc.cpp | 10 +- src/V3PreProc.h | 2 + src/V3PreShell.cpp | 3 + src/V3PreShell.h | 2 + src/V3String.cpp | 139 ++++++++++++++++++++- src/V3String.h | 44 +++++++ src/V3SymTable.h | 18 +++ src/Verilator.cpp | 3 +- src/verilog.l | 5 +- test_regress/t/t_interface_modport_bad.out | 1 + test_regress/t/t_lint_implicit_bad.out | 2 + test_regress/t/t_lint_pindup_bad.out | 15 ++- test_regress/t/t_lint_pindup_bad.v | 14 ++- test_regress/t/t_package_export_bad.out | 6 + test_regress/t/t_pp_misdef_bad.out | 5 +- test_regress/t/t_pp_misdef_bad.v | 9 +- test_regress/t/t_var_dup_bad.out | 3 + test_regress/t/t_var_notfound_bad.out | 3 + test_regress/t/t_var_suggest_bad.out | 2 + 25 files changed, 458 insertions(+), 28 deletions(-) diff --git a/Changes b/Changes index f8d31bf9e..972eff6f1 100644 --- a/Changes +++ b/Changes @@ -4,6 +4,8 @@ The contributors that suggested a given feature are shown in []. Thanks! * Verilator 4.017 devel +** Offer suggestions on bad identifier errors. + *** Change MULTITOP to warning to help linting, see manual. **** Show included-from filenames in warnings, bug1439. [Todd Strader] diff --git a/src/V3Ast.h b/src/V3Ast.h index 7e41f1385..08105ecc4 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -57,7 +57,7 @@ typedef std::set MTaskIdSet; // Set of mtaskIds for Var sorting #define VN_IS(nodep,nodetypename) (AstNode::privateIs ## nodetypename(nodep)) // (V)erilator (N)ode cast: Cast to given type if can; effectively -// dynamic_cast(nodep)(nodetypename) +// dynamic_cast(nodep) #define VN_CAST(nodep,nodetypename) (AstNode::privateCast ## nodetypename(nodep)) #define VN_CAST_CONST(nodep,nodetypename) (AstNode::privateConstCast ## nodetypename(nodep) ) @@ -1114,6 +1114,14 @@ public: }; std::ostream& operator<<(std::ostream& os, const V3Hash& rhs); +//###################################################################### +// Callback base class to determine if node matches some formula + +class VNodeMatcher { +public: + virtual bool nodeMatch(const AstNode* nodep) const { return true; } +}; + //###################################################################### // AstNode -- Base type of all Ast types diff --git a/src/V3LanguageWords.h b/src/V3LanguageWords.h index 2c7539095..3f3f1f73b 100644 --- a/src/V3LanguageWords.h +++ b/src/V3LanguageWords.h @@ -41,7 +41,10 @@ class V3LanguageWords { void init(); }; public: + typedef KeywordMap::const_iterator const_iterator; // METHODS + static const_iterator begin() { return s().s_kwdMap.begin(); } + static const_iterator end() { return s().s_kwdMap.end(); } static string isKeyword(const string& kwd) { KeywordMap::iterator it = s().s_kwdMap.find(kwd); if (it == s().s_kwdMap.end()) return ""; @@ -187,6 +190,62 @@ inline void V3LanguageWords::Singleton::init() { addKwd("sensitive", "SystemC common word"); addKwd("sensitive_neg", "SystemC common word"); addKwd("sensitive_pos", "SystemC common word"); + // Preprocessor defined words + addKwd("`__FILE__", "Verilog preprocessor directive"); + addKwd("`__LINE__", "Verilog preprocessor directive"); + addKwd("`accelerate", "Verilog-XL preprocessor directive"); + addKwd("`autoexpand_vectornets", "Verilog-XL preprocessor directive"); + addKwd("`begin_keywords", "Verilog preprocessor directive"); + addKwd("`celldefine", "Verilog preprocessor directive"); + addKwd("`default_decay_time", "Verilog preprocessor directive"); + addKwd("`default_nettype", "Verilog preprocessor directive"); + addKwd("`default_trireg_strength", "Verilog preprocessor directive"); + addKwd("`define", "Verilog preprocessor directive"); + addKwd("`delay_mode_distributed", "Verilog preprocessor directive"); + addKwd("`delay_mode_path", "Verilog preprocessor directive"); + addKwd("`delay_mode_unit", "Verilog preprocessor directive"); + addKwd("`delay_mode_zero", "Verilog preprocessor directive"); + addKwd("`disable_portfaults", "Verilog-XL preprocessor directive"); + addKwd("`else", "Verilog preprocessor directive"); + addKwd("`elsif", "Verilog preprocessor directive"); + addKwd("`enable_portfaults", "Verilog-XL preprocessor directive"); + addKwd("`end_keywords", "Verilog preprocessor directive"); + addKwd("`endcelldefine", "Verilog preprocessor directive"); + addKwd("`endif", "Verilog preprocessor directive"); + addKwd("`endprotect", "Verilog preprocessor directive"); + addKwd("`endprotected", "Verilog preprocessor directive"); + addKwd("`error", "Verilog preprocessor directive"); + addKwd("`expand_vectornets", "Verilog-XL preprocessor directive"); + addKwd("`ifdef", "Verilog preprocessor directive"); + addKwd("`ifndef", "Verilog preprocessor directive"); + addKwd("`include", "Verilog preprocessor directive"); + addKwd("`inline", "Verilog preprocessor directive"); + addKwd("`line", "Verilog preprocessor directive"); + addKwd("`noaccelerate", "Verilog-XL preprocessor directive"); + addKwd("`noexpand_vectornets", "Verilog-XL preprocessor directive"); + addKwd("`noremove_gatenames", "Verilog-XL preprocessor directive"); + addKwd("`noremove_netnames", "Verilog-XL preprocessor directive"); + addKwd("`nosuppress_faults", "Verilog-XL preprocessor directive"); + addKwd("`nounconnected_drive", "Verilog-XL preprocessor directive"); + addKwd("`portcoerce", "Verilog preprocessor directive"); + addKwd("`pragma", "Verilog preprocessor directive"); + addKwd("`protect", "Verilog preprocessor directive"); + addKwd("`protected", "Verilog preprocessor directive"); + addKwd("`remove_gatenames", "Verilog-XL preprocessor directive"); + addKwd("`remove_netnames", "Verilog-XL preprocessor directive"); + addKwd("`resetall", "Verilog preprocessor directive"); + addKwd("`suppress_faults", "Verilog-XL preprocessor directive"); + addKwd("`systemc_ctor", "Verilator preprocessor directive"); + addKwd("`systemc_dtor", "Verilator preprocessor directive"); + addKwd("`systemc_header", "Verilator preprocessor directive"); + addKwd("`systemc_imp_header", "Verilator preprocessor directive"); + addKwd("`systemc_implementation", "Verilator preprocessor directive"); + addKwd("`systemc_interface", "Verilator preprocessor directive"); + addKwd("`timescale", "Verilog preprocessor directive"); + addKwd("`undef", "Verilog preprocessor directive"); + addKwd("`undefineall", "Verilog preprocessor directive"); + addKwd("`verilator_config", "Verilator preprocessor directive"); + addKwd("`verilog", "Verilator preprocessor directive"); } #endif // Guard diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index 738b950d2..48d7a39ca 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -79,6 +79,50 @@ #include #include +//###################################################################### +// Matcher classes (for suggestion matching) + +class LinkNodeMatcherFTask : public VNodeMatcher { +public: + virtual bool nodeMatch(const AstNode* nodep) const { + return VN_IS(nodep, NodeFTask); + } +}; +class LinkNodeMatcherModport : public VNodeMatcher { +public: + virtual bool nodeMatch(const AstNode* nodep) const { + return VN_IS(nodep, Modport); + } +}; +class LinkNodeMatcherVar : public VNodeMatcher { +public: + virtual bool nodeMatch(const AstNode* nodep) const { + return VN_IS(nodep, Var); + } +}; +class LinkNodeMatcherVarIO : public VNodeMatcher { +public: + virtual bool nodeMatch(const AstNode* nodep) const { + const AstVar* varp = VN_CAST_CONST(nodep, Var); + if (!varp) return false; + return varp->isIO(); + } +}; +class LinkNodeMatcherVarParam : public VNodeMatcher { +public: + virtual bool nodeMatch(const AstNode* nodep) const { + const AstVar* varp = VN_CAST_CONST(nodep, Var); + if (!varp) return false; + return varp->isParam(); + } +}; +class LinkNodeMatcherVarOrScope : public VNodeMatcher { +public: + virtual bool nodeMatch(const AstNode* nodep) const { + return VN_IS(nodep, Var) || VN_IS(nodep, Scope); + } +}; + //###################################################################### // LinkDot state, as a visitor of each AstNode @@ -428,9 +472,14 @@ public: ok = true; } } - if (!ok) ifacerefp->v3error("Modport not found under interface " - <prettyNameQ(ifacerefp->ifaceName()) - <<": "<prettyNameQ(ifacerefp->modportName())); + if (!ok) { + string suggest = suggestSymFallback( + ifaceSymp, ifacerefp->modportName(), LinkNodeMatcherModport()); + ifacerefp->v3error("Modport not found under interface " + <prettyNameQ(ifacerefp->ifaceName())<<": " + <prettyNameQ(ifacerefp->modportName())<warnMore()+suggest)); + } } // Alias won't expand until interfaces and modport names are known; see notes at top insertScopeAlias(SAMN_IFTOP, varSymp, ifOrPortSymp); @@ -601,6 +650,23 @@ public: if (!foundp) baddot = dotname; return foundp; } + string suggestSymFallback(VSymEnt* lookupSymp, const string& name, + const VNodeMatcher& matcher) { + // Suggest alternative symbol in given point in hierarchy + // Does not support inline, as we find user-level errors before inlining + // For simplicity lookupSymp may be passed NULL result from findDotted + if (!lookupSymp) return ""; + VSpellCheck speller; + lookupSymp->candidateIdFallback(&speller, &matcher); + return speller.bestCandidateMsg(name); + } + string suggestSymFlat(VSymEnt* lookupSymp, const string& name, + const VNodeMatcher& matcher) { + if (!lookupSymp) return ""; + VSpellCheck speller; + lookupSymp->candidateIdFlat(&speller, &matcher); + return speller.bestCandidateMsg(name); + } }; LinkDotState* LinkDotState::s_errorThisp = NULL; @@ -1611,11 +1677,21 @@ private: if (!nodep->varp()) { if (!noWarn) { if (nodep->fileline()->warnIsOff(V3ErrorCode::I_DEF_NETTYPE_WIRE)) { + string suggest = m_statep->suggestSymFallback( + moduleSymp, nodep->name(), LinkNodeMatcherVar()); nodep->v3error("Signal definition not found, and implicit disabled with `default_nettype: " - <prettyNameQ()); - } else { + <prettyNameQ()<warnMore()+suggest)); + + } + // Bypass looking for suggestions if IMPLICIT is turned off + // as there could be thousands of these suppressed in large netlists + else if (!nodep->fileline()->warnIsOff(V3ErrorCode::IMPLICIT)) { + string suggest = m_statep->suggestSymFallback( + moduleSymp, nodep->name(), LinkNodeMatcherVar()); nodep->v3warn(IMPLICIT, "Signal definition not found, creating implicitly: " - <prettyNameQ()); + <prettyNameQ()<warnMore()+suggest)); } } AstVar* newp = new AstVar(nodep->fileline(), AstVarType::WIRE, @@ -1762,7 +1838,14 @@ private: nodep->unlinkFrBack()->deleteTree(); VL_DANGLING(nodep); return; } - nodep->v3error(ucfirst(whatp)<<" not found: "<prettyNameQ()); + string suggest + = (nodep->param() + ? m_statep->suggestSymFlat( + m_pinSymp, nodep->name(), LinkNodeMatcherVarParam()) + : m_statep->suggestSymFlat( + m_pinSymp, nodep->name(), LinkNodeMatcherVarIO())); + nodep->v3error(ucfirst(whatp)<<" not found: "<prettyNameQ()<warnMore()+suggest)); } else if (AstVar* refp = VN_CAST(foundp->nodep(), Var)) { if (!refp->isIO() && !refp->isParam() && !refp->isIfaceRef()) { @@ -2050,8 +2133,11 @@ private: } else if (m_ds.m_dotText=="") { UINFO(7," ErrParseRef curSymp=se"<prettyNameQ()); + <<": "<prettyNameQ()<warnMore()+suggest)); } else { nodep->v3error("Can't find definition of '" <<(baddot!="" ? baddot : nodep->prettyName()) @@ -2274,12 +2360,18 @@ private: <<"'"<<" as a "<nodep()->typeName() <<" but expected a task/function"); } else if (nodep->dotted() == "") { + string suggest = m_statep->suggestSymFallback( + dotSymp, nodep->name(), LinkNodeMatcherFTask()); nodep->v3error("Can't find definition of task/function: " - <prettyNameQ()); + <prettyNameQ()<warnMore()+suggest)); } else { + string suggest = m_statep->suggestSymFallback( + dotSymp, nodep->name(), LinkNodeMatcherFTask()); nodep->v3error("Can't find definition of '"<dotted()+"."+nodep->prettyName()<<"'"); + <dotted()+"."+nodep->prettyName()<<"'\n" + <<(suggest.empty() ? "" : nodep->warnMore()+suggest)); okSymp->cellErrorScopes(nodep); } } diff --git a/src/V3ParseImp.cpp b/src/V3ParseImp.cpp index 81275f83f..5c97e24d5 100644 --- a/src/V3ParseImp.cpp +++ b/src/V3ParseImp.cpp @@ -36,6 +36,7 @@ #include "V3File.h" #include "V3ParseImp.h" #include "V3PreShell.h" +#include "V3LanguageWords.h" #include #include @@ -132,6 +133,21 @@ void V3ParseImp::verilatorCmtBad(const char* textp) { } } +void V3ParseImp::errorPreprocDirective(const char* textp) { + // Find all `preprocessor spelling candidates + // Can't make this static as might get more defines later when read cells + VSpellCheck speller; + V3LanguageWords words; + for (V3LanguageWords::const_iterator it = words.begin(); it != words.end(); ++it) { + string ppDirective = it->first; + if (ppDirective[0] == '`') speller.pushCandidate(ppDirective); + } + V3PreShell::candidateDefines(&speller); + string suggest = speller.bestCandidateMsg(textp); + fileline()->v3error("Define or directive not defined: '"<warnMore()+suggest)); +} + void V3ParseImp::tag(const char* text) { if (m_tagNodep) { string tmp = text + strlen("/*verilator tag "); diff --git a/src/V3ParseImp.h b/src/V3ParseImp.h index ef19ef6a9..4078e8293 100644 --- a/src/V3ParseImp.h +++ b/src/V3ParseImp.h @@ -148,6 +148,7 @@ public: void verilatorCmtLintSave(); void verilatorCmtLintRestore(); void verilatorCmtBad(const char* text); + void errorPreprocDirective(const char* textp); void tag(const char* text); void tagNodep(AstNode* nodep) { m_tagNodep = nodep; } AstNode* tagNodep() const { return m_tagNodep;} diff --git a/src/V3PreProc.cpp b/src/V3PreProc.cpp index d4df1eed1..751e0f35b 100644 --- a/src/V3PreProc.cpp +++ b/src/V3PreProc.cpp @@ -27,6 +27,7 @@ #include "V3PreLex.h" #include "V3PreProc.h" #include "V3PreShell.h" +#include "V3String.h" #include #include @@ -233,6 +234,7 @@ public: void insertUnreadbackAtBol(const string& text); void addLineComment(int enterExit); void dumpDefines(std::ostream& os); + void candidateDefines(VSpellCheck* spellerp); // METHODS, callbacks virtual void comment(const string& text); // Comment detected (if keepComments==2) @@ -818,7 +820,7 @@ void V3PreProcImp::addLineComment(int enterExit) { } void V3PreProcImp::dumpDefines(std::ostream& os) { - for (DefinesMap::iterator it = m_defines.begin(); it != m_defines.end(); ++it) { + for (DefinesMap::const_iterator it = m_defines.begin(); it != m_defines.end(); ++it) { os<<"`define "<first; // No need to print "()" below as already part of params() if (!it->second.params().empty()) os<second.params(); @@ -827,6 +829,12 @@ void V3PreProcImp::dumpDefines(std::ostream& os) { } } +void V3PreProcImp::candidateDefines(VSpellCheck* spellerp) { + for (DefinesMap::const_iterator it = m_defines.begin(); it != m_defines.end(); ++it) { + spellerp->pushCandidate(string("`") + it->first); + } +} + int V3PreProcImp::getRawToken() { // Get a token from the file, whatever it may be. while (1) { diff --git a/src/V3PreProc.h b/src/V3PreProc.h index defc3e8ae..4e0424cc8 100644 --- a/src/V3PreProc.h +++ b/src/V3PreProc.h @@ -35,6 +35,7 @@ #define fatalSrc(msg) v3fatalSrc(msg) class V3InFilter; +class VSpellCheck; class V3PreProc { // This defines a preprocessor. Functions are virtual so implementation can be hidden. @@ -95,6 +96,7 @@ public: void error(const string& msg) { fileline()->v3error(msg); } ///< Report an error void fatal(const string& msg) { fileline()->v3fatalSrc(msg); } ///< Report a fatal error virtual void dumpDefines(std::ostream& os) = 0; ///< Print list of `defines + virtual void candidateDefines(VSpellCheck* spellerp) = 0; ///< Spell check candidate defines protected: // CONSTUCTORS diff --git a/src/V3PreShell.cpp b/src/V3PreShell.cpp index 57b5a856b..5a08ace0f 100644 --- a/src/V3PreShell.cpp +++ b/src/V3PreShell.cpp @@ -187,3 +187,6 @@ void V3PreShell::undef(const string& name) { void V3PreShell::dumpDefines(std::ostream& os) { V3PreShellImp::s_preprocp->dumpDefines(os); } +void V3PreShell::candidateDefines(VSpellCheck* spellerp) { + V3PreShellImp::s_preprocp->candidateDefines(spellerp); +} diff --git a/src/V3PreShell.h b/src/V3PreShell.h index b4368571a..91f56132c 100644 --- a/src/V3PreShell.h +++ b/src/V3PreShell.h @@ -29,6 +29,7 @@ class V3ParseImp; class V3InFilter; +class VSpellCheck; //============================================================================ @@ -43,6 +44,7 @@ public: static void defineCmdLine(const string& name, const string& value); static void undef(const string& name); static void dumpDefines(std::ostream& os); + static void candidateDefines(VSpellCheck* spellerp); }; #endif // Guard diff --git a/src/V3String.cpp b/src/V3String.cpp index af56de8d6..574b8879f 100644 --- a/src/V3String.cpp +++ b/src/V3String.cpp @@ -25,6 +25,8 @@ #include "V3String.h" #include "V3Error.h" +#include + size_t VName::s_minLength = 32; size_t VName::s_maxLength = 0; // Disabled @@ -273,7 +275,7 @@ string VHashSha1::digestSymbol() { } void VHashSha1::selfTestOne(const string& data, const string& data2, - const string& exp, const string& exp64) { + const string& exp, const string& exp64) { VHashSha1 digest (data); if (data2!="") digest.insert(data2); if (VL_UNCOVERABLE(digest.digestHex() != exp)) { @@ -329,3 +331,138 @@ string VName::hashedName() { return m_hashed; } } + +//###################################################################### +// VSpellCheck - Algorithm same as GCC's spellcheck.c + +VSpellCheck::EditDistance VSpellCheck::editDistance(const string& s, const string& t) { + // Wagner-Fischer algorithm for the Damerau-Levenshtein distance + size_t sLen = s.length(); + size_t tLen = t.length(); + if (sLen == 0) return tLen; + if (tLen == 0) return sLen; + if (sLen >= LENGTH_LIMIT) return sLen; + if (tLen >= LENGTH_LIMIT) return tLen; + + static EditDistance s_v_two_ago[LENGTH_LIMIT + 1]; + static EditDistance s_v_one_ago[LENGTH_LIMIT + 1]; + static EditDistance s_v_next[LENGTH_LIMIT + 1]; + + for (int i = 0; i < sLen + 1; i++) s_v_one_ago[i] = i; + + for (int i = 0; i < tLen; i++) { + s_v_next[0] = i + 1; + for (int j = 0; j < sLen; j++) { + EditDistance cost =(s[j] == t[i] ? 0 : 1); + EditDistance deletion = s_v_next[j] + 1; + EditDistance insertion = s_v_one_ago[j + 1] + 1; + EditDistance substitution = s_v_one_ago[j] + cost; + EditDistance cheapest = std::min(deletion, insertion); + cheapest = std::min(cheapest, substitution); + if (i > 0 && j > 0 && s[j] == t[i - 1] && s[j - 1] == t[i]) { + EditDistance transposition = s_v_two_ago[j - 1] + 1; + cheapest = std::min(cheapest, transposition); + } + s_v_next[j + 1] = cheapest; + } + for (int j = 0; j < sLen + 1; j++) { + s_v_two_ago[j] = s_v_one_ago[j]; + s_v_one_ago[j] = s_v_next[j]; + } + } + + EditDistance result = s_v_next[sLen]; + return result; +} + +VSpellCheck::EditDistance VSpellCheck::cutoffDistance(size_t goal_len, size_t candidate_len) { + // Return max acceptable edit distance + size_t max_length = std::max(goal_len, candidate_len); + size_t min_length = std::min(goal_len, candidate_len); + if (max_length <= 1) return 0; + if (max_length - min_length <= 1) return std::max(max_length / 3, (size_t)1); + return (max_length + 2) / 3; +} + +string VSpellCheck::bestCandidateInfo(const string& goal, + EditDistance& distancer) { + string bestCandidate; + size_t gLen = goal.length(); + distancer = LENGTH_LIMIT*10; + int suggestionLimit = 1000; // Avoid searching massive netlists + for (Candidates::const_iterator it = m_candidates.begin(); + it != m_candidates.end(); ++it) { + const string candidate = *it; + size_t cLen = candidate.length(); + + // Min distance must be inserting/deleting to make lengths match + EditDistance min_distance = (cLen > gLen ? (cLen - gLen) : (gLen - cLen)); + if (min_distance >= distancer) continue; // Short-circuit if already better + + EditDistance cutoff = cutoffDistance(gLen, cLen); + if (min_distance > cutoff) continue; // Short-circuit if already too bad + + EditDistance dist = editDistance(goal, candidate); + UINFO(9, "EditDistance dist="< Candidates; + // MEMBERS + Candidates m_candidates; // Strings we try to match +public: + // CONSTRUCTORS + explicit VSpellCheck() {} + ~VSpellCheck() {} + // METHODS + // Push a symbol table value to be considered as a candidate + // The first item pushed has highest priority, all else being equal + void pushCandidate(const string& s) { + if (m_candidates.size() < NUM_CANDIDATE_LIMIT) m_candidates.push_back(s); + } + // Return candidate is closest to provided string, or "" for none + string bestCandidate(const string& goal) { + EditDistance dist; + return bestCandidateInfo(goal, dist/*ref*/); + } + // Return friendly message + string bestCandidateMsg(const string& goal) { + string candidate = bestCandidate(goal); + if (candidate.empty()) return ""; + else return string("... Suggested alternative: '")+candidate+"'"; + } + static void selfTest(); +private: + static EditDistance editDistance(const string& s, const string& t); + static EditDistance cutoffDistance(size_t goal_len, size_t candidate_len); + string bestCandidateInfo(const string& goal, EditDistance& distancer); + static void selfTestDistanceOne(const string& a, const string& b, + EditDistance expected); + static void selfTestSuggestOne(bool matches, const string& c, const string& goal, + EditDistance dist); +}; + //###################################################################### #endif // guard diff --git a/src/V3SymTable.h b/src/V3SymTable.h index 1d9d5d925..e1dd32745 100644 --- a/src/V3SymTable.h +++ b/src/V3SymTable.h @@ -27,6 +27,7 @@ #include "V3Global.h" #include "V3Ast.h" #include "V3File.h" +#include "V3String.h" #include #include @@ -158,6 +159,23 @@ public: if (m_fallbackp) return m_fallbackp->findIdFallback(name); return NULL; } + void candidateIdFlat(VSpellCheck* spellerp, const VNodeMatcher* matcherp) const { + // Suggest alternative symbol candidates without looking upward through symbol hierarchy + for (IdNameMap::const_iterator it = m_idNameMap.begin(); + it != m_idNameMap.end(); ++it) { + const AstNode* nodep = it->second->nodep(); + if (nodep && (!matcherp || matcherp->nodeMatch(nodep))) { + spellerp->pushCandidate(nodep->prettyName()); + } + } + } + void candidateIdFallback(VSpellCheck* spellerp, const VNodeMatcher* matcherp) const { + // Suggest alternative symbol candidates with looking upward through symbol hierarchy + // Note VSpellCheck wants the most important (closest) items pushed first + candidateIdFlat(spellerp, matcherp); + // Then suggest the upper begin/end block or module + if (m_fallbackp) m_fallbackp->candidateIdFallback(spellerp, matcherp); + } private: void importOneSymbol(VSymGraph* graphp, const string& name, const VSymEnt* srcp) { if (srcp->exported() diff --git a/src/Verilator.cpp b/src/Verilator.cpp index a7c73173e..9c09e68e7 100644 --- a/src/Verilator.cpp +++ b/src/Verilator.cpp @@ -622,9 +622,10 @@ int main(int argc, char** argv, char** env) { // Internal tests (after option parsing as need debug() setting, // and after removing files as may make debug output) - VHashSha1::selfTest(); AstBasicDTypeKwd::selfTest(); if (v3Global.opt.debugSelfTest()) { + VHashSha1::selfTest(); + VSpellCheck::selfTest(); V3Graph::selfTest(); V3TSP::selfTest(); V3ScoreboardBase::selfTest(); diff --git a/src/verilog.l b/src/verilog.l index 2855637bc..a2a10b176 100644 --- a/src/verilog.l +++ b/src/verilog.l @@ -884,6 +884,7 @@ vnum {vnum1}|{vnum2}|{vnum3}|{vnum4}|{vnum5} /* Preprocessor */ /* Common for all SYSC header states */ /* OPTIMIZE: we return one per line, make it one for the entire block */ + /* If add to this list also add to V3LanguageWords.h */ { "`accelerate" { } // Verilog-XL compatibility "`autoexpand_vectornets" { } // Verilog-XL compatibility @@ -941,6 +942,8 @@ vnum {vnum1}|{vnum2}|{vnum3}|{vnum4}|{vnum5} "`systemc_interface" { BEGIN SYSCINT; } "`verilator_config" { BEGIN VLT; } "`verilog" { BEGIN PARSEP->lastVerilogState(); } + + /* If add to this list also add to V3LanguageWords.h */ } [ \t]*[^` \t\n\r][^\n\r]*{crnl} { FL; NEXTLINE(); yylval.strp = PARSEP->newString(yytext); return yaSCHDR; } @@ -961,7 +964,7 @@ vnum {vnum1}|{vnum2}|{vnum3}|{vnum4}|{vnum5} /* Default rules - leave last */ { - "`"[a-zA-Z_0-9]+ { FL; yyerrorf("Define or directive not defined: %s", yytext); } + "`"[a-zA-Z_0-9]+ { FL; PARSEP->errorPreprocDirective(yytext); } "//"[^\n]* { } /* throw away single line comments */ . { FL; return yytext[0]; } /* return single char ops. */ } diff --git a/test_regress/t/t_interface_modport_bad.out b/test_regress/t/t_interface_modport_bad.out index 433f4f639..e610d48b8 100644 --- a/test_regress/t/t_interface_modport_bad.out +++ b/test_regress/t/t_interface_modport_bad.out @@ -1,2 +1,3 @@ %Error: t/t_interface_modport_bad.v:22: Modport not found under interface 'ifc': 'oop_modport' + : ... Suggested alternative: 'out_modport' %Error: Exiting due to diff --git a/test_regress/t/t_lint_implicit_bad.out b/test_regress/t/t_lint_implicit_bad.out index 56a636472..6ef21f6a2 100644 --- a/test_regress/t/t_lint_implicit_bad.out +++ b/test_regress/t/t_lint_implicit_bad.out @@ -2,5 +2,7 @@ ... Use "/* verilator lint_off IMPLICIT */" and lint_on around source to disable this message. %Warning-IMPLICIT: t/t_lint_implicit.v:12: Signal definition not found, creating implicitly: 'nt0' %Warning-IMPLICIT: t/t_lint_implicit.v:15: Signal definition not found, creating implicitly: 'dummy1' + : ... Suggested alternative: 'dummy_ip' %Warning-IMPLICIT: t/t_lint_implicit.v:15: Signal definition not found, creating implicitly: 'dummy2' + : ... Suggested alternative: 'dummy1' %Error: Exiting due to diff --git a/test_regress/t/t_lint_pindup_bad.out b/test_regress/t/t_lint_pindup_bad.out index cde7efbb2..987229849 100644 --- a/test_regress/t/t_lint_pindup_bad.out +++ b/test_regress/t/t_lint_pindup_bad.out @@ -1,6 +1,13 @@ -%Error: t/t_lint_pindup_bad.v:17: Duplicate pin connection: 'i' - t/t_lint_pindup_bad.v:16: ... Location of original pin connection -%Error: t/t_lint_pindup_bad.v:18: Pin not found: '__pinNumber4' -%Error: t/t_lint_pindup_bad.v:14: Duplicate parameter pin connection: 'P' +%Warning-PINMISSING: t/t_lint_pindup_bad.v:18: Cell has missing pin: 'exists' + ... Use "/* verilator lint_off PINMISSING */" and lint_on around source to disable this message. +%Error: t/t_lint_pindup_bad.v:20: Duplicate pin connection: 'i' + t/t_lint_pindup_bad.v:19: ... Location of original pin connection +%Error: t/t_lint_pindup_bad.v:21: Pin not found: 'nexist' + : ... Suggested alternative: 'exists' +%Error: t/t_lint_pindup_bad.v:15: Parameter pin not found: 'NEXIST' + : ... Suggested alternative: 'EXIST' +%Error: t/t_lint_pindup_bad.v:16: Duplicate parameter pin connection: 'P' + t/t_lint_pindup_bad.v:14: ... Location of original parameter pin connection +%Error: t/t_lint_pindup_bad.v:17: Duplicate parameter pin connection: 'P' t/t_lint_pindup_bad.v:14: ... Location of original parameter pin connection %Error: Exiting due to diff --git a/test_regress/t/t_lint_pindup_bad.v b/test_regress/t/t_lint_pindup_bad.v index e521e9037..971de76d8 100644 --- a/test_regress/t/t_lint_pindup_bad.v +++ b/test_regress/t/t_lint_pindup_bad.v @@ -11,19 +11,25 @@ module t ); sub - #(, .P(2), .P(3)) + #(, // Not found + .NEXIST(1), // Not found + .P(2), + .P(3)) // Dup sub (.o(o), .i(i), - .i(i2), + .i(i2), // Dup + .nexist(i2) // Not found ); endmodule module sub - #(parameter P=1) + #(parameter P=1, + parameter EXIST=9) ( output wire o, - input wire i + input wire i, + input wire exists ); assign o = ~i; diff --git a/test_regress/t/t_package_export_bad.out b/test_regress/t/t_package_export_bad.out index dcb092a7c..e4b758b99 100644 --- a/test_regress/t/t_package_export_bad.out +++ b/test_regress/t/t_package_export_bad.out @@ -1,7 +1,13 @@ %Error: t/t_package_export.v:56: Can't find definition of scope/variable: 'PARAM2' + : ... Suggested alternative: 'PARAM1' %Error: t/t_package_export.v:57: Can't find definition of scope/variable: 'PARAM3' + : ... Suggested alternative: 'PARAM1' %Error: t/t_package_export.v:60: Can't find definition of scope/variable: 'PARAM2' + : ... Suggested alternative: 'PARAM1' %Error: t/t_package_export.v:61: Can't find definition of scope/variable: 'PARAM3' + : ... Suggested alternative: 'PARAM1' %Error: t/t_package_export.v:64: Can't find definition of scope/variable: 'PARAM2' + : ... Suggested alternative: 'PARAM1' %Error: t/t_package_export.v:65: Can't find definition of scope/variable: 'PARAM3' + : ... Suggested alternative: 'PARAM1' %Error: Exiting due to diff --git a/test_regress/t/t_pp_misdef_bad.out b/test_regress/t/t_pp_misdef_bad.out index ce4e90bf3..20111898f 100644 --- a/test_regress/t/t_pp_misdef_bad.out +++ b/test_regress/t/t_pp_misdef_bad.out @@ -1,2 +1,5 @@ -%Error: t/t_pp_misdef_bad.v:10: Define or directive not defined: `NOTDEF +%Error: t/t_pp_misdef_bad.v:10: Define or directive not defined: '`NDEFINED' + : ... Suggested alternative: '`DEFINED' +%Error: t/t_pp_misdef_bad.v:13: Define or directive not defined: '`imescale' + : ... Suggested alternative: '`timescale' %Error: Exiting due to diff --git a/test_regress/t/t_pp_misdef_bad.v b/test_regress/t/t_pp_misdef_bad.v index c1f00cf73..1a2eaa145 100644 --- a/test_regress/t/t_pp_misdef_bad.v +++ b/test_regress/t/t_pp_misdef_bad.v @@ -4,12 +4,13 @@ // without warranty, 2004 by Wilson Snyder. module t; -`define A B +`define DEFINED - // NOTDEF isn't defined here: - `NOTDEF + // NDEFINED isn't defined here: + `NDEFINED -//`include "notfound" + // Botched directive (`timescale) + `imescale initial $stop; // Should have failed diff --git a/test_regress/t/t_var_dup_bad.out b/test_regress/t/t_var_dup_bad.out index 821e5e8bd..a1cc32c01 100644 --- a/test_regress/t/t_var_dup_bad.out +++ b/test_regress/t/t_var_dup_bad.out @@ -28,9 +28,12 @@ %Error: t/t_var_dup_bad.v:40: Duplicate pin connection: 'bad_duport' t/t_var_dup_bad.v:40: ... Location of original pin connection %Error: t/t_var_dup_bad.v:41: Can't find definition of variable: 'bad_mixport' + : ... Suggested alternative: 'bad_duport' %Error: t/t_var_dup_bad.v:41: Duplicate pin connection: 'bad_mixport' t/t_var_dup_bad.v:41: ... Location of original pin connection %Error: t/t_var_dup_bad.v:42: Can't find definition of variable: 'bad_reout_port' + : ... Suggested alternative: 'bad_duport' %Error: t/t_var_dup_bad.v:43: Can't find definition of variable: 'bad_rewire' %Error: t/t_var_dup_bad.v:43: Can't find definition of variable: 'bad_rereg' + : ... Suggested alternative: 'bad_rewire' %Error: Exiting due to diff --git a/test_regress/t/t_var_notfound_bad.out b/test_regress/t/t_var_notfound_bad.out index d5002774a..0c9a50f97 100644 --- a/test_regress/t/t_var_notfound_bad.out +++ b/test_regress/t/t_var_notfound_bad.out @@ -2,8 +2,11 @@ %Error: t/t_var_notfound_bad.v:18: Can't find definition of 'subsubz' in dotted scope/variable: 'sub.subsubz' ... Known scopes under 'sub': subsub %Error: t/t_var_notfound_bad.v:19: Can't find definition of task/function: 'nofunc' + : ... Suggested alternative: 'notfunc' %Error: t/t_var_notfound_bad.v:20: Can't find definition of 'nofuncs' in dotted task/function: 'sub.nofuncs' + : ... Suggested alternative: 'notfuncs' ... Known scopes under 'nofuncs': %Error: t/t_var_notfound_bad.v:21: Can't find definition of task/function: 'notask' + : ... Suggested alternative: 'nottask' %Error: t/t_var_notfound_bad.v:22: Found definition of 'a_var' as a VAR but expected a task/function %Error: Exiting due to diff --git a/test_regress/t/t_var_suggest_bad.out b/test_regress/t/t_var_suggest_bad.out index 99925b2e0..8dab22b8c 100644 --- a/test_regress/t/t_var_suggest_bad.out +++ b/test_regress/t/t_var_suggest_bad.out @@ -1,3 +1,5 @@ %Error: t/t_var_suggest_bad.v:12: Can't find definition of variable: 'foobat' + : ... Suggested alternative: 'foobar' %Error: t/t_var_suggest_bad.v:13: Can't find definition of task/function: 'boobat' + : ... Suggested alternative: 'boobar' %Error: Exiting due to