From a0226f9bffa4c879ec7f8183738205661978cc39 Mon Sep 17 00:00:00 2001 From: peter klausler Date: Fri, 12 Jun 2020 10:05:04 -0700 Subject: [PATCH] [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() 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 --- flang/documentation/ParserCombinators.md | 8 +++-- flang/include/flang/Evaluate/traverse.h | 14 ++++++--- flang/lib/Parser/Fortran-parsers.cpp | 2 +- flang/lib/Parser/basic-parsers.h | 52 +++++++++++++++++--------------- flang/lib/Parser/program-parsers.cpp | 6 ++-- flang/lib/Parser/stmt-parser.h | 2 +- flang/lib/Parser/token-parsers.h | 8 +++-- 7 files changed, 51 insertions(+), 41 deletions(-) diff --git a/flang/documentation/ParserCombinators.md b/flang/documentation/ParserCombinators.md index b05d281..984a4d0 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()` is `pure(T{})` but does not require that T be copy-constructible. * `fail(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())` 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 8d70c39..4888946 100644 --- a/flang/include/flang/Evaluate/traverse.h +++ b/flang/include/flang/Evaluate/traverse.h @@ -257,7 +257,7 @@ private: template > 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 > -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 > 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 cdff928..3192781 100644 --- a/flang/lib/Parser/Fortran-parsers.cpp +++ b/flang/lib/Parser/Fortran-parsers.cpp @@ -1184,7 +1184,7 @@ TYPE_PARSER(extension(construct( TYPE_PARSER(construct("STRUCTURE /" >> name / "/", pure(true), optionalList(entityDecl)) || construct( - "STRUCTURE" >> name, pure(false), defaulted(cut >> many(entityDecl)))) + "STRUCTURE" >> name, pure(false), pure>())) TYPE_PARSER(construct(statement(StructureComponents{})) || construct(indirect(Parser{})) || diff --git a/flang/lib/Parser/basic-parsers.h b/flang/lib/Parser/basic-parsers.h index b05a68d..6d88e52 100644 --- a/flang/lib/Parser/basic-parsers.h +++ b/flang/lib/Parser/basic-parsers.h @@ -65,7 +65,10 @@ template 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() is essentially pure(A{}); it returns a default-constructed A{}, +// and works even when A is not copy-constructible. template class PureParser { public: using resultType = A; @@ -81,6 +84,18 @@ template inline constexpr auto pure(A x) { return PureParser(std::move(x)); } +template class PureDefaultParser { +public: + using resultType = A; + constexpr PureDefaultParser(const PureDefaultParser &) = default; + constexpr PureDefaultParser() {} + std::optional Parse(ParseState &) const { return std::make_optional(); } +}; + +template inline constexpr auto pure() { + return PureDefaultParser(); +} + // 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 class BacktrackingParser { @@ -239,10 +254,10 @@ template 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 Parse(ParseState &state) const { if (pa_.Parse(state)) { - return pb_.Parse(state); + return pb2_.Parse(state); } else { return std::nullopt; } @@ -250,7 +265,7 @@ public: private: const PA pa_; - const PB pb_; + const PB pb2_; }; template @@ -336,7 +351,7 @@ public: using resultType = typename PA::resultType; static_assert(std::is_same_v); 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 Parse(ParseState &state) const { bool originallyDeferred{state.deferMessages()}; ParseState backtrack{state}; @@ -364,7 +379,7 @@ public: bool anyTokenMatched{state.anyTokenMatched()}; state = std::move(backtrack); state.set_deferMessages(true); - std::optional bx{pb_.Parse(state)}; + std::optional bx{pb3_.Parse(state)}; state.messages() = std::move(messages); state.set_deferMessages(originallyDeferred); if (anyTokenMatched) { @@ -383,7 +398,7 @@ public: private: const PA pa_; - const PB pb_; + const PB pb3_; }; template @@ -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 struct FixedParser { +constexpr struct OkParser { using resultType = Success; - constexpr FixedParser() {} + constexpr OkParser() {} static constexpr std::optional Parse(ParseState &) { - if constexpr (pass) { - return Success{}; - } else { - return std::nullopt; - } + return Success{}; } -}; - -constexpr FixedParser ok; -constexpr FixedParser cut; +} ok; // A variant of recovery() above for convenience. template 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()); } // 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 e394e25..fc2c7c3 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( extension(skipStuffBeforeStatement >> - !nextCh >> defaulted(cut >> some(Parser{}))) || + !nextCh >> pure>()) || some(StartNewSubprogram{} >> Parser{} / skipMany(";"_tok) / space / recovery(endOfLine, SkipPast<'\n'>{})) / skipStuffBeforeStatement)) @@ -509,8 +509,8 @@ TYPE_PARSER( construct(many(prefixSpec), "SUBROUTINE" >> name, parenthesized(optionalList(dummyArg)), maybe(languageBindingSpec)) || construct(many(prefixSpec), "SUBROUTINE" >> name, - defaulted(cut >> many(dummyArg)), - defaulted(cut >> maybe(languageBindingSpec)))) + pure>(), + pure>())) // R1536 dummy-arg -> dummy-arg-name | * TYPE_PARSER(construct(name) || construct(star)) diff --git a/flang/lib/Parser/stmt-parser.h b/flang/lib/Parser/stmt-parser.h index 058a3b6..7dcc1f4 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>()}; 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 fdb0a3a..fe43182 100644 --- a/flang/lib/Parser/token-parsers.h +++ b/flang/lib/Parser/token-parsers.h @@ -584,13 +584,15 @@ template struct SkipTo { // [[, xyz] ::] is optionalBeforeColons(xyz) // [[, xyz]... ::] is optionalBeforeColons(nonemptyList(xyz)) template inline constexpr auto optionalBeforeColons(const PA &p) { - return "," >> construct>(p) / "::" || - ("::"_tok || !","_tok) >> defaulted(cut >> maybe(p)); + using resultType = std::optional; + return "," >> construct(p) / "::" || + ("::"_tok || !","_tok) >> pure(); } template inline constexpr auto optionalListBeforeColons(const PA &p) { + using resultType = std::list; return "," >> nonemptyList(p) / "::" || - ("::"_tok || !","_tok) >> defaulted(cut >> nonemptyList(p)); + ("::"_tok || !","_tok) >> pure(); } // Skip over empty lines, leading spaces, and some compiler directives (viz., -- 2.7.4