Build with -pedantic, fix problems (#3892)

* Add pedantic flag to build * Fix string literals in exceptions.h * Remove extraneous semicolons * Remove designated initializers (They are not standard C++ until C++20.) * Ignore implicit-fallthrough if generated lexers * Don't warn about ##__VA_ARGS__ when using clang
p4lang · Feb 16, 2023 · 19ed58d · 19ed58d
1 parent 341079a
commit 19ed58d
Show file tree

Hide file tree

Showing 12 changed files with 28 additions and 21 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -212,6 +212,16 @@ add_cxx_compiler_option ("-Wextra")
 add_cxx_compiler_option ("-Wno-overloaded-virtual")
 add_cxx_compiler_option ("-Wno-deprecated")
 add_cxx_compiler_option ("-Wno-deprecated-declarations")
+# Make compiler follow the standard and not be too lenient. The increases
+# likelyhood that compilation will not break with other compilers (mainly clang)
+# or in future versions of GCC.
+add_cxx_compiler_option ("-pedantic")
+
+if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
+  # The ##__VA_ARGS__ GNU extension is needed for IR. But clang compains about it.
+  # FIXME: with C++20 we would be able to use standard __VA_OPT__
+  add_cxx_compiler_option ("-Wno-gnu-zero-variadic-macro-arguments")
+endif()
 
 if (ENABLE_SANITIZERS)
   append("-fsanitize=undefined,address" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)

diff --git a/backends/bmv2/common/options.h b/backends/bmv2/common/options.h
@@ -62,6 +62,6 @@ class BMV2Options : public CompilerOptions {
 
 using BMV2Context = P4CContextWithOptions<BMV2Options>;
 
-};  // namespace BMV2
+}  // namespace BMV2
 
 #endif /* BACKENDS_BMV2_COMMON_OPTIONS_H_ */
diff --git a/backends/bmv2/psa_switch/options.h b/backends/bmv2/psa_switch/options.h
@@ -43,6 +43,6 @@ class PsaSwitchOptions : public BMV2Options {
 
 using PsaSwitchContext = P4CContextWithOptions<PsaSwitchOptions>;
 
-};  // namespace BMV2
+}  // namespace BMV2
 
 #endif /* BACKENDS_BMV2_PSA_SWITCH_OPTIONS_H_ */
diff --git a/backends/bmv2/simple_switch/options.h b/backends/bmv2/simple_switch/options.h
@@ -40,6 +40,6 @@ class SimpleSwitchOptions : public BMV2Options {
 
 using SimpleSwitchContext = P4CContextWithOptions<SimpleSwitchOptions>;
 
-};  // namespace BMV2
+}  // namespace BMV2
 
 #endif /* BACKENDS_BMV2_SIMPLE_SWITCH_OPTIONS_H_ */
diff --git a/backends/dpdk/dpdkArch.h b/backends/dpdk/dpdkArch.h
@@ -1420,5 +1420,5 @@ struct DpdkHandleIPSec : public PassManager {
     }
 };
 
-};     // namespace DPDK
+}  // namespace DPDK
 #endif /* BACKENDS_DPDK_DPDKARCH_H_ */
diff --git a/backends/dpdk/options.cpp b/backends/dpdk/options.cpp
@@ -26,4 +26,4 @@ const char *DpdkOptions::getIncludePath() {
     return path.c_str();
 }
 
-};  // namespace DPDK
+}  // namespace DPDK
diff --git a/backends/ebpf/psa/ebpfPsaTable.cpp b/backends/ebpf/psa/ebpfPsaTable.cpp
@@ -882,10 +882,9 @@ EBPFTablePSA::EntriesGroupedByMask_t EBPFTablePSA::getConstEntriesGroupedByMask(
     unsigned priority = entries->entries.size() + 1;
     for (auto entry : entries->entries) {
         cstring mask = maskGenerator.getMaskStr(entry);
-        ConstTernaryEntryDesc desc = {
-            .entry = entry,
-            .priority = priority--,
-        };
+        ConstTernaryEntryDesc desc;
+        desc.entry = entry;
+        desc.priority = priority--;
         entriesGroupedByMask[mask].emplace_back(desc);
     }
 

diff --git a/backends/p4tools/common/compiler/reachability.cpp b/backends/p4tools/common/compiler/reachability.cpp
@@ -355,7 +355,7 @@ void ReachabilityEngineState::setState(std::list<const DCGVertexType *> ls) { st
 
 const DCGVertexType *ReachabilityEngineState::getPrevNode() { return prevNode; }
 
-void ReachabilityEngineState::setPrevNode(const DCGVertexType *n) { prevNode = n; };
+void ReachabilityEngineState::setPrevNode(const DCGVertexType *n) { prevNode = n; }
 
 bool ReachabilityEngineState::isEmpty() { return state.empty(); }
 

diff --git a/control-plane/typeSpecConverter.cpp b/control-plane/typeSpecConverter.cpp
@@ -62,16 +62,11 @@ bool hasTranslationAnnotation(const IR::Type *type, TranslationAnnotation *paylo
     // See p4rtControllerType in p4parser.ypp for an explanation of how the
     // second argument is encoded.
     if (second_arg->to<IR::StringLiteral>() != nullptr) {
-        payload->controller_type = ControllerType{
-            .type = ControllerType::kString,
-            .width = 0,
-        };
+        payload->controller_type = ControllerType{ControllerType::kString, 0};
         return true;
     } else if (second_arg->to<IR::Constant>() != nullptr) {
-        payload->controller_type = ControllerType{
-            .type = ControllerType::kBit,
-            .width = second_arg->to<IR::Constant>()->asInt(),
-        };
+        payload->controller_type =
+            ControllerType{ControllerType::kBit, second_arg->to<IR::Constant>()->asInt()};
         return true;
     }
     BUG("%1%: expected second argument to @p4runtime_translation to parse as an"

diff --git a/frontends/parsers/p4/p4lexer.ll b/frontends/parsers/p4/p4lexer.ll
@@ -24,6 +24,7 @@ using Parser = P4::P4Parser;
 #pragma GCC diagnostic ignored "-Wunused-variable"
 #pragma GCC diagnostic ignored "-Wsign-compare"
 #pragma GCC diagnostic ignored "-Wtautological-undefined-compare"
+#pragma GCC diagnostic ignored "-Wimplicit-fallthrough"
 #ifdef __clang__
 #pragma clang diagnostic ignored "-Wnull-conversion"
 #pragma clang diagnostic ignored "-Wregister"

diff --git a/frontends/parsers/v1/v1lexer.ll b/frontends/parsers/v1/v1lexer.ll
@@ -20,6 +20,7 @@ using Parser = V1::V1Parser;
 #pragma GCC diagnostic ignored "-Wunused-variable"
 #pragma GCC diagnostic ignored "-Wsign-compare"
 #pragma GCC diagnostic ignored "-Wtautological-undefined-compare"
+#pragma GCC diagnostic ignored "-Wimplicit-fallthrough"
 #ifdef __clang__
 #pragma clang diagnostic ignored "-Wnull-conversion"
 #pragma clang diagnostic ignored "-Wregister"

diff --git a/lib/exceptions.h b/lib/exceptions.h
@@ -28,9 +28,10 @@ limitations under the License.
 namespace Util {
 
 // colors to pretty print messages
-constexpr char ANSI_RED[] = "\e[31m";
-constexpr char ANSI_BLUE[] = "\e[34m";
-constexpr char ANSI_CLR[] = "\e[0m";
+// \e is non-standard escape sequence, use codepoint \33 instead
+constexpr char ANSI_RED[] = "\33[31m";
+constexpr char ANSI_BLUE[] = "\33[34m";
+constexpr char ANSI_CLR[] = "\33[0m";
 
 /// Checks if stderr is redirected to a file
 /// Check is done only once and then saved to a static variable