Merge branch 'master' into netlist

* master:
  fix addOne/subOne for SVInt (MikePopoloski#729)
  Fix off-by-one error in check for max object size
  Update CHANGELOG
  Make error about implicit named port connection type mismatches be convertible to a warning for VCS compat
  Fix gcc build
  Add a command line option to suppress warnings for files within a given path

CHANGELOG.md

@@ -20,6 +20,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### General Features
 * New option `--obfuscate-ids` when used with `--preprocess` will replace all identifiers in the output with obfuscated alphanumeric strings (thanks to @Sustrak)
 * Added a group of new warnings to report on unused code elements; see [-Wunused](https://sv-lang.com/warning-ref.html#unused)
+* A new (currently experimental) tool, `slang-tidy`, has been added as a place to collect more in-depth or project-specific static analysis rules (thanks to @Sustrak).
+* New option `--suppress-warnings` allows suppressing warnings from one or more file paths, intended to allow easily hiding warnings from 3rd party code
 
 ### Improvements
 * The default library name for slang has been changed to "libsvlang" to avoid clashing with an existing "S-lang" package on most Linux systems. The library name is now configureable via CMake.
@@ -29,7 +31,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 * Added human-friendly names for function arguments in Python bindings (thanks to @Kuree)
 * Conditional statements in constant functions now implement unique/priority semantics (thanks to @HungMingWu)
 * Procedural force/release of bit selects or part selects is an error according to the LRM; this error can now be downgraded to a warning for compatibility with other tools (thanks to @udif)
+* Implicit named port connections with inequivalent types are disallowed by the LRM; this error can now be downgraded to a warning for compatibility with other tools
 * When printing type names in diagnostics, if more than one type shares the same simple name, they will now be disambiguated with their full path
+* A new `--timescale` option allows setting a default time scale for design elements. If this option is not set, there is no default and an error will be
+issued if not all elements have a time scale specified (as required by the LRM).
 
 ### Fixes
 * Parameters used inside specify blocks will now issue an appropriate warning
@@ -46,6 +51,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 * Hierarchical references from packages to items outside that package are now correctly disallowed
 * Bit-vector system functions (such as `$countbits`) can now be used with bitstream types (as opposed to just integral types)
 * Max size limits for packed and unpacked arrays and structs are now strictly enforced
+* Invalid inferred time scales will now issue an appropriate error
 
 
 ## [v2.0] - 2022-10-29

bindings/python/util.cpp

@@ -107,6 +107,7 @@ void registerUtil(py::module_& m) {
         .def("getLineNumber", &SourceManager::getLineNumber, "location"_a)
         .def("getFileName", &SourceManager::getFileName, "location"_a)
         .def("getRawFileName", &SourceManager::getRawFileName, "buffer"_a)
+        .def("getFullPath", &SourceManager::getFullPath, "buffer"_a)
         .def("getColumnNumber", &SourceManager::getColumnNumber, "location"_a)
         .def("getIncludedFrom", &SourceManager::getIncludedFrom, "buffer"_a)
         .def("getMacroName", &SourceManager::getMacroName, "location"_a)

docs/command-line-ref.dox

@@ -307,6 +307,11 @@ Show or hide macro expansion backtraces in diagnostic output. The default is to
 
 Show or hide hierarchy locations in diagnostic output. The default is to show.
 
+`--suppress-warnings`
+
+One or more paths in which to suppress warnings. Use this if you want to generally turn on warnings
+for your project and have it build cleanly but have some files which you can't modify for some reason.
+
 `--error-limit`
 
 Set a limit on the number of errors that will be printed. Setting this to zero will

include/slang/diagnostics/DiagnosticEngine.h

@@ -7,6 +7,7 @@
 //------------------------------------------------------------------------------
 #pragma once
 
+#include <filesystem>
 #include <memory>
 #include <string>
 #include <typeindex>
@@ -142,6 +143,10 @@ class SLANG_EXPORT DiagnosticEngine {
     /// severity type.
     void clearMappings(DiagnosticSeverity severity);
 
+    /// Adds a path prefix for which all warnings will be supressed. This applies to
+    /// all natural warnings, regardless of whether they've been upgraded to an error.
+    void addIgnorePath(const std::filesystem::path& path);
+
     /// Sets a custom formatter function for the given type. This is used to
     /// provide formatting for diagnostic arguments of a custom type.
     template<typename ForType>
@@ -207,7 +212,7 @@ class SLANG_EXPORT DiagnosticEngine {
 
     std::optional<DiagnosticSeverity> findMappedSeverity(DiagCode code,
                                                          SourceLocation location) const;
-    void issueImpl(const Diagnostic& diagnostic, DiagnosticSeverity severity);
+    bool issueImpl(const Diagnostic& diagnostic, DiagnosticSeverity severity);
 
     template<typename TDirective>
     void setMappingsFromPragmasImpl(BufferID buffer, span<const TDirective> directives,
@@ -216,6 +221,22 @@ class SLANG_EXPORT DiagnosticEngine {
     // The source manager used to resolve locations into file names.
     const SourceManager& sourceManager;
 
+    // Tracking for the number of errors and warnings we've issued.
+    int numErrors = 0;
+    int numWarnings = 0;
+
+    // Configurable controls for when and how we issue diagnostics.
+    int errorLimit = 0;
+    bool ignoreAllWarnings = false;
+    bool ignoreAllNotes = false;
+    bool warningsAsErrors = false;
+    bool errorsAsFatal = false;
+    bool fatalsAsErrors = false;
+
+    // Tracking for whether we've already issued an error for going over
+    // the configured error limit (we only want to do that once).
+    bool issuedOverLimitErr = false;
+
     // A global mapping from diagnostic to a configured severity it should have.
     flat_hash_map<DiagCode, DiagnosticSeverity> severityTable;
 
@@ -230,6 +251,9 @@ class SLANG_EXPORT DiagnosticEngine {
     // These correspond to `pragma diagnostic entries in the source code.
     flat_hash_map<DiagCode, flat_hash_map<BufferID, std::vector<DiagnosticMapping>>> diagMappings;
 
+    // A list of path prefixes to use to suppress warnings.
+    std::vector<std::filesystem::path> ignoreWarnPrefixes;
+
     // A list of all registered clients that receive issued diagnostics.
     std::vector<std::shared_ptr<DiagnosticClient>> clients;
 
@@ -241,22 +265,6 @@ class SLANG_EXPORT DiagnosticEngine {
     // A set of default formatters that will be assigned to each new DiagnosticEngine instance
     // that gets created. These can later be overridden on a per-instance basis.
     static FormatterMap defaultFormatters;
-
-    // Tracking for the number of errors and warnings we've issued.
-    int numErrors = 0;
-    int numWarnings = 0;
-
-    // Configurable controls for when and how we issue diagnostics.
-    int errorLimit = 0;
-    bool ignoreAllWarnings = false;
-    bool ignoreAllNotes = false;
-    bool warningsAsErrors = false;
-    bool errorsAsFatal = false;
-    bool fatalsAsErrors = false;
-
-    // Tracking for whether we've already issued an error for going over
-    // the configured error limit (we only want to do that once).
-    bool issuedOverLimitErr = false;
 };
 
 } // namespace slang

include/slang/driver/Driver.h

@@ -208,6 +208,9 @@ class SLANG_EXPORT Driver {
         /// A list of warning options that will be passed to the DiagnosticEngine.
         std::vector<std::string> warningOptions;
 
+        /// A list of paths in which to suppress warnings.
+        std::vector<std::string> suppressWarningsPaths;
+
         /// @}
         /// @name File lists
         /// @{

include/slang/text/SourceManager.h

@@ -20,8 +20,6 @@
 #include "slang/util/Hash.h"
 #include "slang/util/Util.h"
 
-namespace fs = std::filesystem;
-
 namespace slang {
 
 enum class DiagnosticSeverity;
@@ -58,6 +56,11 @@ class SLANG_EXPORT SourceManager {
     /// into account any `line directives that may be in the file.
     string_view getRawFileName(BufferID buffer) const;
 
+    /// Gets the full path to the given source buffer. This does not take
+    /// into account any `line directives. If the buffer is not a file buffer,
+    /// returns an empty string.
+    const std::filesystem::path& getFullPath(BufferID buffer) const;
+
     /// Gets the column line number for a given source location.
     /// @a location must be a file location.
     size_t getColumnNumber(SourceLocation location) const;
@@ -132,13 +135,13 @@ class SLANG_EXPORT SourceManager {
                               SourceLocation includedFrom = SourceLocation());
 
     /// Read in a source file from disk.
-    SourceBuffer readSource(const fs::path& path);
+    SourceBuffer readSource(const std::filesystem::path& path);
 
     /// Read in a header file from disk.
     SourceBuffer readHeader(string_view path, SourceLocation includedFrom, bool isSystemPath);
 
     /// Returns true if the given file path is already loaded and cached in the source manager.
-    bool isCached(const fs::path& path) const;
+    bool isCached(const std::filesystem::path& path) const;
 
     /// Sets whether filenames should be made "proximate" to the current directory
     /// for diagnostic reporting purposes. This is on by default but can be
@@ -203,13 +206,16 @@ class SLANG_EXPORT SourceManager {
 
     // Stores actual file contents and metadata; only one per loaded file
     struct FileData {
-        const std::string name;          // name of the file
-        const std::vector<char> mem;     // file contents
-        std::vector<size_t> lineOffsets; // cache of compute line offsets
-        const fs::path* const directory; // directory in which the file exists
-
-        FileData(const fs::path* directory, std::string name, std::vector<char>&& data) :
-            name(std::move(name)), mem(std::move(data)), directory(directory) {}
+        const std::string name;                       // name of the file
+        const std::vector<char> mem;                  // file contents
+        std::vector<size_t> lineOffsets;              // cache of compute line offsets
+        const std::filesystem::path* const directory; // directory in which the file exists
+        const std::filesystem::path fullPath;         // full path to the file
+
+        FileData(const std::filesystem::path* directory, std::string name, std::vector<char>&& data,
+                 std::filesystem::path fullPath) :
+            name(std::move(name)),
+            mem(std::move(data)), directory(directory), fullPath(std::move(fullPath)) {}
     };
 
     // Stores a pointer to file data along with information about where we included it.
@@ -261,11 +267,11 @@ class SLANG_EXPORT SourceManager {
     flat_hash_map<std::string, std::unique_ptr<FileData>> lookupCache;
 
     // directories for system and user includes
-    std::vector<fs::path> systemDirectories;
-    std::vector<fs::path> userDirectories;
+    std::vector<std::filesystem::path> systemDirectories;
+    std::vector<std::filesystem::path> userDirectories;
 
     // uniquified backing memory for directories
-    std::set<fs::path> directories;
+    std::set<std::filesystem::path> directories;
 
     // map from buffer to diagnostic directive lists
     flat_hash_map<BufferID, std::vector<DiagnosticDirectiveInfo>> diagDirectives;
@@ -278,9 +284,9 @@ class SLANG_EXPORT SourceManager {
     SourceBuffer createBufferEntry(FileData* fd, SourceLocation includedFrom,
                                    std::unique_lock<std::shared_mutex>& lock);
 
-    SourceBuffer openCached(const fs::path& fullPath, SourceLocation includedFrom);
-    SourceBuffer cacheBuffer(const fs::path& path, SourceLocation includedFrom,
-                             std::vector<char>&& buffer);
+    SourceBuffer openCached(const std::filesystem::path& fullPath, SourceLocation includedFrom);
+    SourceBuffer cacheBuffer(std::filesystem::path&& path, std::string&& pathStr,
+                             SourceLocation includedFrom, std::vector<char>&& buffer);
 
     // Get raw line number of a file location, ignoring any line directives
     size_t getRawLineNumber(SourceLocation location) const;

scripts/diagnostics.txt

@@ -320,7 +320,6 @@ error TimeScaleFirstInScope "time scale declaration must come before all other i
 error GenvarDuplicate "loop generation value {} repeats more than once"
 error GenvarUnknownBits "value of {} is invalid for loop generation value"
 error ImplicitNamedPortNotFound "could not find connection for implicit named port '{}'"
-error ImplicitNamedPortTypeMismatch "implicit named port '{}' of type {} connects to value of inequivalent type {}"
 error MaxGenerateStepsExceeded "generate loop hit maximum step limit; possible infinite loop?"
 error MixingSubroutinePortKinds "port declarations not allowed when subroutine defines a formal argument list"
 error UnpackedArrayParamType "unpacked array parameter requires explicit data type"
@@ -475,6 +474,7 @@ warning net-inconsistent NetInconsistent "net type '{}' of connection is inconsi
 warning net-inconsistent NetRangeInconsistent "net type '{}' of connection bits [{}:{}] is inconsistent with net type '{}' of corresponding port range"
 warning dup-timing-path DupTimingPath "duplicate timing path"
 warning invalid-pulsestyle InvalidPulseStyle "invalid use of {} for '{}' since it has previously appeared in a path declaration"
+warning implicit-port-type-mismatch ImplicitNamedPortTypeMismatch "implicit named port '{}' of type {} connects to value of inequivalent type {}"
 
 subsystem Expressions
 error BadUnaryExpression "invalid operand type {} to unary expression"
@@ -989,7 +989,7 @@ group default = { real-underflow real-overflow vector-overflow int-overflow unco
                   protect-arglist unknown-protect-encoding unknown-protect-option invalid-pragma-number
                   invalid-pragma-viewport duplicate-definition protect-encoding-bytes invalid-encoding-byte
                   raw-protect-eof protected-envelope specify-param dup-timing-path invalid-pulsestyle
-                  negative-timing-limit bad-procedural-force duplicate-defparam }
+                  negative-timing-limit bad-procedural-force duplicate-defparam implicit-port-type-mismatch }
 
 group extra = { empty-member empty-stmt dup-import pointless-void-cast case-gen-none case-gen-dup
                 unused-result format-real ignored-slice task-ignored width-trunc dup-attr event-const

scripts/warning_docs.txt

@@ -994,3 +994,18 @@ module top;
     defparam m1.p = 2;
 endmodule
 ```
+
+-Wimplicit-port-type-mismatch
+An implicit named port connection is made between two inequivalent types. Unlike with a normal
+port connection, where the type of the connection undergoes implicit conversion, the LRM specifies
+that this case is an error. slang makes this an error by default but it can be turned into a warning,
+to support code compatibility with commercial tools.
+```
+module m(logic p);
+endmodule
+
+module n;
+    int p;
+    m m1(.p);
+endmodule
+```

source/ast/symbols/PortSymbols.cpp

@@ -1738,25 +1738,29 @@ const Expression* PortConnection::getExpression() const {
 
             if (!e->type->isEquivalent(*type)) {
                 auto& comp = context.getCompilation();
-                if (!parentInstance.arrayPath.empty() && direction != ArgumentDirection::Ref) {
-                    if (direction == ArgumentDirection::In) {
-                        e = &Expression::convertAssignment(context, *type, *e,
-                                                           implicitNameRange.start());
-                    }
-                    else {
-                        auto rhs = comp.emplace<EmptyArgumentExpression>(*type, implicitNameRange);
-                        Expression::convertAssignment(context, *e->type, *rhs,
-                                                      implicitNameRange.start(), &e, &assignFlags);
-                    }
+                if (direction == ArgumentDirection::In) {
+                    e = &Expression::convertAssignment(context, *type, *e,
+                                                       implicitNameRange.start());
                 }
-                else if (!e->bad() && !type->isError()) {
+                else if (direction != ArgumentDirection::Ref) {
+                    auto rhs = comp.emplace<EmptyArgumentExpression>(*type, implicitNameRange);
+                    Expression::convertAssignment(context, *e->type, *rhs,
+                                                  implicitNameRange.start(), &e, &assignFlags);
+                }
+
+                // We should warn for this case unless convertAssignment already issued an error,
+                // or if we're in an instance array unwrapping case.
+                if ((parentInstance.arrayPath.empty() || direction == ArgumentDirection::Ref) &&
+                    !e->bad() && !type->isError()) {
                     auto& diag = context.addDiag(diag::ImplicitNamedPortTypeMismatch,
                                                  implicitNameRange);
                     diag << port.name;
                     diag << *type;
                     diag << *e->type;
 
-                    e = comp.emplace<InvalidExpression>(e, comp.getErrorType());
+                    // There's no way to represent this expression for the ref case.
+                    if (direction == ArgumentDirection::Ref)
+                        e = comp.emplace<InvalidExpression>(e, comp.getErrorType());
                 }
             }
 

source/ast/types/AllTypes.cpp

@@ -691,9 +691,10 @@ const Type& FixedSizeUnpackedArrayType::fromDim(const Scope& scope, const Type&
 
     auto& comp = scope.getCompilation();
     auto width = checkedMulU32(elementType.getSelectableWidth(), dim.width());
-    if (!width || width > uint32_t(INT32_MAX)) {
+    const uint32_t maxWidth = uint32_t(INT32_MAX) + 1;
+    if (!width || width > maxWidth) {
         uint64_t fullWidth = uint64_t(elementType.getSelectableWidth()) * dim.width();
-        scope.addDiag(diag::ObjectTooLarge, sourceRange.get()) << fullWidth << INT32_MAX;
+        scope.addDiag(diag::ObjectTooLarge, sourceRange.get()) << fullWidth << maxWidth;
         return comp.getErrorType();
     }
 
@@ -886,9 +887,10 @@ const Type& UnpackedStructType::fromSyntax(const ASTContext& context,
             fields.push_back(field);
 
             bitOffset += field->getType().getSelectableWidth();
-            if (bitOffset > uint32_t(INT32_MAX)) {
+            const uint32_t maxWidth = uint32_t(INT32_MAX) + 1;
+            if (bitOffset > maxWidth) {
                 context.addDiag(diag::ObjectTooLarge, syntax.sourceRange())
-                    << bitOffset << INT32_MAX;
+                    << bitOffset << maxWidth;
                 return comp.getErrorType();
             }
         }