diff --git a/flang/docs/Extensions.md b/flang/docs/Extensions.md index 054e38c623bca..751706d00965e 100644 --- a/flang/docs/Extensions.md +++ b/flang/docs/Extensions.md @@ -545,7 +545,9 @@ end program (but not Hollerith) [-fbackslash], including Unicode escapes with `\U`. * Logical abbreviations `.T.`, `.F.`, `.N.`, `.A.`, `.O.`, and `.X.` - [-flogical-abbreviations] + [-flogical-abbreviations]; when the option is not enabled and a parse + error occurs on a source line bearing one of these abbreviations, the + error output includes a note suggesting the option. * `.XOR.` as a synonym for `.NEQV.` [-fxor-operator] * The default `INTEGER` type is required by the standard to occupy the same amount of storage as the default `REAL` type. Default diff --git a/flang/include/flang/Parser/message.h b/flang/include/flang/Parser/message.h index c70c335133ba8..4c87f58e163a5 100644 --- a/flang/include/flang/Parser/message.h +++ b/flang/include/flang/Parser/message.h @@ -292,6 +292,15 @@ class Message : public common::ReferenceCounted { return Attach(new Message{std::forward(args)...}); // reference-counted } + // During parsing (before ResolveProvenances) a message locates itself with a + // CharBlock into the cooked source; returns it when present. + std::optional GetCookedSourceLocation() const { + if (const CharBlock *cb{std::get_if(&location_)}) { + return *cb; + } + return std::nullopt; + } + bool SortBefore(const Message &that) const; bool IsFatal() const; Severity severity() const; diff --git a/flang/include/flang/Parser/parsing.h b/flang/include/flang/Parser/parsing.h index 3365628dc4e0c..c48531c93f9d1 100644 --- a/flang/include/flang/Parser/parsing.h +++ b/flang/include/flang/Parser/parsing.h @@ -21,6 +21,8 @@ namespace Fortran::parser { +class UserState; + class Parsing { public: explicit Parsing(AllCookedSources &); @@ -55,6 +57,11 @@ class Parsing { } private: + // Adds a note suggesting -flogical-abbreviations when the parse failed on a + // source line bearing a logical abbreviation (.T./.F./.N./.A./.O.) that the + // disabled LogicalAbbreviations feature would otherwise accept. + void SuggestLogicalAbbreviations(const UserState &, Messages &); + Options options_; AllCookedSources &allCooked_; CookedSource *currentCooked_{nullptr}; diff --git a/flang/include/flang/Parser/user-state.h b/flang/include/flang/Parser/user-state.h index 129f9fb8fee05..6b8bc5d7af7e9 100644 --- a/flang/include/flang/Parser/user-state.h +++ b/flang/include/flang/Parser/user-state.h @@ -20,6 +20,7 @@ #include "flang/Support/Fortran-features.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include #include @@ -89,6 +90,28 @@ class UserState { return oldStructureComponents_.find(name) != oldStructureComponents_.end(); } + // When the LogicalAbbreviations feature is disabled, the parser records the + // source location of any logical abbreviation (.T./.F./.N./.A./.O.) it + // encounters, so that a parse failure on that same source line can suggest + // enabling the -flogical-abbreviations option. + // CharBlock's own relational operators compare contents, not positions, so + // the set is ordered by position in the cooked character stream: distinct + // occurrences of the same spelling stay distinct, while backtracking + // re-attempts at the same position collapse into one entry. + struct CharBlockByPosition { + bool operator()(const CharBlock &x, const CharBlock &y) const { + return std::less{}(x.begin(), y.begin()); + } + }; + void NoteDisabledLogicalAbbreviation(CharBlock at) { + disabledLogicalAbbreviations_.insert(at); + } + // Recorded in increasing source order. + const std::set & + disabledLogicalAbbreviations() const { + return disabledLogicalAbbreviations_; + } + private: const AllCookedSources &allCooked_; @@ -101,6 +124,7 @@ class UserState { int nonlabelDoConstructNestingDepth_{0}; std::set oldStructureComponents_; + std::set disabledLogicalAbbreviations_; common::LanguageFeatureControl features_; }; diff --git a/flang/lib/Parser/basic-parsers.h b/flang/lib/Parser/basic-parsers.h index eeb59a830fc0c..86e0d6e726220 100644 --- a/flang/lib/Parser/basic-parsers.h +++ b/flang/lib/Parser/basic-parsers.h @@ -848,6 +848,37 @@ template class NonstandardParser { std::optional Parse(ParseState &state) const { if (UserState * ustate{state.userState()}) { if (!ustate->features().IsEnabled(LF)) { + if constexpr (LF == LanguageFeature::LogicalAbbreviations) { + // The feature is disabled, but if the source actually spells a + // logical abbreviation here, remember its location so that a later + // parse failure at this spot can suggest -flogical-abbreviations. + // Every such abbreviation begins with '.', so only attempt the + // speculative parse (which copies the parse state) when the next + // non-blank character could start one. The forked parse state + // shares this state's UserState pointer, so the speculatively run + // inner parser must have no side effects on it (the wrapped + // abbreviation parsers are pure token matchers). + const char *p{state.GetLocation()}; + const char *limit{p + state.BytesRemaining()}; + while (p < limit && *p == ' ') { + ++p; + } + if (p < limit && *p == '.') { + ParseState fork{state}; + if (parser_.Parse(fork)) { + // Anchor the recorded span at the first non-blank character so + // that re-attempts at the same occurrence from different + // productions record the same position, and trim any trailing + // blanks the token parser consumed so the span covers exactly + // the abbreviation. + const char *end{std::max(fork.GetLocation(), p + 1)}; + while (end > p + 1 && end[-1] == ' ') { + --end; + } + ustate->NoteDisabledLogicalAbbreviation(CharBlock{p, end}); + } + } + } return std::nullopt; } } diff --git a/flang/lib/Parser/parsing.cpp b/flang/lib/Parser/parsing.cpp index 667d8d9297ecb..99e504185b24d 100644 --- a/flang/lib/Parser/parsing.cpp +++ b/flang/lib/Parser/parsing.cpp @@ -13,7 +13,10 @@ #include "flang/Parser/preprocessor.h" #include "flang/Parser/provenance.h" #include "flang/Parser/source.h" +#include "flang/Parser/user-state.h" #include "llvm/Support/raw_ostream.h" +#include +#include namespace Fortran::parser { @@ -292,8 +295,52 @@ void Parsing::Parse(llvm::raw_ostream &out) { CHECK( !parseState.anyErrorRecovery() || parseState.messages().AnyFatalError()); consumedWholeFile_ = parseState.IsAtEnd(); - messages_.Annex(std::move(parseState.messages())); finalRestingPlace_ = parseState.GetLocation(); + SuggestLogicalAbbreviations(userState, parseState.messages()); + messages_.Annex(std::move(parseState.messages())); +} + +// When the LogicalAbbreviations feature is disabled, the parser records the +// location of every logical abbreviation (.T./.F./.N./.A./.O.) it sees. For +// each such occurrence that lies on a source line where the parse failed, +// suggest the -flogical-abbreviations option. Tying each suggestion to +// a failing source line keeps it from appearing for an occurrence that parses +// successfully as a defined operator (those fail later in semantics, not here) +// or on a line that failed for an unrelated reason. Line granularity is a +// heuristic, though: a valid defined-operator use that happens to share its +// source line with an unrelated parse error still receives the suggestion. +void Parsing::SuggestLogicalAbbreviations( + const UserState &userState, Messages &messages) { + const std::set &abbreviations{ + userState.disabledLogicalAbbreviations()}; + if (abbreviations.empty()) { + return; + } + std::set> errorLines; + for (const Message &message : messages.messages()) { + if (!message.IsFatal()) { + continue; + } + if (std::optional errorLoc{message.GetCookedSourceLocation()}) { + if (auto pos{allCooked_.GetSourcePositionRange(*errorLoc)}) { + errorLines.emplace(&*pos->first.sourceFile, pos->first.line); + } + } + } + if (errorLines.empty()) { + return; + } + // The set is keyed by source position, so iterating it visits each + // occurrence once, in increasing source order: a note is emitted for every + // .T./.F. usage on a failing line, even when spellings repeat. + for (const CharBlock &abbreviation : abbreviations) { + if (auto pos{allCooked_.GetSourcePositionRange(abbreviation)}) { + if (errorLines.count({&*pos->first.sourceFile, pos->first.line}) > 0) { + messages.Say(abbreviation, + "This nonstandard logical abbreviation requires the '-flogical-abbreviations' option"_en_US); + } + } + } } void Parsing::ClearLog() { log_.clear(); } diff --git a/flang/test/Parser/logical-abbreviation-defined-operator-mixed.f90 b/flang/test/Parser/logical-abbreviation-defined-operator-mixed.f90 new file mode 100644 index 0000000000000..97cd9b05b5baf --- /dev/null +++ b/flang/test/Parser/logical-abbreviation-defined-operator-mixed.f90 @@ -0,0 +1,26 @@ +! A valid defined-operator use of .f. must not swallow the suggestion for a +! later parse failure on the same spelling: the suggestion is emitted at the +! failing occurrence only, not at the valid one. + +! RUN: not %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck %s --implicit-check-not='This nonstandard logical abbreviation' + +module m + interface operator(.f.) + module procedure neg + end interface +contains + pure integer function neg(a) + integer, intent(in) :: a + neg = -a + end function +end module +program p + use m + integer :: r + logical :: z + r = .f.(4) + z = .f. +end program + +! CHECK: This nonstandard logical abbreviation requires the '-flogical-abbreviations' option +! CHECK-NEXT: z = .f. diff --git a/flang/test/Parser/logical-abbreviation-defined-operator.f90 b/flang/test/Parser/logical-abbreviation-defined-operator.f90 new file mode 100644 index 0000000000000..ba3a59100910b --- /dev/null +++ b/flang/test/Parser/logical-abbreviation-defined-operator.f90 @@ -0,0 +1,23 @@ +! A program that legitimately uses .f. as a defined unary operator parses and +! compiles cleanly with the default (LogicalAbbreviations disabled). The +! abbreviation spelling is recorded by the parser, but because the parse +! succeeds no -flogical-abbreviations suggestion is emitted. + +! RUN: %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck %s --allow-empty --implicit-check-not='-flogical-abbreviations' + +module m + interface operator(.f.) + module procedure neg + end interface +contains + pure integer function neg(a) + integer, intent(in) :: a + neg = -a + end function +end module +program p + use m + integer :: r + r = .f. 4 + print *, r +end program diff --git a/flang/test/Parser/logical-abbreviation-no-suggestion.f90 b/flang/test/Parser/logical-abbreviation-no-suggestion.f90 new file mode 100644 index 0000000000000..dec7e78ed9aea --- /dev/null +++ b/flang/test/Parser/logical-abbreviation-no-suggestion.f90 @@ -0,0 +1,25 @@ +! Mitigation test: .f. is used here as a legitimate defined unary operator on +! one line, while an unrelated parse error occurs on a different line. Because +! the failure is not on the line bearing the abbreviation, the +! -flogical-abbreviations suggestion must NOT be emitted. + +! RUN: not %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck %s --implicit-check-not='-flogical-abbreviations' + +module m + interface operator(.f.) + module procedure neg + end interface +contains + pure integer function neg(a) + integer, intent(in) :: a + neg = -a + end function +end module +program p + use m + integer :: r + r = .f. 4 + this is not valid +end program + +! CHECK: error: diff --git a/flang/test/Parser/logical-abbreviation-same-line-error.f90 b/flang/test/Parser/logical-abbreviation-same-line-error.f90 new file mode 100644 index 0000000000000..2b91e9ec354c6 --- /dev/null +++ b/flang/test/Parser/logical-abbreviation-same-line-error.f90 @@ -0,0 +1,26 @@ +! Known limitation of the line-granularity heuristic: a valid defined-operator +! use of .f. that shares its source line with an unrelated parse error still +! receives the -flogical-abbreviations suggestion, because suggestions are tied +! to failing source lines rather than to the failing token itself. + +! RUN: not %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck %s + +module m + interface operator(.f.) + module procedure neg + end interface +contains + pure integer function neg(a) + integer, intent(in) :: a + neg = -a + end function +end module +program p + use m + integer :: r + r = .f. 4 garbage here +end program + +! CHECK: error: Could not parse +! CHECK: This nonstandard logical abbreviation requires the '-flogical-abbreviations' option +! CHECK-NEXT: r = .f. 4 garbage here diff --git a/flang/test/Parser/logical-abbreviation-suggestion-duplicate.f90 b/flang/test/Parser/logical-abbreviation-suggestion-duplicate.f90 new file mode 100644 index 0000000000000..9b1938fde3d55 --- /dev/null +++ b/flang/test/Parser/logical-abbreviation-suggestion-duplicate.f90 @@ -0,0 +1,15 @@ +! Each failing occurrence of a logical abbreviation gets its own suggestion, +! even when the spellings are identical: the recorded occurrences are keyed by +! source position, not by spelled text. + +! RUN: not %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck %s --implicit-check-not='This nonstandard logical abbreviation' + +logical :: x, y +x = .F. +y = .F. +end + +! CHECK: This nonstandard logical abbreviation requires the '-flogical-abbreviations' option +! CHECK-NEXT: x = .F. +! CHECK: This nonstandard logical abbreviation requires the '-flogical-abbreviations' option +! CHECK-NEXT: y = .F. diff --git a/flang/test/Parser/logical-abbreviation-suggestion-multiple.f90 b/flang/test/Parser/logical-abbreviation-suggestion-multiple.f90 new file mode 100644 index 0000000000000..83e6d1174516f --- /dev/null +++ b/flang/test/Parser/logical-abbreviation-suggestion-multiple.f90 @@ -0,0 +1,15 @@ +! Every distinct .T./.F. abbreviation that causes a parse failure gets its own +! suggestion, not just the first one. The operator form .A. parses as a defined +! operator and fails later in semantics rather than in the parser, so it gets no +! parse-time suggestion. + +! RUN: not %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck %s + +logical :: x +x = .T. +x = .F. +x = x .A. x +end + +! CHECK-COUNT-2: This nonstandard logical abbreviation requires the '-flogical-abbreviations' option +! CHECK-NOT: This nonstandard logical abbreviation diff --git a/flang/test/Parser/logical-abbreviation-suggestion-true.f90 b/flang/test/Parser/logical-abbreviation-suggestion-true.f90 new file mode 100644 index 0000000000000..59b2d2de1471d --- /dev/null +++ b/flang/test/Parser/logical-abbreviation-suggestion-true.f90 @@ -0,0 +1,13 @@ +! The .T. spelling of .TRUE. is handled like .F.: the usual parse error plus a +! suggestion to enable -flogical-abbreviations. + +! RUN: not %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck %s +! RUN: %flang_fc1 -fsyntax-only -flogical-abbreviations %s + +program p + logical :: x + x = .T. +end program + +! CHECK: error: expected '(' +! CHECK: This nonstandard logical abbreviation requires the '-flogical-abbreviations' option diff --git a/flang/test/Parser/logical-abbreviation-suggestion.f b/flang/test/Parser/logical-abbreviation-suggestion.f new file mode 100644 index 0000000000000..ef08d744109c5 --- /dev/null +++ b/flang/test/Parser/logical-abbreviation-suggestion.f @@ -0,0 +1,13 @@ +! Fixed-form counterpart: the .F. abbreviation in an assignment is first +! misparsed as a statement function (so the error points at the '='), but the +! suggestion is still anchored at the abbreviation and offered to the user. + +! RUN: not %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck %s +! RUN: %flang_fc1 -fsyntax-only -flogical-abbreviations %s + + logical flag + flag = .F. + end + +! CHECK: error: expected '(' +! CHECK: This nonstandard logical abbreviation requires the '-flogical-abbreviations' option diff --git a/flang/test/Parser/logical-abbreviation-suggestion.f90 b/flang/test/Parser/logical-abbreviation-suggestion.f90 new file mode 100644 index 0000000000000..ad59243fc5237 --- /dev/null +++ b/flang/test/Parser/logical-abbreviation-suggestion.f90 @@ -0,0 +1,13 @@ +! Verify that a parse failure caused by the disabled .F. logical abbreviation +! still reports the usual errors and additionally suggests the +! -flogical-abbreviations option, and that enabling the option compiles cleanly. + +! RUN: not %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck %s +! RUN: %flang_fc1 -fsyntax-only -flogical-abbreviations %s + +logical :: x +x = .F. +end + +! CHECK: error: expected '(' +! CHECK: This nonstandard logical abbreviation requires the '-flogical-abbreviations' option