[flang-commits] [PATCH] D106727: [flang] Produce proper "preprocessor output" for -E option
Peter Klausler via Phabricator via flang-commits
flang-commits at lists.llvm.org
Wed Jul 28 09:10:48 PDT 2021
klausler marked 5 inline comments as done.
klausler added inline comments.
================
Comment at: flang/include/flang/Frontend/FrontendOptions.h:31-33
+ /// -fdebug-dump-cooked-chars
+ DebugDumpCookedChars,
+
----------------
awarzynski wrote:
> Not needed, `-fdebug-dump-cooked-chars` is not an action.
It used to be so in my first patch. Removed.
================
Comment at: flang/include/flang/Frontend/FrontendOptions.h:255-276
+public:
+ // -P: Suppress #line directives in -E output
+ bool noLineDirectives() const { return noLineDirectives_; }
+ FrontendOptions &set_noLineDirectives(bool yes = true) {
+ noLineDirectives_ = yes;
+ return *this;
+ }
----------------
awarzynski wrote:
> Do you think that we need accessors and mutators here? We've kept `FrontendOptions` rather basic - it's just a plain aggregate. There's no intention to make it any more "clever". Keeping everything "public" allows us to have a rather basic and concise interface.
>
> If we do add member methods for `noLineDirectives_` and `noLineDirectives_`, then perhaps we should do it for all member variables? But then we'll bloat this class.
>
> Also, we've been using bitfields rather than `bool`s for this class to keep it slim. Could we stick to bitfields for consistency?
I put them there because you're using names in that class with terminal underscores, and that's a convention in f18 (and google3) meant only for private data members; I didn't want to perpetuate that error. If everything here is meant to be public, this should be a struct whose members are all data and all public.
Bitfields can't have default initializers, which is the best way to initialize these things; otherwise the default value is separated from the declaration and possibly in another file.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D106727/new/
https://reviews.llvm.org/D106727
More information about the flang-commits
mailing list