Fix some problems and suggestions found by clang-tidy

frontends/common/constantFolding.h

@@ -65,7 +65,6 @@ class DoConstantFolding : public Transform {
     // we substituting constants there.
     bool assignmentTarget;
 
- protected:
     /// @returns a constant equivalent to @p expr or `nullptr`
     const IR::Expression *getConstant(const IR::Expression *expr) const;
 

frontends/p4/coreLibrary.h

@@ -99,19 +99,19 @@ class P4Exception_Model : public ::Model::Elem {
 // To be kept in sync with core.p4
 class P4CoreLibrary : public ::Model::Model {
  protected:
+    // NOLINTBEGIN(bugprone-throw-keyword-missing)
     P4CoreLibrary()
         : noAction("NoAction"),
           exactMatch("exact"),
           ternaryMatch("ternary"),
           lpmMatch("lpm"),
-          packetIn(PacketIn()),
-          packetOut(PacketOut()),
           noError(StandardExceptions::NoError),
           packetTooShort(StandardExceptions::PacketTooShort),
           noMatch(StandardExceptions::NoMatch),
           stackOutOfBounds(StandardExceptions::StackOutOfBounds),
           overwritingHeader(StandardExceptions::OverwritingHeader),
           headerTooShort(StandardExceptions::HeaderTooShort) {}
+    // NOLINTEND(bugprone-throw-keyword-missing)
 
  public:
     static P4CoreLibrary &instance() {

frontends/p4/fromv1.0/converters.h

@@ -311,7 +311,7 @@ class ComputeCallGraph : public Inspector {
         auto name = primitive->name;
         const IR::GlobalRef *glob = nullptr;
         const IR::Declaration_Instance *extrn = nullptr;
-        if (primitive->operands.size() >= 1) glob = primitive->operands[0]->to<IR::GlobalRef>();
+        if (!primitive->operands.empty()) glob = primitive->operands[0]->to<IR::GlobalRef>();
         if (glob) extrn = glob->obj->to<IR::Declaration_Instance>();
 
         if (extrn) {
@@ -798,7 +798,7 @@ class InsertCompilerGeneratedStartState : public Transform {
 
     // rename original start state
     const IR::Node *postorder(IR::ParserState *state) override {
-        if (!structure->parserEntryPoints.size()) return state;
+        if (structure->parserEntryPoints.empty()) return state;
         if (state->name == IR::ParserState::start) {
             state->name = newStartState;
         }
@@ -807,7 +807,7 @@ class InsertCompilerGeneratedStartState : public Transform {
 
     // Rename any path refering to original start state
     const IR::Node *postorder(IR::Path *path) override {
-        if (!structure->parserEntryPoints.size()) return path;
+        if (structure->parserEntryPoints.empty()) return path;
         // At this point any identifier called start should have been renamed
         // to unique name (e.g. start_1) => we can safely assume that any
         // "start" refers to the parser state
@@ -825,7 +825,7 @@ class InsertCompilerGeneratedStartState : public Transform {
     }
 
     const IR::Node *postorder(IR::P4Parser *parser) override {
-        if (!structure->parserEntryPoints.size()) return parser;
+        if (structure->parserEntryPoints.empty()) return parser;
         IR::IndexedVector<IR::SerEnumMember> members;
         // transition to original start state
         members.push_back(new IR::SerEnumMember("START", new IR::Constant(0)));
@@ -1023,9 +1023,7 @@ class FindRecirculated : public Inspector {
     }
 
     void postorder(const IR::Primitive *primitive) override {
-        if (primitive->name == "recirculate") {
-            add(primitive, 0);
-        } else if (primitive->name == "resubmit") {
+        if (primitive->name == "recirculate" || primitive->name == "resubmit") {
             add(primitive, 0);
         } else if (primitive->name.startsWith("clone") && primitive->operands.size() == 2) {
             add(primitive, 1);

frontends/p4/fromv1.0/programStructure.h

@@ -205,7 +205,6 @@ class ProgramStructure {
     const IR::Parameter *parserPacketIn = nullptr;
     const IR::Parameter *parserHeadersOut = nullptr;
 
- public:
     // output is constructed here
     IR::Vector<IR::Node> *declarations;
 

frontends/p4/fromv1.0/v1model.h

@@ -125,8 +125,7 @@ struct CounterOrMeter_Model : public ::Model::Extern_Model {
         : Extern_Model(name),
           sizeParam("size"),
           typeParam("type"),
-          size_type(IR::Type_Bits::get(32)),
-          counterType() {}
+          size_type(IR::Type_Bits::get(32)) {}
     ::Model::Elem sizeParam;
     ::Model::Elem typeParam;
     const IR::Type *size_type;
@@ -218,7 +217,7 @@ struct Cloner_Model : public ::Model::Extern_Model {
     Cloner_Model()
         : Extern_Model("clone"),
           clone3("clone_preserving_field_list"),
-          cloneType(),
+
           sessionType(IR::Type_Bits::get(32)) {}
     ::Model::Elem clone3;
     CloneType_Model cloneType;

ir/dbprint.h

@@ -64,12 +64,13 @@ int dbsetflags(std::ostream &out, int val, int mask = ~0U);
 inline int getprec(std::ostream &out) { return dbgetflags(out) & DBPrint::Precedence; }
 class setflags_helper {
     int val, mask;
-    setflags_helper() = delete;
 
  protected:
     setflags_helper(int v, int m) : val(v), mask(m) { assert((val & ~mask) == 0); }
 
  public:
+    setflags_helper() = delete;
+
     void set(std::ostream &out) const { dbsetflags(out, val, mask); }
 };
 struct setprec : public setflags_helper {

ir/node.h

@@ -158,17 +158,18 @@ inline bool equiv(const Node *a, const Node *b) { return a == b || (a && b && a-
 inline bool equiv(const INode *a, const INode *b) {
     return a == b || (a && b && a->getNode()->equiv(*b->getNode()));
 }
-
+// NOLINTBEGIN(bugprone-macro-parentheses)
 /* common things that ALL Node subclasses must define */
 #define IRNODE_SUBCLASS(T)                             \
  public:                                               \
     T *clone() const override { return new T(*this); } \
     IRNODE_COMMON_SUBCLASS(T)
-
 #define IRNODE_ABSTRACT_SUBCLASS(T) \
  public:                            \
     T *clone() const override = 0;  \
     IRNODE_COMMON_SUBCLASS(T)
+
+// NOLINTEND(bugprone-macro-parentheses)
 #define IRNODE_COMMON_SUBCLASS(T)                                           \
  public:                                                                    \
     using Node::operator==;                                                 \

ir/visitor.h

@@ -78,13 +78,14 @@ class Visitor {
         Visitor &v;
         uint64_t start;
         explicit profile_t(Visitor &);
+        friend class Visitor;
+
+     public:
         profile_t() = delete;
         profile_t(const profile_t &) = delete;
         profile_t &operator=(const profile_t &) = delete;
         profile_t &operator=(profile_t &&) = delete;
-        friend class Visitor;
 
-     public:
         ~profile_t();
         profile_t(profile_t &&);
     };

lib/compile_context.h

@@ -36,6 +36,8 @@ class ICompileContext {
 /// Compilation contexts can be nested to allow composing programs without
 /// intermingling their stack.
 struct CompileContextStack final {
+    CompileContextStack() = delete;
+
     /// @return the current compilation context (i.e., the top of the
     /// compilation context stack), cast to the requested type. If the current
     /// compilation context is of the wrong type, or the stack is empty, an
@@ -64,8 +66,6 @@ struct CompileContextStack final {
     static void push(ICompileContext *context);
     static void pop();
     static StackType &getStack();
-
-    CompileContextStack() = delete;
 };
 
 /// A RAII helper which pushes a compilation context onto the stack when it's

lib/error_catalog.h

@@ -107,7 +107,7 @@ class ErrorCatalog {
     }
 
     /// retrieve the name for errorCode
-    const cstring getName(int errorCode) {
+    cstring getName(int errorCode) {
         if (errorCatalog.count(errorCode)) return errorCatalog.at(errorCode);
         return "--unknown--";
     }

lib/error_message.h

@@ -37,10 +37,10 @@ struct ErrorMessage {
     enum class MessageType : std::size_t { None, Error, Warning, Info };
 
     MessageType type = MessageType::None;
-    std::string prefix = "";                       /// Typically error/warning type from catalog
-    std::string message = "";                      /// Particular formatted message
+    std::string prefix;                            /// Typically error/warning type from catalog
+    std::string message;                           /// Particular formatted message
     std::vector<Util::SourceInfo> locations = {};  /// Relevant source locations for this error
-    std::string suffix = "";                       /// Used by errorWithSuffix
+    std::string suffix;                            /// Used by errorWithSuffix
 
     ErrorMessage() {}
     // Invoked from backwards compatible error_helper

lib/hashvec.cpp

@@ -216,28 +216,32 @@ hash_vector_base::hash_vector_base(hash_vector_base &&a)
 }
 
 hash_vector_base &hash_vector_base::operator=(const hash_vector_base &a) {
-    freehash();
-    info = a.info;
-    hashsize = a.hashsize;
-    inuse = a.inuse;
-    collisions = a.collisions;
-    log_hashsize = a.log_hashsize;
-    erased = a.erased;
-    allochash();
-    memcpy(hash.s1, a.hash.s1, hashsize * info->hashelsize);
+    if (this != &a) {
+        freehash();
+        info = a.info;
+        hashsize = a.hashsize;
+        inuse = a.inuse;
+        collisions = a.collisions;
+        log_hashsize = a.log_hashsize;
+        erased = a.erased;
+        allochash();
+        memcpy(hash.s1, a.hash.s1, hashsize * info->hashelsize);
+    }
     return *this;
 }
 
 hash_vector_base &hash_vector_base::operator=(hash_vector_base &&a) {
-    freehash();
-    info = a.info;
-    hashsize = a.hashsize;
-    inuse = a.inuse;
-    collisions = a.collisions;
-    log_hashsize = a.log_hashsize;
-    erased = a.erased;
-    hash.s1 = a.hash.s1;
-    a.hash.s1 = nullptr;
+    if (this != &a) {
+        freehash();
+        info = a.info;
+        hashsize = a.hashsize;
+        inuse = a.inuse;
+        collisions = a.collisions;
+        log_hashsize = a.log_hashsize;
+        erased = a.erased;
+        hash.s1 = a.hash.s1;
+        a.hash.s1 = nullptr;
+    }
     return *this;
 }
 

lib/indent.h

@@ -88,10 +88,10 @@ inline std::ostream &unindent(std::ostream &out) {
 
 class TempIndent {
     // an indent that can be added to any stream and unrolls when the object is destroyed
-    std::vector<std::ostream *> streams;      // streams that have been indented
-    TempIndent(const TempIndent &) = delete;  // not copyable
+    std::vector<std::ostream *> streams;  // streams that have been indented
 
  public:
+    TempIndent(const TempIndent &) = delete;  // not copyable
     TempIndent() = default;
     friend std::ostream &operator<<(std::ostream &out, TempIndent &ti) {
         ti.streams.push_back(&out);

lib/log.h

@@ -105,6 +105,7 @@ void increaseVerbosity();
 #define MAX_LOGGING_LEVEL 10
 #endif
 
+// NOLINTBEGIN(bugprone-macro-parentheses)
 #define LOGGING(N)                                                            \
     ((N) <= MAX_LOGGING_LEVEL && ::Log::fileLogLevelIsAtLeast(__FILE__, N) && \
      ::Log::enableLogging())
@@ -144,6 +145,7 @@ void increaseVerbosity();
 #define P4C_ERROR(X) (std::clog << "ERROR: " << X << std::endl)
 #define P4C_WARNING(X) (::Log::verbose() ? std::clog << "WARNING: " << X << std::endl : std::clog)
 #define ERRWARN(C, X) ((C) ? P4C_ERROR(X) : P4C_WARNING(X))
+// NOLINTEND(bugprone-macro-parentheses)
 
 static inline std::ostream &operator<<(std::ostream &out,
                                        std::function<std::ostream &(std::ostream &)> fn) {

lib/ordered_map.h

@@ -86,7 +86,6 @@ class ordered_map {
  public:
     typedef typename map_type::size_type size_type;
 
- public:
     ordered_map() {}
     ordered_map(const ordered_map &a) : data(a.data) { init_data_map(); }
     template <typename InputIt>

lib/safe_vector.h

@@ -21,13 +21,13 @@ limitations under the License.
 
 /// An enhanced version of std::vector that performs bounds checking for
 /// operator[].
-template <class T, class _Alloc = std::allocator<T>>
-class safe_vector : public std::vector<T, _Alloc> {
+template <class T, class Alloc = std::allocator<T>>
+class safe_vector : public std::vector<T, Alloc> {
  public:
-    using std::vector<T, _Alloc>::vector;
-    typedef typename std::vector<T, _Alloc>::reference reference;
-    typedef typename std::vector<T, _Alloc>::const_reference const_reference;
-    typedef typename std::vector<T, _Alloc>::size_type size_type;
+    using std::vector<T, Alloc>::vector;
+    typedef typename std::vector<T, Alloc>::reference reference;
+    typedef typename std::vector<T, Alloc>::const_reference const_reference;
+    typedef typename std::vector<T, Alloc>::size_type size_type;
     typedef typename std::vector<T>::const_iterator const_iterator;
     reference operator[](size_type n) { return this->at(n); }
     const_reference operator[](size_type n) const { return this->at(n); }

lib/sourceCodeBuilder.h

@@ -57,7 +57,7 @@ class SourceCodeBuilder {
         newline();
     }
     void append(const std::string &str) {
-        if (str.size() == 0) return;
+        if (str.empty()) return;
         endsInSpace = ::isspace(str.at(str.size() - 1));
         buffer << str;
     }

lib/source_file.h

@@ -138,7 +138,7 @@ class SourceInfo final {
         this->srcBrief = srcBrief;
     }
     /// Creates an "invalid" SourceInfo
-    SourceInfo() : sources(nullptr), start(SourcePosition()), end(SourcePosition()) {}
+    SourceInfo() : sources(nullptr) {}
 
     /// Creates a SourceInfo for a 'point' in the source, or invalid
     SourceInfo(const InputSources *sources, SourcePosition point)
@@ -153,7 +153,7 @@ class SourceInfo final {
     /**
         A SourceInfo that spans both this and rhs.
         However, if this or rhs is invalid, it is not taken into account */
-    const SourceInfo operator+(const SourceInfo &rhs) const {
+    SourceInfo operator+(const SourceInfo &rhs) const {
         if (!this->isValid()) return rhs;
         if (!rhs.isValid()) return *this;
         SourcePosition s = start.min(rhs.start);

lib/stringref.h

@@ -51,11 +51,14 @@ struct StringRef {
         len = 0;
     }
     StringRef(const StringRef &a) : p(a.p), len(a.len) {}
+    // avoid clang-tidy complaining about assignment that is actually safe
+    // NOLINTBEGIN(bugprone-unhandled-self-assignment)
     StringRef &operator=(const StringRef &a) {
         p = a.p;
         len = a.len;
         return *this;
     }
+    // NOLINTEND(bugprone-unhandled-self-assignment)
     explicit operator bool() const { return p != 0; }
 
     bool operator==(const StringRef &a) const {