[flang-commits] [flang] a0226f9 - [flang] Dodge bogus uninitialized data warning from gcc 10.1 via code cleanup

Tim Keith via flang-commits flang-commits at lists.llvm.org
Fri Jun 12 10:05:24 PDT 2020


Author: peter klausler
Date: 2020-06-12T10:05:05-07:00
New Revision: a0226f9bffa4c879ec7f8183738205661978cc39

URL: https://github.com/llvm/llvm-project/commit/a0226f9bffa4c879ec7f8183738205661978cc39
DIFF: https://github.com/llvm/llvm-project/commit/a0226f9bffa4c879ec7f8183738205661978cc39.diff

LOG: [flang] Dodge bogus uninitialized data warning from gcc 10.1 via code cleanup

G++ 10.1 emits inappropriate "use of uninitialized data" warnings when
compiling f18.  The warnings stem from two sites in templatized code
whose multiple instantiations magnified the number of warnings.

These changes dodge those warnings by making some innocuous changes to
the code.  In the parser, the idiom defaulted(cut >> x), which yields a
parser that always succeeds, has been replaced with a new equivalent
pass<T>() parser that returns a default-constructed value T{} in an
arguably more readable fashion.  This idiom was the only attestation of
the basic parser cut, so it has been removed and the remaining code
simplified.  In Evaluate/traverse.h, a return {}; was replaced with a
return of a default-constructed member.

Differential Revision: https://reviews.llvm.org/D81747

Added: 
    

Modified: 
    flang/documentation/ParserCombinators.md
    flang/include/flang/Evaluate/traverse.h
    flang/lib/Parser/Fortran-parsers.cpp
    flang/lib/Parser/basic-parsers.h
    flang/lib/Parser/program-parsers.cpp
    flang/lib/Parser/stmt-parser.h
    flang/lib/Parser/token-parsers.h

Removed: 
    


################################################################################
diff  --git a/flang/documentation/ParserCombinators.md b/flang/documentation/ParserCombinators.md
index b05d281590e9..984a4d043439 100644
--- a/flang/documentation/ParserCombinators.md
+++ b/flang/documentation/ParserCombinators.md
@@ -47,10 +47,10 @@ These objects and functions are (or return) the fundamental parsers:
 * `ok` is a trivial parser that always succeeds without advancing.
 * `pure(x)` returns a trivial parser that always succeeds without advancing,
   returning some value `x`.
+* `pure<T>()` is `pure(T{})` but does not require that T be copy-constructible.
 * `fail<T>(msg)` denotes a trivial parser that always fails, emitting the
   given message as a side effect.  The template parameter is the type of
   the value that the parser never returns.
-* `cut` is a trivial parser that always fails silently.
 * `nextCh` consumes the next character and returns its location,
   and fails at EOF.
 * `"xyz"_ch` succeeds if the next character consumed matches any of those
@@ -100,8 +100,10 @@ They are `constexpr`, so they should be viewed as type-safe macros.
 * `recovery(p, q)` is equivalent to `p || q`, except that error messages
   generated from the first parser are retained, and a flag is set in
   the ParseState to remember that error recovery was necessary.
-* `localRecovery(msg, p, q)` is equivalent to `recovery(withMessage(msg, p), defaulted(cut >> p) >> q)`.  It is useful for targeted error recovery situations
-  within statements.
+* `localRecovery(msg, p, q)` is equivalent to
+  `recovery(withMessage(msg, p), q >> pure<A>())` where `A` is the
+  result type of 'p'.
+  It is useful for targeted error recovery situations within statements.
 
 Note that
 ```

diff  --git a/flang/include/flang/Evaluate/traverse.h b/flang/include/flang/Evaluate/traverse.h
index 8d70c39513ef..4888946c1a6c 100644
--- a/flang/include/flang/Evaluate/traverse.h
+++ b/flang/include/flang/Evaluate/traverse.h
@@ -257,7 +257,7 @@ template <typename Visitor, typename Result> class Traverse {
 template <typename Visitor, bool DefaultValue,
     typename Base = Traverse<Visitor, bool>>
 struct AllTraverse : public Base {
-  AllTraverse(Visitor &v) : Base{v} {}
+  explicit AllTraverse(Visitor &v) : Base{v} {}
   using Base::operator();
   static bool Default() { return DefaultValue; }
   static bool Combine(bool x, bool y) { return x && y; }
@@ -268,10 +268,11 @@ struct AllTraverse : public Base {
 // and std::optional<>.
 template <typename Visitor, typename Result = bool,
     typename Base = Traverse<Visitor, Result>>
-struct AnyTraverse : public Base {
-  AnyTraverse(Visitor &v) : Base{v} {}
+class AnyTraverse : public Base {
+public:
+  explicit AnyTraverse(Visitor &v) : Base{v} {}
   using Base::operator();
-  static Result Default() { return {}; }
+  Result Default() const { return default_; }
   static Result Combine(Result &&x, Result &&y) {
     if (x) {
       return std::move(x);
@@ -279,12 +280,15 @@ struct AnyTraverse : public Base {
       return std::move(y);
     }
   }
+
+private:
+  Result default_{};
 };
 
 template <typename Visitor, typename Set,
     typename Base = Traverse<Visitor, Set>>
 struct SetTraverse : public Base {
-  SetTraverse(Visitor &v) : Base{v} {}
+  explicit SetTraverse(Visitor &v) : Base{v} {}
   using Base::operator();
   static Set Default() { return {}; }
   static Set Combine(Set &&x, Set &&y) {

diff  --git a/flang/lib/Parser/Fortran-parsers.cpp b/flang/lib/Parser/Fortran-parsers.cpp
index cdff9289faab..3192781d4bcc 100644
--- a/flang/lib/Parser/Fortran-parsers.cpp
+++ b/flang/lib/Parser/Fortran-parsers.cpp
@@ -1184,7 +1184,7 @@ TYPE_PARSER(extension<LanguageFeature::CrayPointer>(construct<BasedPointerStmt>(
 TYPE_PARSER(construct<StructureStmt>("STRUCTURE /" >> name / "/", pure(true),
                 optionalList(entityDecl)) ||
     construct<StructureStmt>(
-        "STRUCTURE" >> name, pure(false), defaulted(cut >> many(entityDecl))))
+        "STRUCTURE" >> name, pure(false), pure<std::list<EntityDecl>>()))
 
 TYPE_PARSER(construct<StructureField>(statement(StructureComponents{})) ||
     construct<StructureField>(indirect(Parser<Union>{})) ||

diff  --git a/flang/lib/Parser/basic-parsers.h b/flang/lib/Parser/basic-parsers.h
index b05a68da7922..6d88e5277fbe 100644
--- a/flang/lib/Parser/basic-parsers.h
+++ b/flang/lib/Parser/basic-parsers.h
@@ -65,7 +65,10 @@ template <typename A = Success> inline constexpr auto fail(MessageFixedText t) {
 }
 
 // pure(x) returns a parser that always succeeds, does not advance the
-// parse, and returns a captured value whose type must be copy-constructible.
+// parse, and returns a captured value x whose type must be copy-constructible.
+//
+// pure<A>() is essentially pure(A{}); it returns a default-constructed A{},
+// and works even when A is not copy-constructible.
 template <typename A> class PureParser {
 public:
   using resultType = A;
@@ -81,6 +84,18 @@ template <typename A> inline constexpr auto pure(A x) {
   return PureParser<A>(std::move(x));
 }
 
+template <typename A> class PureDefaultParser {
+public:
+  using resultType = A;
+  constexpr PureDefaultParser(const PureDefaultParser &) = default;
+  constexpr PureDefaultParser() {}
+  std::optional<A> Parse(ParseState &) const { return std::make_optional<A>(); }
+};
+
+template <typename A> inline constexpr auto pure() {
+  return PureDefaultParser<A>();
+}
+
 // If a is a parser, attempt(a) is the same parser, but on failure
 // the ParseState is guaranteed to have been restored to its initial value.
 template <typename A> class BacktrackingParser {
@@ -239,10 +254,10 @@ template <typename PA, typename PB> class SequenceParser {
 public:
   using resultType = typename PB::resultType;
   constexpr SequenceParser(const SequenceParser &) = default;
-  constexpr SequenceParser(PA pa, PB pb) : pa_{pa}, pb_{pb} {}
+  constexpr SequenceParser(PA pa, PB pb) : pa_{pa}, pb2_{pb} {}
   std::optional<resultType> Parse(ParseState &state) const {
     if (pa_.Parse(state)) {
-      return pb_.Parse(state);
+      return pb2_.Parse(state);
     } else {
       return std::nullopt;
     }
@@ -250,7 +265,7 @@ template <typename PA, typename PB> class SequenceParser {
 
 private:
   const PA pa_;
-  const PB pb_;
+  const PB pb2_;
 };
 
 template <typename PA, typename PB>
@@ -336,7 +351,7 @@ template <typename PA, typename PB> class RecoveryParser {
   using resultType = typename PA::resultType;
   static_assert(std::is_same_v<resultType, typename PB::resultType>);
   constexpr RecoveryParser(const RecoveryParser &) = default;
-  constexpr RecoveryParser(PA pa, PB pb) : pa_{pa}, pb_{pb} {}
+  constexpr RecoveryParser(PA pa, PB pb) : pa_{pa}, pb3_{pb} {}
   std::optional<resultType> Parse(ParseState &state) const {
     bool originallyDeferred{state.deferMessages()};
     ParseState backtrack{state};
@@ -364,7 +379,7 @@ template <typename PA, typename PB> class RecoveryParser {
     bool anyTokenMatched{state.anyTokenMatched()};
     state = std::move(backtrack);
     state.set_deferMessages(true);
-    std::optional<resultType> bx{pb_.Parse(state)};
+    std::optional<resultType> bx{pb3_.Parse(state)};
     state.messages() = std::move(messages);
     state.set_deferMessages(originallyDeferred);
     if (anyTokenMatched) {
@@ -383,7 +398,7 @@ template <typename PA, typename PB> class RecoveryParser {
 
 private:
   const PA pa_;
-  const PB pb_;
+  const PB pb3_;
 };
 
 template <typename PA, typename PB>
@@ -785,31 +800,18 @@ inline constexpr auto nonemptySeparated(PA p, PB sep) {
 // must discard its result in order to be compatible in type with other
 // parsers in an alternative, e.g. "x >> ok || y >> ok" is type-safe even
 // when x and y have distinct result types.
-//
-// cut is a parser that always fails.  It is useful when a parser must
-// have its type implicitly set; one use is the idiom "defaulted(cut >> x)",
-// which is essentially what "pure(T{})" would be able to do for x's
-// result type T, but without requiring that T have a default constructor
-// or a non-trivial destructor.  The state is preserved.
-template <bool pass> struct FixedParser {
+constexpr struct OkParser {
   using resultType = Success;
-  constexpr FixedParser() {}
+  constexpr OkParser() {}
   static constexpr std::optional<Success> Parse(ParseState &) {
-    if constexpr (pass) {
-      return Success{};
-    } else {
-      return std::nullopt;
-    }
+    return Success{};
   }
-};
-
-constexpr FixedParser<true> ok;
-constexpr FixedParser<false> cut;
+} ok;
 
 // A variant of recovery() above for convenience.
 template <typename PA, typename PB>
 inline constexpr auto localRecovery(MessageFixedText msg, PA pa, PB pb) {
-  return recovery(withMessage(msg, pa), pb >> defaulted(cut >> pa));
+  return recovery(withMessage(msg, pa), pb >> pure<typename PA::resultType>());
 }
 
 // nextCh is a parser that succeeds if the parsing state is not

diff  --git a/flang/lib/Parser/program-parsers.cpp b/flang/lib/Parser/program-parsers.cpp
index e394e25631d3..fc2c7c324eb6 100644
--- a/flang/lib/Parser/program-parsers.cpp
+++ b/flang/lib/Parser/program-parsers.cpp
@@ -29,7 +29,7 @@ namespace Fortran::parser {
 // and then skip over the end of the line here.
 TYPE_PARSER(construct<Program>(
     extension<LanguageFeature::EmptySourceFile>(skipStuffBeforeStatement >>
-        !nextCh >> defaulted(cut >> some(Parser<ProgramUnit>{}))) ||
+        !nextCh >> pure<std::list<ProgramUnit>>()) ||
     some(StartNewSubprogram{} >> Parser<ProgramUnit>{} / skipMany(";"_tok) /
             space / recovery(endOfLine, SkipPast<'\n'>{})) /
         skipStuffBeforeStatement))
@@ -509,8 +509,8 @@ TYPE_PARSER(
     construct<SubroutineStmt>(many(prefixSpec), "SUBROUTINE" >> name,
         parenthesized(optionalList(dummyArg)), maybe(languageBindingSpec)) ||
     construct<SubroutineStmt>(many(prefixSpec), "SUBROUTINE" >> name,
-        defaulted(cut >> many(dummyArg)),
-        defaulted(cut >> maybe(languageBindingSpec))))
+        pure<std::list<DummyArg>>(),
+        pure<std::optional<LanguageBindingSpec>>()))
 
 // R1536 dummy-arg -> dummy-arg-name | *
 TYPE_PARSER(construct<DummyArg>(name) || construct<DummyArg>(star))

diff  --git a/flang/lib/Parser/stmt-parser.h b/flang/lib/Parser/stmt-parser.h
index 058a3b6cc215..7dcc1f4620a9 100644
--- a/flang/lib/Parser/stmt-parser.h
+++ b/flang/lib/Parser/stmt-parser.h
@@ -83,7 +83,7 @@ constexpr auto executionPartErrorRecovery{stmtErrorRecoveryStart >>
     !("!$OMP "_sptok >> ("END"_tok || "SECTION"_id)) >> skipBadLine};
 
 // END statement error recovery
-constexpr auto missingOptionalName{defaulted(cut >> maybe(name))};
+constexpr auto missingOptionalName{pure<std::optional<Name>>()};
 constexpr auto noNameEnd{"END" >> missingOptionalName};
 constexpr auto atEndOfStmt{space >>
     withMessage("expected end of statement"_err_en_US, lookAhead(";\n"_ch))};

diff  --git a/flang/lib/Parser/token-parsers.h b/flang/lib/Parser/token-parsers.h
index fdb0a3a09899..fe43182e386f 100644
--- a/flang/lib/Parser/token-parsers.h
+++ b/flang/lib/Parser/token-parsers.h
@@ -584,13 +584,15 @@ template <char goal> struct SkipTo {
 //   [[, xyz] ::]     is  optionalBeforeColons(xyz)
 //   [[, xyz]... ::]  is  optionalBeforeColons(nonemptyList(xyz))
 template <typename PA> inline constexpr auto optionalBeforeColons(const PA &p) {
-  return "," >> construct<std::optional<typename PA::resultType>>(p) / "::" ||
-      ("::"_tok || !","_tok) >> defaulted(cut >> maybe(p));
+  using resultType = std::optional<typename PA::resultType>;
+  return "," >> construct<resultType>(p) / "::" ||
+      ("::"_tok || !","_tok) >> pure<resultType>();
 }
 template <typename PA>
 inline constexpr auto optionalListBeforeColons(const PA &p) {
+  using resultType = std::list<typename PA::resultType>;
   return "," >> nonemptyList(p) / "::" ||
-      ("::"_tok || !","_tok) >> defaulted(cut >> nonemptyList(p));
+      ("::"_tok || !","_tok) >> pure<resultType>();
 }
 
 // Skip over empty lines, leading spaces, and some compiler directives (viz.,


        


More information about the flang-commits mailing list