FarisRehman updated this revision to Diff 313108.
FarisRehman edited the summary of this revision.
FarisRehman added a comment.
Address review comments
Address comments with the following changes:
- Add a dedicated method to adding preprocessing options in Flang.cpp
- Change preprocessorOpts to use std::shared_ptr
- Separate SetDefaultFortranOpts from SetFortranOpts in CompilerInvocation
- Fix method and variable naming in CompilerInvocation.cpp
- Add comments to new methods and fields
- Add missing file header to PreprocessorOptions.h
- Remove unnecessary changes and method calls
- Add a regression test to check end-of-line character behaviour in a macro
definition
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93401/new/
https://reviews.llvm.org/D93401
Files:
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Flang.cpp
clang/lib/Driver/ToolChains/Flang.h
flang/include/flang/Frontend/CompilerInstance.h
flang/include/flang/Frontend/CompilerInvocation.h
flang/include/flang/Frontend/PreprocessorOptions.h
flang/lib/Frontend/CompilerInstance.cpp
flang/lib/Frontend/CompilerInvocation.cpp
flang/test/Flang-Driver/driver-help-hidden.f90
flang/test/Flang-Driver/driver-help.f90
flang/test/Flang-Driver/macro_def_undef.f90
flang/test/Flang-Driver/macro_multiline.f90
Index: flang/test/Flang-Driver/macro_multiline.f90
===================================================================
--- /dev/null
+++ flang/test/Flang-Driver/macro_multiline.f90
@@ -0,0 +1,26 @@
+! Ensure \n and anything that follows after in a macro definition (-D) is ignored.
+
+! REQUIRES: new-flang-driver
+
+!--------------------------
+! FLANG DRIVER (flang-new)
+!--------------------------
+! RUN: printf -- "-DX=A\\\\\nTHIS_SHOULD_NOT_EXIST_IN_THE_OUTPUT\n" | xargs %flang-new -E %s 2>&1 | FileCheck -strict-whitespace %s
+
+!-----------------------------------------
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!-----------------------------------------
+! RUN: printf -- "-DX=A\\\\\nTHIS_SHOULD_NOT_EXIST_IN_THE_OUTPUT\n" | xargs %flang-new -fc1 -E %s 2>&1 | FileCheck -strict-whitespace %s
+
+
+!-------------------------------
+! EXPECTED OUTPUT FOR MACRO 'X'
+!-------------------------------
+! printf -- "-DX=A\\\\\nTHIS_SHOULD_NOT_EXIST_IN_THE_OUTPUT\n" | xargs flang-new -E %s
+! CHECK:program a
+! CHECK-NOT:program x
+! CHECK-NEXT:end
+
+! Macro.F:
+program X
+end
\ No newline at end of file
Index: flang/test/Flang-Driver/macro_def_undef.f90
===================================================================
--- /dev/null
+++ flang/test/Flang-Driver/macro_def_undef.f90
@@ -0,0 +1,43 @@
+! Ensure arguments -D and -U work as expected.
+
+! REQUIRES: new-flang-driver
+
+!--------------------------
+! FLANG DRIVER (flang-new)
+!--------------------------
+! RUN: %flang-new -E %s 2>&1 | FileCheck %s --check-prefix=UNDEFINED
+! RUN: %flang-new -E -DX=A %s 2>&1 | FileCheck %s --check-prefix=DEFINED
+! RUN: %flang-new -E -DX=A -UX %s 2>&1 | FileCheck %s --check-prefix=UNDEFINED
+
+!-----------------------------------------
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!-----------------------------------------
+! RUN: %flang-new -fc1 -E %s 2>&1 | FileCheck %s --check-prefix=UNDEFINED
+! RUN: %flang-new -fc1 -E -DX=A %s 2>&1 | FileCheck %s --check-prefix=DEFINED
+! RUN: %flang-new -fc1 -E -DX -UX %s 2>&1 | FileCheck %s --check-prefix=UNDEFINED
+
+
+!--------------------------------------------
+! EXPECTED OUTPUT FOR AN UNDEFINED MACRO
+!--------------------------------------------
+! flang-new -E %s
+! flang-new -E -DX=A -UX %s
+! UNDEFINED:program b
+! UNDEFINED-NOT:program x
+! UNDEFINED-NEXT:end
+
+!--------------------------------------------
+! EXPECTED OUTPUT FOR MACRO 'X' DEFINED AS A
+!--------------------------------------------
+! flang-new -E -DX=A %s
+! DEFINED:program a
+! DEFINED-NOT:program b
+! DEFINED-NEXT:end
+
+! Macro.F:
+#ifdef X
+program X
+#else
+program B
+#endif
+end
\ No newline at end of file
Index: flang/test/Flang-Driver/driver-help.f90
===================================================================
--- flang/test/Flang-Driver/driver-help.f90
+++ flang/test/Flang-Driver/driver-help.f90
@@ -19,11 +19,13 @@
! HELP-EMPTY:
! HELP-NEXT:OPTIONS:
! HELP-NEXT: -### Print (but do not run) the commands to run for this compilation
+! HELP-NEXT: -D <macro>=<value> Define <macro> to <value> (or 1 if <value> omitted)
! HELP-NEXT: -E Only run the preprocessor
! HELP-NEXT: -fcolor-diagnostics Enable colors in diagnostics
! HELP-NEXT: -fno-color-diagnostics Disable colors in diagnostics
! HELP-NEXT: -help Display available options
! HELP-NEXT: -o <file> Write output to <file>
+! HELP-NEXT: -U <macro> Undefine macro <macro>
! HELP-NEXT: --version Print version information
!-------------------------------------------------------------
@@ -32,10 +34,12 @@
! HELP-FC1:USAGE: flang-new
! HELP-FC1-EMPTY:
! HELP-FC1-NEXT:OPTIONS:
-! HELP-FC1-NEXT: -E Only run the preprocessor
-! HELP-FC1-NEXT: -help Display available options
-! HELP-FC1-NEXT: -o <file> Write output to <file>
-! HELP-FC1-NEXT: --version Print version information
+! HELP-FC1-NEXT: -D <macro>=<value> Define <macro> to <value> (or 1 if <value> omitted)
+! HELP-FC1-NEXT: -E Only run the preprocessor
+! HELP-FC1-NEXT: -help Display available options
+! HELP-FC1-NEXT: -o <file> Write output to <file>
+! HELP-FC1-NEXT: -U <macro> Undefine macro <macro>
+! HELP-FC1-NEXT: --version Print version information
!---------------
! EXPECTED ERROR
Index: flang/test/Flang-Driver/driver-help-hidden.f90
===================================================================
--- flang/test/Flang-Driver/driver-help-hidden.f90
+++ flang/test/Flang-Driver/driver-help-hidden.f90
@@ -19,12 +19,14 @@
! CHECK-EMPTY:
! CHECK-NEXT:OPTIONS:
! CHECK-NEXT: -### Print (but do not run) the commands to run for this compilation
+! CHECK-NEXT: -D <macro>=<value> Define <macro> to <value> (or 1 if <value> omitted)
! CHECK-NEXT: -E Only run the preprocessor
! CHECK-NEXT: -fcolor-diagnostics Enable colors in diagnostics
! CHECK-NEXT: -fno-color-diagnostics Disable colors in diagnostics
! CHECK-NEXT: -help Display available options
! CHECK-NEXT: -o <file> Write output to <file>
! CHECK-NEXT: -test-io Run the InputOuputTest action. Use for development and testing only.
+! CHECK-NEXT: -U <macro> Undefine macro <macro>
! CHECK-NEXT: --version Print version information
!-------------------------------------------------------------
Index: flang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- flang/lib/Frontend/CompilerInvocation.cpp
+++ flang/lib/Frontend/CompilerInvocation.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "flang/Frontend/CompilerInvocation.h"
+#include "flang/Frontend/PreprocessorOptions.h"
#include "clang/Basic/AllDiagnostics.h"
#include "clang/Basic/DiagnosticDriver.h"
#include "clang/Basic/DiagnosticOptions.h"
@@ -26,10 +27,12 @@
// Initialization.
//===----------------------------------------------------------------------===//
CompilerInvocationBase::CompilerInvocationBase()
- : diagnosticOpts_(new clang::DiagnosticOptions()) {}
+ : diagnosticOpts_(new clang::DiagnosticOptions()),
+ preprocessorOpts_(new PreprocessorOptions()) {}
CompilerInvocationBase::CompilerInvocationBase(const CompilerInvocationBase &x)
- : diagnosticOpts_(new clang::DiagnosticOptions(x.GetDiagnosticOpts())) {}
+ : diagnosticOpts_(new clang::DiagnosticOptions(x.GetDiagnosticOpts())),
+ preprocessorOpts_(new PreprocessorOptions(x.preprocessorOpts())) {}
CompilerInvocationBase::~CompilerInvocationBase() = default;
@@ -152,6 +155,24 @@
return dashX;
}
+/// Parses all preprocessor input arguments and populates the preprocessor
+/// options accordingly.
+///
+/// \param opts The preprocessor options instance
+/// \param args The list of input arguments
+static void parsePreprocessorArgs(
+ Fortran::frontend::PreprocessorOptions &opts, llvm::opt::ArgList &args) {
+ // Add macros from the command line.
+ for (const auto *currentArg : args.filtered(
+ clang::driver::options::OPT_D, clang::driver::options::OPT_U)) {
+ if (currentArg->getOption().matches(clang::driver::options::OPT_D)) {
+ opts.addMacroDef(currentArg->getValue());
+ } else {
+ opts.addMacroUndef(currentArg->getValue());
+ }
+ }
+}
+
bool CompilerInvocation::CreateFromArgs(CompilerInvocation &res,
llvm::ArrayRef<const char *> commandLineArgs,
clang::DiagnosticsEngine &diags) {
@@ -180,10 +201,47 @@
// Parse the frontend args
ParseFrontendArgs(res.frontendOpts(), args, diags);
+ // Parse the preprocessor args
+ parsePreprocessorArgs(res.preprocessorOpts(), args);
return success;
}
+/// Collect the macro definitions provided by the given preprocessor
+/// options into the parser options.
+///
+/// \param ppOpts The preprocessor options (input)
+/// \param opts The fortran options (output)
+static void collectMacroDefinitions(
+ const PreprocessorOptions &ppOpts, Fortran::parser::Options &opts) {
+ for (unsigned i = 0, n = ppOpts.Macros.size(); i != n; ++i) {
+ llvm::StringRef macro = ppOpts.Macros[i].first;
+ bool isUndef = ppOpts.Macros[i].second;
+
+ std::pair<llvm::StringRef, llvm::StringRef> macroPair = macro.split('=');
+ llvm::StringRef macroName = macroPair.first;
+ llvm::StringRef macroBody = macroPair.second;
+
+ // For an #undef'd macro, we only care about the name.
+ if (isUndef) {
+ opts.predefinitions.emplace_back(
+ macroName.str(), std::optional<std::string>{});
+ continue;
+ }
+
+ // For a #define'd macro, figure out the actual definition.
+ if (macroName.size() == macro.size())
+ macroBody = "1";
+ else {
+ // Note: GCC drops anything following an end-of-line character.
+ llvm::StringRef::size_type End = macroBody.find_first_of("\n\r");
+ macroBody = macroBody.substr(0, End);
+ }
+ opts.predefinitions.emplace_back(
+ macroName, std::optional<std::string>(macroBody.str()));
+ }
+}
+
void CompilerInvocation::SetDefaultFortranOpts() {
auto &fortranOptions = fortranOpts();
@@ -192,3 +250,10 @@
fortranOptions.searchDirectories = searchDirectories;
fortranOptions.isFixedForm = false;
}
+
+void CompilerInvocation::SetFortranOpts() {
+ auto &fortranOptions = fortranOpts();
+ const auto &preprocessorOptions = preprocessorOpts();
+
+ collectMacroDefinitions(preprocessorOptions, fortranOptions);
+}
\ No newline at end of file
Index: flang/lib/Frontend/CompilerInstance.cpp
===================================================================
--- flang/lib/Frontend/CompilerInstance.cpp
+++ flang/lib/Frontend/CompilerInstance.cpp
@@ -130,6 +130,8 @@
// TODO: Instead of defaults we should be setting these options based on the
// user input.
this->invocation().SetDefaultFortranOpts();
+ // Set the fortran options to user-based input.
+ this->invocation().SetFortranOpts();
// Connect Input to a CompileInstance
for (const FrontendInputFile &fif : frontendOpts().inputs_) {
Index: flang/include/flang/Frontend/PreprocessorOptions.h
===================================================================
--- /dev/null
+++ flang/include/flang/Frontend/PreprocessorOptions.h
@@ -0,0 +1,42 @@
+//===- PreprocessorOptions.h ------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file contains the declaration of the PreprocessorOptions class, which
+/// is the class for all preprocessor options.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_FLANG_PREPROCESSOROPTIONS_H
+#define LLVM_FLANG_PREPROCESSOROPTIONS_H
+
+#include "llvm/ADT/StringRef.h"
+
+namespace Fortran::frontend {
+
+/// This class is used for passing the various options used
+/// in preprocessor initialization to the parser options.
+class PreprocessorOptions {
+public:
+ std::vector<std::pair<std::string, /*isUndef*/ bool>> Macros;
+
+public:
+ PreprocessorOptions() {}
+
+ void addMacroDef(llvm::StringRef Name) {
+ Macros.emplace_back(std::string(Name), false);
+ }
+
+ void addMacroUndef(llvm::StringRef Name) {
+ Macros.emplace_back(std::string(Name), true);
+ }
+};
+
+} // namespace Fortran::frontend
+
+#endif // LLVM_FLANG_PREPROCESSOROPTIONS_H
\ No newline at end of file
Index: flang/include/flang/Frontend/CompilerInvocation.h
===================================================================
--- flang/include/flang/Frontend/CompilerInvocation.h
+++ flang/include/flang/Frontend/CompilerInvocation.h
@@ -9,10 +9,12 @@
#define LLVM_FLANG_FRONTEND_COMPILERINVOCATION_H
#include "flang/Frontend/FrontendOptions.h"
+#include "flang/Frontend/PreprocessorOptions.h"
#include "flang/Parser/parsing.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticOptions.h"
#include "llvm/Option/ArgList.h"
+#include <memory>
namespace Fortran::frontend {
@@ -27,6 +29,8 @@
public:
/// Options controlling the diagnostic engine.
llvm::IntrusiveRefCntPtr<clang::DiagnosticOptions> diagnosticOpts_;
+ /// Options for the preprocessor.
+ std::shared_ptr<Fortran::frontend::PreprocessorOptions> preprocessorOpts_;
CompilerInvocationBase();
CompilerInvocationBase(const CompilerInvocationBase &x);
@@ -38,6 +42,11 @@
const clang::DiagnosticOptions &GetDiagnosticOpts() const {
return *diagnosticOpts_.get();
}
+
+ PreprocessorOptions &preprocessorOpts() { return *preprocessorOpts_; }
+ const PreprocessorOptions &preprocessorOpts() const {
+ return *preprocessorOpts_;
+ }
};
class CompilerInvocation : public CompilerInvocationBase {
@@ -74,6 +83,10 @@
// need to extend frontendOpts_ first. Next, we need to add the corresponding
// compiler driver options in libclangDriver.
void SetDefaultFortranOpts();
+
+ /// Set the Fortran options to user-specified values.
+ /// These values are found in the preprocessor options.
+ void SetFortranOpts();
};
} // end namespace Fortran::frontend
Index: flang/include/flang/Frontend/CompilerInstance.h
===================================================================
--- flang/include/flang/Frontend/CompilerInstance.h
+++ flang/include/flang/Frontend/CompilerInstance.h
@@ -10,6 +10,7 @@
#include "flang/Frontend/CompilerInvocation.h"
#include "flang/Frontend/FrontendAction.h"
+#include "flang/Frontend/PreprocessorOptions.h"
#include "flang/Parser/parsing.h"
#include "flang/Parser/provenance.h"
#include "llvm/Support/raw_ostream.h"
@@ -110,6 +111,13 @@
return invocation_->frontendOpts();
}
+ PreprocessorOptions &preprocessorOpts() {
+ return invocation_->preprocessorOpts();
+ }
+ const PreprocessorOptions &preprocessorOpts() const {
+ return invocation_->preprocessorOpts();
+ }
+
/// }
/// @name Diagnostics Engine
/// {
Index: clang/lib/Driver/ToolChains/Flang.h
===================================================================
--- clang/lib/Driver/ToolChains/Flang.h
+++ clang/lib/Driver/ToolChains/Flang.h
@@ -23,6 +23,15 @@
/// Flang compiler tool.
class LLVM_LIBRARY_VISIBILITY Flang : public Tool {
+private:
+ /// Add the specified preprocessing options from Args to CmdArgs,
+ /// claiming the arguments used.
+ ///
+ /// \param Args The list of input arguments
+ /// \param CmdArgs The list of output arguments
+ void AddPreprocessingOptions(const llvm::opt::ArgList &Args,
+ llvm::opt::ArgStringList &CmdArgs) const;
+
public:
Flang(const ToolChain &TC);
~Flang() override;
Index: clang/lib/Driver/ToolChains/Flang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Flang.cpp
+++ clang/lib/Driver/ToolChains/Flang.cpp
@@ -19,6 +19,11 @@
using namespace clang;
using namespace llvm::opt;
+void Flang::AddPreprocessingOptions(const ArgList &Args,
+ ArgStringList &CmdArgs) const {
+ Args.AddAllArgs(CmdArgs, {options::OPT_D, options::OPT_U});
+}
+
void Flang::ConstructJob(Compilation &C, const JobAction &JA,
const InputInfo &Output, const InputInfoList &Inputs,
const ArgList &Args, const char *LinkingOutput) const {
@@ -28,6 +33,8 @@
ArgStringList CmdArgs;
+ const InputInfo &Input = Inputs[0];
+
CmdArgs.push_back("-fc1");
// TODO: Eventually all actions will require a triple (e.g. `-triple
@@ -63,6 +70,13 @@
assert(false && "Unexpected action class for Flang tool.");
}
+ types::ID InputType = Input.getType();
+
+ // Add preprocessing options like -I, -D, etc. if we are using the
+ // preprocessor (i.e. skip when dealing with e.g. binary files).
+ if (types::getPreprocessedType(InputType) != types::TY_INVALID)
+ AddPreprocessingOptions(Args, CmdArgs);
+
if (Output.isFilename()) {
CmdArgs.push_back("-o");
CmdArgs.push_back(Output.getFilename());
@@ -70,7 +84,6 @@
assert(Output.isNothing() && "Invalid output.");
}
- const InputInfo &Input = Inputs[0];
assert(Input.isFilename() && "Invalid input.");
CmdArgs.push_back(Input.getFilename());
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -644,7 +644,7 @@
HelpText<"Include comments in preprocessed output">,
MarshallingInfoFlag<"PreprocessorOutputOpts.ShowComments">;
def D : JoinedOrSeparate<["-"], "D">, Group<Preprocessor_Group>,
- Flags<[CC1Option]>, MetaVarName<"<macro>=<value>">,
+ Flags<[CC1Option, FlangOption, FC1Option]>, MetaVarName<"<macro>=<value>">,
HelpText<"Define <macro> to <value> (or 1 if <value> omitted)">;
def E : Flag<["-"], "E">, Flags<[NoXarchOption,CC1Option, FlangOption, FC1Option]>, Group<Action_Group>,
HelpText<"Only run the preprocessor">;
@@ -743,7 +743,7 @@
def T : JoinedOrSeparate<["-"], "T">, Group<T_Group>,
MetaVarName<"<script>">, HelpText<"Specify <script> as linker script">;
def U : JoinedOrSeparate<["-"], "U">, Group<Preprocessor_Group>,
- Flags<[CC1Option]>, MetaVarName<"<macro>">, HelpText<"Undefine macro <macro>">;
+ Flags<[CC1Option, FlangOption, FC1Option]>, MetaVarName<"<macro>">, HelpText<"Undefine macro <macro>">;
def V : JoinedOrSeparate<["-"], "V">, Flags<[NoXarchOption, Unsupported]>;
def Wa_COMMA : CommaJoined<["-"], "Wa,">,
HelpText<"Pass the comma separated arguments in <arg> to the assembler">,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
