More code improvements based on static analysis

backends/dpdk/dpdkArch.cpp

@@ -2640,7 +2640,7 @@ void CollectAddOnMissTable::postorder(const IR::P4Table *t) {
         ::error(ErrorType::ERR_UNEXPECTED,
                 "%1%: add_on_miss property is defined, "
                 "but default_action not specificed for table %2%",
-                default_action, t->name);
+                add_on_miss, t->name);
         return;
     }
     if (default_action->value->is<IR::ExpressionValue>()) {

frontends/common/parser_options.h

@@ -176,6 +176,11 @@ class P4CContextWithOptions final : public P4CContext {
         optionsInstance = context.options();
     }
 
+    template <typename OptionsDerivedType>
+    P4CContextWithOptions &operator=(P4CContextWithOptions<OptionsDerivedType> &context) {
+        optionsInstance = context.options();
+    }
+
     /// @return the compiler options for this compilation context.
     OptionsType &options() override { return optionsInstance; }
 

frontends/p4/fromv1.0/converters.h

@@ -327,8 +327,10 @@ class ComputeCallGraph : public Inspector {
                 ctr = gr->obj->to<IR::Counter>();
             else if (auto nr = ctrref->to<IR::PathExpression>())
                 ctr = structure->counters.get(nr->path->name);
-            if (ctr == nullptr)
+            if (ctr == nullptr) {
                 ::error(ErrorType::ERR_NOT_FOUND, "%1%: Cannot find counter", ctrref);
+                return;
+            }
             auto parent = findContext<IR::ActionFunction>();
             BUG_CHECK(parent != nullptr, "%1%: Counter call not within action", primitive);
             structure->calledCounters.calls(parent->name, ctr->name.name);
@@ -340,7 +342,10 @@ class ComputeCallGraph : public Inspector {
                 mtr = gr->obj->to<IR::Meter>();
             else if (auto nr = mtrref->to<IR::PathExpression>())
                 mtr = structure->meters.get(nr->path->name);
-            if (mtr == nullptr) ::error(ErrorType::ERR_NOT_FOUND, "%1%: Cannot find meter", mtrref);
+            if (mtr == nullptr) {
+                ::error(ErrorType::ERR_NOT_FOUND, "%1%: Cannot find meter", mtrref);
+                return;
+            }
             auto parent = findContext<IR::ActionFunction>();
             BUG_CHECK(parent != nullptr, "%1%: not within action", primitive);
             structure->calledMeters.calls(parent->name, mtr->name.name);
@@ -356,8 +361,10 @@ class ComputeCallGraph : public Inspector {
                 reg = gr->obj->to<IR::Register>();
             else if (auto nr = regref->to<IR::PathExpression>())
                 reg = structure->registers.get(nr->path->name);
-            if (reg == nullptr)
+            if (reg == nullptr) {
                 ::error(ErrorType::ERR_NOT_FOUND, "%1%: Cannot find register", regref);
+                return;
+            }
             auto parent = findContext<IR::ActionFunction>();
             BUG_CHECK(parent != nullptr, "%1%: not within action", primitive);
             structure->calledRegisters.calls(parent->name, reg->name.name);
@@ -407,11 +414,15 @@ class ComputeTableCallGraph : public Inspector {
     void postorder(const IR::Apply *apply) override {
         LOG3("Scanning " << apply->name);
         auto tbl = structure->tables.get(apply->name.name);
-        if (tbl == nullptr)
+        if (tbl == nullptr) {
             ::error(ErrorType::ERR_NOT_FOUND, "%1%: Could not find table", apply->name);
+            return;
+        }
         auto parent = findContext<IR::V1Control>();
-        if (!parent)
+        if (!parent) {
             ::error(ErrorType::ERR_UNEXPECTED, "%1%: Apply not within a control block?", apply);
+            return;
+        }
 
         auto ctrl = get(structure->tableMapping, tbl);
 

frontends/p4/frontend.h

@@ -31,13 +31,14 @@ class FrontEnd {
 
  public:
     FrontEnd() = default;
-    explicit FrontEnd(ParseAnnotations parseAnnotations) : parseAnnotations(parseAnnotations) {}
-    explicit FrontEnd(DebugHook hook) { hooks.push_back(hook); }
-    explicit FrontEnd(ParseAnnotations parseAnnotations, DebugHook hook)
+    explicit FrontEnd(const ParseAnnotations &parseAnnotations)
+        : parseAnnotations(parseAnnotations) {}
+    explicit FrontEnd(const DebugHook &hook) { hooks.push_back(hook); }
+    explicit FrontEnd(const ParseAnnotations &parseAnnotations, const DebugHook &hook)
         : FrontEnd(parseAnnotations) {
         hooks.push_back(hook);
     }
-    void addDebugHook(DebugHook hook) { hooks.push_back(hook); }
+    void addDebugHook(const DebugHook &hook) { hooks.push_back(hook); }
     // If p4c is run with option '--listFrontendPasses', outStream is used for printing passes names
     const IR::P4Program *run(const CompilerOptions &options, const IR::P4Program *program,
                              bool skipSideEffectOrdering = false,

frontends/p4/typeChecking/typeSubstitution.h

@@ -33,7 +33,8 @@ class TypeSubstitution : public IHasDbPrint {
 
  public:
     TypeSubstitution() = default;
-    TypeSubstitution(const TypeSubstitution &other) : binding(other.binding) {}
+    TypeSubstitution(const TypeSubstitution &other) = default;
+    TypeSubstitution &operator=(const TypeSubstitution &other) = default;
 
     /** True if this is the empty substitution, which does not replace anything. */
     bool isIdentity() const { return binding.size() == 0; }

ir/dbprint-expression.cpp

@@ -143,8 +143,8 @@ void IR::Apply::dbprint(std::ostream &out) const {
     int prec = getprec(out);
     if (!actions.empty()) {
         out << " {" << indent << setprec(0);
-        for (auto act : actions)
-            out << Log::endl << act.first << " {" << indent << act.second << unindent << " }";
+        for (const auto &[actName, actExprs] : actions)
+            out << Log::endl << actName << " {" << indent << actExprs << unindent << " }";
         out << setprec(prec) << " }" << unindent;
     } else if (prec == 0) {
         out << ';';

ir/expression.cpp

@@ -60,7 +60,7 @@ int IR::Member::offset_bits() const {
 
 int IR::Member::lsb() const {
     int rv = 0;
-    auto header_type = dynamic_cast<const IR::Type_StructLike *>(expr->type);
+    auto header_type = expr->type->checkedTo<IR::Type_StructLike>();
     auto field_iter = header_type->fields.rbegin();
     // This assumes little-endian number for bits.
     while ((*field_iter)->name != member) {

ir/ir.cpp

@@ -120,8 +120,7 @@ bool IFunctional::callMatches(const Vector<Argument> *arguments) const {
     }
     // Check if all remaining parameters have default values
     // or are optional.
-    for (auto it : paramNames) {
-        auto param = it.second;
+    for (const auto &[_, param] : paramNames) {
         if (!param->isOptional() && !param->defaultValue) return false;
     }
     return true;
@@ -158,8 +157,11 @@ size_t Type_Stack::getSize() const {
         ::error(ErrorType::ERR_OVERLIMIT, "Index too large: %1%", cst);
         return 0;
     }
-    int size = cst->asInt();
-    if (size < 0) ::error(ErrorType::ERR_OVERLIMIT, "Illegal array size: %1%", cst);
+    auto size = cst->asInt();
+    if (size < 0) {
+        ::error(ErrorType::ERR_OVERLIMIT, "Illegal array size: %1%", cst);
+        return 0;
+    }
     return static_cast<size_t>(size);
 }
 

lib/compile_context.cpp

@@ -56,11 +56,6 @@ AutoCompileContext::AutoCompileContext(ICompileContext *context) {
 
 AutoCompileContext::~AutoCompileContext() { CompileContextStack::pop(); }
 
-BaseCompileContext::BaseCompileContext() {}
-
-BaseCompileContext::BaseCompileContext(const BaseCompileContext &other)
-    : errorReporterInstance(other.errorReporterInstance) {}
-
 /* static */ BaseCompileContext &BaseCompileContext::get() {
     return CompileContextStack::top<BaseCompileContext>();
 }

lib/compile_context.h

@@ -82,8 +82,9 @@ struct AutoCompileContext {
 /// BaseCompileContext.
 class BaseCompileContext : public ICompileContext {
  protected:
-    BaseCompileContext();
-    BaseCompileContext(const BaseCompileContext &other);
+    BaseCompileContext() = default;
+    BaseCompileContext(const BaseCompileContext &other) = default;
+    BaseCompileContext &operator=(const BaseCompileContext &other) = default;
 
  public:
     /// @return the current compilation context, which must inherit from

lib/source_file.h

@@ -57,13 +57,12 @@ position are the "smallest", which is a reasonable choice.
 class SourcePosition final {
  public:
     /// Creates an invalid source position
-    SourcePosition() : lineNumber(0), columnNumber(0) {}
+    SourcePosition() = default;
 
     SourcePosition(unsigned lineNumber, unsigned columnNumber);
-    SourcePosition &operator=(const SourcePosition &) = default;
 
-    SourcePosition(const SourcePosition &other)
-        : lineNumber(other.lineNumber), columnNumber(other.columnNumber) {}
+    SourcePosition(const SourcePosition &other) = default;
+    SourcePosition &operator=(const SourcePosition &) = default;
 
     inline bool operator==(const SourcePosition &rhs) const {
         return columnNumber == rhs.columnNumber && lineNumber == rhs.lineNumber;
@@ -109,8 +108,8 @@ class SourcePosition final {
 
  private:
     // Input sources where this character position is interpreted.
-    unsigned lineNumber;
-    unsigned columnNumber;
+    unsigned lineNumber = 0;
+    unsigned columnNumber = 0;
 };
 
 class InputSources;
@@ -138,7 +137,7 @@ class SourceInfo final {
         this->srcBrief = srcBrief;
     }
     /// Creates an "invalid" SourceInfo
-    SourceInfo() : sources(nullptr) {}
+    SourceInfo() = default;
 
     /// Creates a SourceInfo for a 'point' in the source, or invalid
     SourceInfo(const InputSources *sources, SourcePosition point)
@@ -147,8 +146,8 @@ class SourceInfo final {
     SourceInfo(const InputSources *sources, SourcePosition start, SourcePosition end);
 
     SourceInfo(const SourceInfo &other) = default;
-    ~SourceInfo() = default;
     SourceInfo &operator=(const SourceInfo &other) = default;
+    ~SourceInfo() = default;
 
     /**
         A SourceInfo that spans both this and rhs.

lib/stringify.cpp

@@ -145,6 +145,7 @@ cstring vprintf_format(const char *fmt_str, va_list ap) {
     if (static_cast<size_t>(size) >= sizeof(buf)) {
         char *formatted = new char[size + 1];
         vsnprintf(formatted, size + 1, fmt_str, ap_copy);
+        va_end(ap_copy);
         return cstring::own(formatted, size);
     }
     va_end(ap_copy);

test/gtest/helpers.cpp

@@ -154,7 +154,7 @@ namespace Test {
     const std::string &source,
     CompilerOptions::FrontendVersion langVersion
     /* = CompilerOptions::FrontendVersion::P4_16 */,
-    P4::ParseAnnotations parseAnnotations
+    const P4::ParseAnnotations &parseAnnotations
     /* = P4::ParseAnnotations() */) {
     auto *program = P4::parseP4String(source, langVersion);
     if (program == nullptr) {

test/gtest/helpers.h

@@ -116,10 +116,10 @@ struct FrontendTestCase {
     /// Create a test case that only requires the frontend to run.
     static std::optional<FrontendTestCase> create(
         const std::string &source, CompilerOptions::FrontendVersion langVersion = defaultVersion,
-        P4::ParseAnnotations parseAnnotations = P4::ParseAnnotations());
+        const P4::ParseAnnotations &parseAnnotations = P4::ParseAnnotations());
 
     static std::optional<FrontendTestCase> create(const std::string &source,
-                                                  P4::ParseAnnotations parseAnnotations) {
+                                                  const P4::ParseAnnotations &parseAnnotations) {
         return create(source, defaultVersion, parseAnnotations);
     }