Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "-M" and "-MM" command line options to dump the dependencies of t… #313

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mingodad
Copy link

…he main source file

@mingodad
Copy link
Author

I just found that it has a bug that can print a line continuation after the last filename if noSysPath is specified and there is one filename withing SysPath in the last position.

@robertoraggi
Copy link
Owner

Thanks for this PR. I think this breaks the WASI build. The problem is that the filesystem API is not available in the WASI SDK, mostly due to limitations of the WASI API (e.g. missing cwd). So we cannot use filesystem directly. Please have a look at the tiny wrapper in src/parser/cxx/private/path.h that I use when targeting WASI. If you have nodejs and docker installed you can test the change using the commands:

nom ci
npm run build:wasi

...

/code/src/parser/cxx/preprocessor.cc:2268:30: error: no member named 'stem' in 'cxx::fs::path'
  const std::string stem = p.stem();

@mingodad
Copy link
Author

mingodad commented Jan 2, 2024

Can it be surrounded in preprocessor macros to be available only for other targets ?

@robertoraggi
Copy link
Owner

robertoraggi commented Jan 2, 2024

Can it be surrounded in preprocessor macros to be available only for other targets ?

yes of course, you can test for CXX_NO_FILESYSTEM, I can add some code to emulate stem() when I have time.
Also please prefix the commit message with chore: or fix:. And it may be a good to have a test for this feature in the tests/unit_testss/preprocessor. This project is using lit and python for testing. You will need to install tests/unit_tests/requirements.txt, reconfigure and then you can use ctest, please look at https://github.com/robertoraggi/cplusplus/blob/main/.github/workflows/ci.yml#L28C15-L28C19 for more info

@mingodad
Copy link
Author

mingodad commented Jan 2, 2024

Do you mean something like this ?

--------------------------- src/frontend/cxx/cli.cc ---------------------------
index 795cd68..7ed34a7 100644
@@ -131,13 +131,13 @@ std::vector<CLIOptionDescr> options{
 
     {"-dM", "Print macro definitions in -E mode instead of normal output",
      &CLI::opt_dM},
-
+#ifndef CXX_NO_FILESYSTEM
     {"-M", "Print dependencies of the main source file",
      &CLI::opt_M},
 
     {"-MM", "Print dependencies of the main source file, excluding system path",
      &CLI::opt_MM},
-
+#endif
     {"-S", "Only run preprocess and compilation steps", &CLI::opt_S,
      CLIOptionVisibility::kExperimental},
 

---------------------------- src/frontend/cxx/cli.h ----------------------------
index 9c7ec57..c7f3fce 100644
@@ -54,8 +54,10 @@ class CLI {
   bool opt_ast_dump = false;
   bool opt_ir_dump = false;
   bool opt_dM = false;
+#ifndef CXX_NO_FILESYSTEM
   bool opt_MM = false;
   bool opt_M = false;
+#endif
   bool opt_dump_symbols = false;
   bool opt_dump_tokens = false;
   bool opt_E = false;

------------------------- src/frontend/cxx/frontend.cc -------------------------
index d51809b..c1b14cc 100644
@@ -238,7 +238,11 @@ auto runOnFile(const CLI& cli, const std::string& fileName) -> bool {
   }
 
   if (auto source = readAll(fileName)) {
-    if (cli.opt_E && !(cli.opt_dM || cli.opt_M || cli.opt_MM)) {
+    if (cli.opt_E && !(cli.opt_dM
+#ifndef CXX_NO_FILESYSTEM
+            || cli.opt_M || cli.opt_MM
+#endif
+            )) {
       preprocesor->preprocess(std::move(*source), fileName, output);
       shouldExit = true;
     } else {
@@ -247,12 +251,14 @@ auto runOnFile(const CLI& cli, const std::string& fileName) -> bool {
       if (cli.opt_dM) {
         preprocesor->printMacros(output);
         shouldExit = true;
+#ifndef CXX_NO_FILESYSTEM
       } else if (cli.opt_M) {
         preprocesor->printDependencies(output, false);
         shouldExit = true;
       } else if (cli.opt_MM) {
         preprocesor->printDependencies(output, true);
         shouldExit = true;
+#endif
       } else if (cli.opt_dump_tokens) {
         dumpTokens(cli, unit, output);
         shouldExit = true;

------------------------ src/parser/cxx/preprocessor.cc ------------------------
index 296a37e..21d40ed 100644
@@ -2261,7 +2261,7 @@ void Preprocessor::printMacros(std::ostream &out) const {
     out << cxx::format("\n");
   }
 }
-
+#ifndef CXX_NO_FILESYSTEM
 void Preprocessor::printDependencies(std::ostream &out, bool noSysPath) const {
   if (d->sourceFiles_.size() == 0) return;
   const auto p = fs::path(d->sourceFiles_[0]->fileName);
@@ -2282,7 +2282,7 @@ void Preprocessor::printDependencies(std::ostream &out, bool noSysPath) const {
   }
   out << "\n";
 }
-
+#endif
 void Preprocessor::getTokenStartPosition(const Token &token, unsigned *line,
                                          unsigned *column,
                                          std::string_view *fileName) const {

------------------------ src/parser/cxx/preprocessor.h ------------------------
index 89938a0..a32b96e 100644
@@ -96,9 +96,9 @@ class Preprocessor {
   void undefMacro(const std::string &name);
 
   void printMacros(std::ostream &out) const;
-
+#ifndef CXX_NO_FILESYSTEM
   void printDependencies(std::ostream &out, bool noSysPath) const;
-
+#endif
   void getTokenStartPosition(const Token &token, unsigned *line,
                              unsigned *column,
                              std::string_view *fileName) const;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants