[flang] Disable Fortran free form line continuation in non-source lin… #94663
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-parser Author: Peter Klausler (klausler) Changes…e produced by keyword macro replacement When later initial keyword macro replacement will yield a line that is not Fortran source, don't interpret "&" as a Fortran source line continuation marker during tokenization of the line. Fixes #82579. Full diff: https://github.com/llvm/llvm-project/pull/94663.diff 5 Files Affected:
diff --git a/flang/include/flang/Parser/token-sequence.h b/flang/include/flang/Parser/token-sequence.h
index 849240d8ec62c..ee5f71edd03c8 100644
--- a/flang/include/flang/Parser/token-sequence.h
+++ b/flang/include/flang/Parser/token-sequence.h
@@ -124,7 +124,7 @@ class TokenSequence {
TokenSequence &RemoveRedundantBlanks(std::size_t firstChar = 0);
TokenSequence &ClipComment(const Prescanner &, bool skipFirst = false);
const TokenSequence &CheckBadFortranCharacters(
- Messages &, const Prescanner &) const;
+ Messages &, const Prescanner &, bool allowAmpersand) const;
const TokenSequence &CheckBadParentheses(Messages &) const;
void Emit(CookedSource &) const;
llvm::raw_ostream &Dump(llvm::raw_ostream &) const;
diff --git a/flang/lib/Parser/prescan.cpp b/flang/lib/Parser/prescan.cpp
index f9b9c3d2c6e56..cfc14b8f61854 100644
--- a/flang/lib/Parser/prescan.cpp
+++ b/flang/lib/Parser/prescan.cpp
@@ -180,6 +180,24 @@ void Prescanner::Statement() {
}
} else {
SkipSpaces();
+ // Check for a leading identifier that might be a keyword macro
+ // that will expand to anything indicating a non-source line, like
+ // a comment marker or directive sentinel. If so, disable line
+ // continuation, so that NextToken() won't consume anything from
+ // following lines.
+ if (IsLegalIdentifierStart(*at_)) {
+ CHECK(NextToken(tokens));
+ CHECK(tokens.SizeInTokens() == 1);
+ CharBlock id{tokens.TokenAt(0)};
+ if (preprocessor_.IsNameDefined(id) &&
+ !preprocessor_.IsFunctionLikeDefinition(id)) {
+ if (auto replaced{preprocessor_.MacroReplacement(tokens, *this)}) {
+ disableSourceContinuation_ =
+ ClassifyLine(*replaced, GetCurrentProvenance()).kind !=
+ LineClassification::Kind::Source;
+ }
+ }
+ }
}
break;
}
@@ -197,17 +215,13 @@ void Prescanner::Statement() {
Provenance newlineProvenance{GetCurrentProvenance()};
if (std::optional<TokenSequence> preprocessed{
preprocessor_.MacroReplacement(tokens, *this)}) {
- // Reprocess the preprocessed line. Append a newline temporarily.
- preprocessed->PutNextTokenChar('\n', newlineProvenance);
- preprocessed->CloseToken();
- const char *ppd{preprocessed->ToCharBlock().begin()};
- LineClassification ppl{ClassifyLine(ppd)};
- preprocessed->pop_back(); // remove the newline
+ // Reprocess the preprocessed line.
+ LineClassification ppl{ClassifyLine(*preprocessed, newlineProvenance)};
switch (ppl.kind) {
case LineClassification::Kind::Comment:
break;
case LineClassification::Kind::IncludeLine:
- FortranInclude(ppd + ppl.payloadOffset);
+ FortranInclude(preprocessed->TokenAt(0).begin() + ppl.payloadOffset);
break;
case LineClassification::Kind::ConditionalCompilationDirective:
case LineClassification::Kind::IncludeDirective:
@@ -270,7 +284,8 @@ void Prescanner::Statement() {
void Prescanner::CheckAndEmitLine(
TokenSequence &tokens, Provenance newlineProvenance) {
- tokens.CheckBadFortranCharacters(messages_, *this);
+ tokens.CheckBadFortranCharacters(
+ messages_, *this, disableSourceContinuation_);
// Parenthesis nesting check does not apply while any #include is
// active, nor on the lines before and after a top-level #include.
// Applications play shenanigans with line continuation before and
@@ -1243,7 +1258,9 @@ bool Prescanner::IsImplicitContinuation() const {
}
bool Prescanner::Continuation(bool mightNeedFixedFormSpace) {
- if (*at_ == '\n' || *at_ == '&') {
+ if (disableSourceContinuation_) {
+ return false;
+ } else if (*at_ == '\n' || *at_ == '&') {
if (inFixedForm_) {
return FixedFormContinuation(mightNeedFixedFormSpace);
} else {
@@ -1255,8 +1272,9 @@ bool Prescanner::Continuation(bool mightNeedFixedFormSpace) {
BeginSourceLine(nextLine_);
NextLine();
return true;
+ } else {
+ return false;
}
- return false;
}
std::optional<Prescanner::LineClassification>
@@ -1418,6 +1436,17 @@ Prescanner::LineClassification Prescanner::ClassifyLine(
return {LineClassification::Kind::Source};
}
+Prescanner::LineClassification Prescanner::ClassifyLine(
+ TokenSequence &tokens, Provenance newlineProvenance) const {
+ // Append a newline temporarily.
+ tokens.PutNextTokenChar('\n', newlineProvenance);
+ tokens.CloseToken();
+ const char *ppd{tokens.ToCharBlock().begin()};
+ LineClassification classification{ClassifyLine(ppd)};
+ tokens.pop_back(); // remove the newline
+ return classification;
+}
+
void Prescanner::SourceFormChange(std::string &&dir) {
if (dir == "!dir$ free") {
inFixedForm_ = false;
diff --git a/flang/lib/Parser/prescan.h b/flang/lib/Parser/prescan.h
index 491b1fe0a7517..1b414afcd2419 100644
--- a/flang/lib/Parser/prescan.h
+++ b/flang/lib/Parser/prescan.h
@@ -117,6 +117,7 @@ class Prescanner {
parenthesisNesting_ = 0;
continuationLines_ = 0;
isPossibleMacroCall_ = false;
+ disableSourceContinuation_ = false;
}
Provenance GetProvenance(const char *sourceChar) const {
@@ -192,6 +193,8 @@ class Prescanner {
std::optional<LineClassification> IsFreeFormCompilerDirectiveLine(
const char *) const;
LineClassification ClassifyLine(const char *) const;
+ LineClassification ClassifyLine(
+ TokenSequence &, Provenance newlineProvenance) const;
void SourceFormChange(std::string &&);
bool CompilerDirectiveContinuation(TokenSequence &, const char *sentinel);
bool SourceLineContinuation(TokenSequence &);
@@ -211,6 +214,7 @@ class Prescanner {
int continuationLines_{0};
bool isPossibleMacroCall_{false};
bool afterIncludeDirective_{false};
+ bool disableSourceContinuation_{false};
Provenance startProvenance_;
const char *start_{nullptr}; // beginning of current source file content
diff --git a/flang/lib/Parser/token-sequence.cpp b/flang/lib/Parser/token-sequence.cpp
index d0254ecd5aaef..40560bbacb54f 100644
--- a/flang/lib/Parser/token-sequence.cpp
+++ b/flang/lib/Parser/token-sequence.cpp
@@ -347,7 +347,8 @@ ProvenanceRange TokenSequence::GetProvenanceRange() const {
}
const TokenSequence &TokenSequence::CheckBadFortranCharacters(
- Messages &messages, const Prescanner &prescanner) const {
+ Messages &messages, const Prescanner &prescanner,
+ bool allowAmpersand) const {
std::size_t tokens{SizeInTokens()};
for (std::size_t j{0}; j < tokens; ++j) {
CharBlock token{TokenAt(j)};
@@ -362,6 +363,8 @@ const TokenSequence &TokenSequence::CheckBadFortranCharacters(
++j;
continue;
}
+ } else if (ch == '&' && allowAmpersand) {
+ continue;
}
if (ch < ' ' || ch >= '\x7f') {
messages.Say(GetTokenProvenanceRange(j),
diff --git a/flang/test/Preprocessing/directive-contin-with-pp.F90 b/flang/test/Preprocessing/directive-contin-with-pp.F90
index 9a06ae8438210..7e30b03c349d1 100644
--- a/flang/test/Preprocessing/directive-contin-with-pp.F90
+++ b/flang/test/Preprocessing/directive-contin-with-pp.F90
@@ -5,6 +5,7 @@
#define FIRST(x) DIR_START x
#define NEXT(x) DIR_CONT x
#define AMPER &
+#define COMMENT !
subroutine s(x1, x2, x3, x4, x5, x6, x7)
@@ -28,6 +29,14 @@ subroutine s(x1, x2, x3, x4, x5, x6, x7)
NEXT(x7 &)
NEXT(x8)
+COMMENT blah &
+COMMENT & more
+stop 1
+
+DIR_START blah &
+DIR_START & more
+stop 2
+
end
!CHECK: subroutine s(x1, x2, x3, x4, x5, x6, x7)
@@ -38,4 +47,6 @@ subroutine s(x1, x2, x3, x4, x5, x6, x7)
!CHECK: !dir$ ignore_tkr x5
!CHECK: !dir$ ignore_tkr x6
!CHECK: !dir$ ignore_tkr x7 x8
+!CHECK: stop 1
+!CHECK: stop 2
!CHECK: end
|
|
I'm testing this patch
However, I'm hitting a very strange issue. When I do The error went crazy. actually there is a line error: Could not parse macro_omp.f90 However, if I extract preprocessed file and then I also noticed the compiler seems behavior non-deterministicly when running |
…e produced by keyword macro replacement When later initial keyword macro replacement will yield a line that is not Fortran source, don't interpret "&" as a Fortran source line continuation marker during tokenization of the line. Fixes llvm#82579.
|
Patch has been updated. |
ye-luo
approved these changes
Jun 11, 2024
Does that mean that you still have other bugs? |
|
@ye-luo ping -- do you have other related bugs that should be fixed before I merge this? |
Everything looks good on my side from minimal reproducers to full applications. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…e produced by keyword macro replacement
When later initial keyword macro replacement will yield a line that is not Fortran source, don't interpret "&" as a Fortran source line continuation marker during tokenization of the line.
Fixes #82579.