Make preprocess and getIncludePath const functions. (#4785)

* Make preprocess and getIncludePath const functions. Signed-off-by: fruffy <[email protected]> * Clarification. Signed-off-by: fruffy <[email protected]> * Use a unique pointer instead of a custom structure. Signed-off-by: fruffy <[email protected]> --------- Signed-off-by: fruffy <[email protected]>
p4lang · Jul 16, 2024 · 0fafda0 · 0fafda0
1 parent bc05831
commit 0fafda0
Show file tree

Hide file tree

Showing 15 changed files with 85 additions and 72 deletions.
diff --git a/backends/dpdk/options.cpp b/backends/dpdk/options.cpp
@@ -17,7 +17,7 @@ std::vector<const char *> *DpdkOptions::process(int argc, char *const argv[]) {
     return remainingOptions;
 }
 
-const char *DpdkOptions::getIncludePath() {
+const char *DpdkOptions::getIncludePath() const {
     char *driverP4IncludePath =
         isv1() ? getenv("P4C_14_INCLUDE_PATH") : getenv("P4C_16_INCLUDE_PATH");
     cstring path = cstring::empty;

diff --git a/backends/dpdk/options.h b/backends/dpdk/options.h
@@ -104,7 +104,7 @@ class DpdkOptions : public CompilerOptions {
     /// Process the command line arguments and set options accordingly.
     std::vector<const char *> *process(int argc, char *const argv[]) override;
 
-    const char *getIncludePath() override;
+    const char *getIncludePath() const override;
 };
 
 using DpdkContext = P4CContextWithOptions<DpdkOptions>;

diff --git a/backends/p4tools/modules/smith/options.cpp b/backends/p4tools/modules/smith/options.cpp
@@ -17,7 +17,7 @@ SmithOptions &SmithOptions::get() {
     return INSTANCE;
 }
 
-const char *SmithOptions::getIncludePath() {
+const char *SmithOptions::getIncludePath() const {
     P4C_UNIMPLEMENTED("getIncludePath is not implemented for P4Smith.");
 }
 

diff --git a/backends/p4tools/modules/smith/options.h b/backends/p4tools/modules/smith/options.h
@@ -11,7 +11,7 @@ class SmithOptions : public AbstractP4cToolOptions {
     ~SmithOptions() override = default;
     static SmithOptions &get();
 
-    const char *getIncludePath() override;
+    const char *getIncludePath() const override;
     void processArgs(const std::vector<const char *> &args);
 
  private:

diff --git a/backends/p4tools/modules/testgen/options.cpp b/backends/p4tools/modules/testgen/options.cpp
@@ -24,7 +24,7 @@ TestgenOptions &TestgenOptions::get() {
     return INSTANCE;
 }
 
-const char *TestgenOptions::getIncludePath() {
+const char *TestgenOptions::getIncludePath() const {
     P4C_UNIMPLEMENTED("getIncludePath not implemented for P4Testgen.");
 }
 

diff --git a/backends/p4tools/modules/testgen/options.h b/backends/p4tools/modules/testgen/options.h
@@ -102,7 +102,7 @@ class TestgenOptions : public AbstractP4cToolOptions {
     /// Defaults to the name of the input program, if provided.
     std::optional<cstring> testBaseName;
 
-    const char *getIncludePath() override;
+    const char *getIncludePath() const override;
 
  protected:
     bool validateOptions() const override;

diff --git a/frontends/common/parseInput.cpp b/frontends/common/parseInput.cpp
@@ -17,25 +17,21 @@ limitations under the License.
 #include "parseInput.h"
 
 #include <cstdio>
-#include <iostream>
-#include <optional>
 #include <sstream>
 
 #include "frontends/p4/fromv1.0/converters.h"
-#include "frontends/p4/frontend.h"
 #include "frontends/parsers/parserDriver.h"
 #include "lib/error.h"
-#include "lib/source_file.h"
 
 namespace P4 {
 
 const IR::P4Program *parseP4String(const char *sourceFile, unsigned sourceLine,
                                    const std::string &input,
                                    CompilerOptions::FrontendVersion version) {
     std::istringstream stream(input);
-    auto result =
+    const auto *result =
         version == CompilerOptions::FrontendVersion::P4_14
-            ? parseV1Program<std::istringstream, P4V1::Converter>(stream, sourceFile, sourceLine)
+            ? parseV1Program<std::istringstream &, P4V1::Converter>(stream, sourceFile, sourceLine)
             : P4ParserDriver::parse(stream, sourceFile, sourceLine);
 
     if (::errorCount() > 0) {

diff --git a/frontends/common/parseInput.h b/frontends/common/parseInput.h
@@ -18,11 +18,10 @@ limitations under the License.
 #define FRONTENDS_COMMON_PARSEINPUT_H_
 
 #include "frontends/common/options.h"
+#include "frontends/common/parser_options.h"
 #include "frontends/p4/fromv1.0/converters.h"
-#include "frontends/p4/frontend.h"
 #include "frontends/parsers/parserDriver.h"
 #include "lib/error.h"
-#include "lib/source_file.h"
 
 namespace IR {
 class P4Program;
@@ -31,7 +30,7 @@ class P4Program;
 namespace P4 {
 
 template <typename Input, typename C = P4V1::Converter>
-static const IR::P4Program *parseV1Program(Input &stream, std::string_view sourceFile,
+static const IR::P4Program *parseV1Program(Input stream, std::string_view sourceFile,
                                            unsigned sourceLine,
                                            std::optional<DebugHook> debugHook = std::nullopt) {
     // We load the model before parsing the input file, so that the SourceInfo
@@ -61,29 +60,33 @@ static const IR::P4Program *parseV1Program(Input &stream, std::string_view sourc
  * on failure. If failure occurs, an error will also be reported.
  */
 template <typename C = P4V1::Converter>
-const IR::P4Program *parseP4File(ParserOptions &options) {
+const IR::P4Program *parseP4File(const ParserOptions &options) {
     BUG_CHECK(&options == &P4CContext::get().options(),
               "Parsing using options that don't match the current "
               "compiler context");
-    FILE *in = nullptr;
+
+    const IR::P4Program *result = nullptr;
     if (options.doNotPreprocess) {
-        in = fopen(options.file.c_str(), "r");
-        if (in == nullptr) {
+        auto *file = fopen(options.file.c_str(), "r");
+        if (file == nullptr) {
             ::error(ErrorType::ERR_NOT_FOUND, "%1%: No such file or directory.", options.file);
             return nullptr;
         }
+        result = options.isv1() ? parseV1Program<FILE *, C>(file, options.file.string(), 1,
+                                                            options.getDebugHook())
+                                : P4ParserDriver::parse(file, options.file.string());
+        fclose(file);
     } else {
-        in = options.preprocess();
-        if (::errorCount() > 0 || in == nullptr) return nullptr;
-    }
-
-    auto result = options.isv1() ? parseV1Program<FILE *, C>(in, options.file.string(), 1,
-                                                             options.getDebugHook())
-                                 : P4ParserDriver::parse(in, options.file.string());
-    if (options.doNotPreprocess) {
-        fclose(in);
-    } else {
-        options.closePreprocessedInput(in);
+        auto preprocessorResult = options.preprocess();
+        if (::errorCount() > 0 || !preprocessorResult.has_value()) {
+            return nullptr;
+        }
+        // Need to assign file here because the parser requires an lvalue.
+        result =
+            options.isv1()
+                ? parseV1Program<FILE *, C>(preprocessorResult.value().get(), options.file.string(),
+                                            1, options.getDebugHook())
+                : P4ParserDriver::parse(preprocessorResult.value().get(), options.file.string());
     }
 
     if (::errorCount() > 0) {

diff --git a/frontends/common/parser_options.cpp b/frontends/common/parser_options.cpp
@@ -23,6 +23,7 @@ limitations under the License.
 #include <sys/types.h>
 #include <sys/wait.h>
 
+#include <memory>
 #include <regex>
 #include <unordered_set>
 
@@ -45,6 +46,22 @@ const char *p4_14includePath = CONFIG_PKGDATADIR "/p4_14include";
 
 using namespace P4::literals;
 
+void ParserOptions::closeFile(FILE *file) {
+    if (file == nullptr) {
+        return;
+    }
+    int exitCode = pclose(file);
+    if (WIFEXITED(exitCode) && WEXITSTATUS(exitCode) == 4) {
+        ::error(ErrorType::ERR_IO, "input file does not exist");
+        return;
+    }
+    if (exitCode != 0) {
+        ::error(ErrorType::ERR_IO, "Preprocessor returned exit code %d; aborting compilation",
+                exitCode);
+        return;
+    }
+}
+
 ParserOptions::ParserOptions(std::string_view defaultMessage) : Util::Options(defaultMessage) {
     registerOption(
         "--help", nullptr,
@@ -387,7 +404,7 @@ std::vector<const char *> *ParserOptions::process(int argc, char *const argv[])
     return remainingOptions;
 }
 
-const char *ParserOptions::getIncludePath() {
+const char *ParserOptions::getIncludePath() const {
     cstring path = cstring::empty;
     // the p4c driver sets environment variables for include
     // paths.  check the environment and add these to the command
@@ -400,11 +417,10 @@ const char *ParserOptions::getIncludePath() {
     return path.c_str();
 }
 
-FILE *ParserOptions::preprocess() {
+std::optional<ParserOptions::PreprocessorResult> ParserOptions::preprocess() const {
     FILE *in = nullptr;
 
     if (file == "-") {
-        file = "<stdin>";
         in = stdin;
     } else {
 #ifdef __clang__
@@ -421,32 +437,21 @@ FILE *ParserOptions::preprocess() {
         if (in == nullptr) {
             ::error(ErrorType::ERR_IO, "Error invoking preprocessor");
             perror("");
-            return nullptr;
+            return std::nullopt;
         }
-        close_input = true;
     }
 
     if (doNotCompile) {
-        char *line = NULL;
+        char *line = nullptr;
         size_t len = 0;
-        ssize_t read;
+        ssize_t read = 0;
 
-        while ((read = getline(&line, &len, in)) != -1) printf("%s", line);
-        closePreprocessedInput(in);
-        return nullptr;
-    }
-    return in;
-}
-
-void ParserOptions::closePreprocessedInput(FILE *inputStream) const {
-    if (close_input) {
-        int exitCode = pclose(inputStream);
-        if (WIFEXITED(exitCode) && WEXITSTATUS(exitCode) == 4)
-            ::error(ErrorType::ERR_IO, "input file %s does not exist", file);
-        else if (exitCode != 0)
-            ::error(ErrorType::ERR_IO, "Preprocessor returned exit code %d; aborting compilation",
-                    exitCode);
+        while ((read = getline(&line, &len, in)) != -1) {
+            printf("%s", line);
+        }
+        return std::nullopt;
     }
+    return ParserOptions::PreprocessorResult(in, &closeFile);
 }
 
 // From (folder, file.ext, suffix)  returns

diff --git a/frontends/common/parser_options.h b/frontends/common/parser_options.h
@@ -19,6 +19,7 @@ limitations under the License.
 #ifndef FRONTENDS_COMMON_PARSER_OPTIONS_H_
 #define FRONTENDS_COMMON_PARSER_OPTIONS_H_
 
+#include <cstdio>
 #include <filesystem>
 #include <set>
 
@@ -37,8 +38,6 @@ extern const char *p4_14includePath;
 /// This class contains the options for the front-ends.
 /// Each back-end should subclass this file.
 class ParserOptions : public Util::Options {
-    bool close_input = false;
-
     /// Annotation names that are to be ignored by the compiler.
     std::set<cstring> disabledAnnotations;
 
@@ -54,6 +53,12 @@ class ParserOptions : public Util::Options {
 
     std::vector<const char *> *process(int argc, char *const argv[]) override;
     enum class FrontendVersion { P4_14, P4_16 };
+
+    /// Tries to close the input stream associated with the result.
+    static void closeFile(FILE *file);
+    /// Records the result of the preprocessor.
+    using PreprocessorResult = std::unique_ptr<FILE, decltype(&closeFile)>;
+
     /// Name of executable that is being run.
     cstring exe_name;
     /// Which language to compile
@@ -77,11 +82,9 @@ class ParserOptions : public Util::Options {
     /// Expect that the only remaining argument is the input file.
     void setInputFile();
     /// Return target specific include path.
-    const char *getIncludePath() override;
+    const char *getIncludePath() const override;
     /// Returns the output of the preprocessor.
-    FILE *preprocess();
-    /// Closes the input stream returned by preprocess.
-    void closePreprocessedInput(FILE *input) const;
+    std::optional<ParserOptions::PreprocessorResult> preprocess() const;
     /// True if we are compiling a P4 v1.0 or v1.1 program
     bool isv1() const;
     /// Get a debug hook function suitable for insertion

diff --git a/frontends/p4/fromv1.0/programStructure.cpp b/frontends/p4/fromv1.0/programStructure.cpp
@@ -661,28 +661,34 @@ void ProgramStructure::createParser() {
 }
 
 void ProgramStructure::include(cstring filename, cstring ppoptions) {
-    if (included_files.count(filename)) return;
+    if (included_files.count(filename) != 0U) {
+        return;
+    }
     included_files.insert(filename);
     // the p4c driver sets environment variables for include
     // paths.  check the environment and add these to the command
-    // line for the preporicessor
+    // line for the preprocessor
     char *drvP4IncludePath = getenv("P4C_16_INCLUDE_PATH");
-    std::filesystem::path path(drvP4IncludePath ? drvP4IncludePath : p4includePath);
+    std::filesystem::path path((drvP4IncludePath != nullptr) ? drvP4IncludePath : p4includePath);
     path /= std::string(filename);
 
     CompilerOptions options;
-    if (ppoptions) {
+    if (ppoptions != nullptr) {
         options.preprocessor_options += " ";
         options.preprocessor_options += ppoptions;
     }
     options.langVersion = CompilerOptions::FrontendVersion::P4_16;
     options.file = path;
-    if (!::errorCount()) {
-        if (FILE *file = options.preprocess()) {
-            auto code = P4::P4ParserDriver::parse(file, options.file.string());
-            if (code && !::errorCount())
-                for (auto decl : code->objects) declarations->push_back(decl);
-            options.closePreprocessedInput(file);
+    if (::errorCount() == 0U) {
+        auto preprocessorResult = options.preprocess();
+        if (preprocessorResult.has_value()) {
+            const auto *code =
+                P4::P4ParserDriver::parse(preprocessorResult.value().get(), options.file.string());
+            if ((code != nullptr) && (::errorCount() == 0U)) {
+                for (const auto *decl : code->objects) {
+                    declarations->push_back(decl);
+                }
+            }
         }
     }
 }

diff --git a/lib/options.h b/lib/options.h
@@ -100,7 +100,7 @@ class Options {
      */
     virtual std::vector<const char *> *process(int argc, char *const argv[]);
 
-    virtual const char *getIncludePath() = 0;
+    [[nodiscard]] virtual const char *getIncludePath() const = 0;
     cstring getCompileCommand() { return compileCommand; }
     cstring getBuildDate() { return buildDate; }
     cstring getBinaryName() { return cstring(binaryName); }

diff --git a/test/gtest/midend_def_use.cpp b/test/gtest/midend_def_use.cpp
@@ -20,6 +20,7 @@ limitations under the License.
 #include <string>
 
 #include "frontends/common/parseInput.h"
+#include "frontends/p4/frontend.h"
 #include "helpers.h"
 #include "midend/def_use.h"
 #include "midend_pass.h"

diff --git a/test/gtest/p4runtime.cpp b/test/gtest/p4runtime.cpp
@@ -19,7 +19,6 @@ limitations under the License.
 #include <google/protobuf/util/message_differencer.h>
 #include <gtest/gtest.h>
 
-#include <iterator>
 #include <optional>
 #include <string>
 #include <vector>
@@ -36,6 +35,7 @@ limitations under the License.
 #include "frontends/common/parseInput.h"
 #include "frontends/common/resolveReferences/referenceMap.h"
 #include "frontends/common/resolveReferences/resolveReferences.h"
+#include "frontends/p4/frontend.h"
 #include "frontends/p4/parseAnnotations.h"
 #include "frontends/p4/typeChecking/typeChecker.h"
 #include "frontends/p4/typeMap.h"

diff --git a/test/gtest/parser_unroll.cpp b/test/gtest/parser_unroll.cpp
@@ -1,11 +1,10 @@
 #include <gtest/gtest.h>
 
 #include <cstdlib>
-#include <fstream>
 
 #include "frontends/common/parseInput.h"
+#include "frontends/p4/frontend.h"
 #include "ir/ir.h"
-#include "lib/log.h"
 #include "test/gtest/env.h"
 #include "test/gtest/helpers.h"
 #include "test/gtest/midend_pass.h"