-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[flang] Don't create bogus tokens from token pasting (##) #147596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-flang-parser Author: Peter Klausler (klausler) ChangesWhen blank tokens arise from macro replacement in token sequences with token pasting (##), the preprocessor is producing some bogus tokens (e.g., "name(") that can lead to subtle bugs later when macro names are not recognized as such. The fix is to not paste tokens together when the result would not be a valid Fortran or C token in the preprocessing context. Full diff: https://github.com/llvm/llvm-project/pull/147596.diff 2 Files Affected:
diff --git a/flang/lib/Parser/preprocessor.cpp b/flang/lib/Parser/preprocessor.cpp
index 8ef9810463d5a..9f711a9001057 100644
--- a/flang/lib/Parser/preprocessor.cpp
+++ b/flang/lib/Parser/preprocessor.cpp
@@ -156,23 +156,50 @@ static TokenSequence TokenPasting(TokenSequence &&text) {
}
TokenSequence result;
std::size_t tokens{text.SizeInTokens()};
- bool pasting{false};
+ std::optional<CharBlock> before; // last non-blank token before ##
for (std::size_t j{0}; j < tokens; ++j) {
- if (IsTokenPasting(text.TokenAt(j))) {
- if (!pasting) {
+ CharBlock after{text.TokenAt(j)};
+ if (!before) {
+ if (IsTokenPasting(after)) {
while (!result.empty() &&
result.TokenAt(result.SizeInTokens() - 1).IsBlank()) {
result.pop_back();
}
if (!result.empty()) {
- result.ReopenLastToken();
- pasting = true;
+ before = result.TokenAt(result.SizeInTokens() - 1);
}
+ } else {
+ result.AppendRange(text, j, 1);
+ }
+ } else if (after.IsBlank() || IsTokenPasting(after)) {
+ // drop it
+ } else { // pasting before ## after
+ bool doPaste{false};
+ char last{before->end()[-1]};
+ char first{after.begin()[0]};
+ // Apply basic sanity checking to pasting so avoid constructing a bogus
+ // token that might cause macro replacement to fail, like "macro(".
+ if (IsLegalInIdentifier(last) && IsLegalInIdentifier(first)) {
+ doPaste = true;
+ } else if (IsDecimalDigit(first) &&
+ (last == '.' || last == '+' || last == '-')) {
+ doPaste = true; // 1. ## 0, - ## 1
+ } else if (before->size() == 1 && after.size() == 1) {
+ if (first == last &&
+ (last == '<' || last == '>' || last == '*' || last == '/' ||
+ last == '=' || last == '&' || last == '|' || last == ':')) {
+ // Fortran **, //, ==, ::
+ // C <<, >>, &&, || for use in #if expressions
+ doPaste = true;
+ } else if (first == '=' && (last == '!' || last == '/')) {
+ doPaste = true; // != and /=
+ }
+ }
+ if (doPaste) {
+ result.ReopenLastToken();
}
- } else if (pasting && text.TokenAt(j).IsBlank()) {
- } else {
result.AppendRange(text, j, 1);
- pasting = false;
+ before.reset();
}
}
return result;
diff --git a/flang/test/Preprocessing/bug1126.F90 b/flang/test/Preprocessing/bug1126.F90
new file mode 100644
index 0000000000000..ae5bb5633581d
--- /dev/null
+++ b/flang/test/Preprocessing/bug1126.F90
@@ -0,0 +1,20 @@
+! RUN: %flang -E %s 2>&1 | FileCheck %s
+#define STRINGIFY(x) #x
+#define TOSTRING(x) STRINGIFY(x)
+#define PREFIX(x) prefix ## x
+#define NAME(x) PREFIX(foo ## x)
+#define AUGMENT(x) NAME(x ## suffix)
+
+! CHECK: subroutine prefixfoosuffix()
+! CHECK: print *, "prefixfoosuffix"
+! CHECK: end subroutine prefixfoosuffix
+subroutine AUGMENT()()
+ print *, TOSTRING(AUGMENT())
+end subroutine AUGMENT()
+
+! CHECK: subroutine prefixfoobarsuffix()
+! CHECK: print *, "prefixfoobarsuffix"
+! CHECK: end subroutine prefixfoobarsuffix
+subroutine AUGMENT(bar)()
+ print *, TOSTRING(AUGMENT(bar))
+end subroutine AUGMENT(bar)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
} else if (first == '=' && (last == '!' || last == '/')) { | ||
doPaste = true; // != and /= | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there tests in the test suite that exercise these sanity checks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, all of the token pasting in the test suite that I know of applies to identifiers.
When blank tokens arise from macro replacement in token sequences with token pasting (##), the preprocessor is producing some bogus tokens (e.g., "name(") that can lead to subtle bugs later when macro names are not recognized as such. The fix is to not paste tokens together when the result would not be a valid Fortran or C token in the preprocessing context.
When blank tokens arise from macro replacement in token sequences with token pasting (##), the preprocessor is producing some bogus tokens (e.g., "name(") that can lead to subtle bugs later when macro names are not recognized as such.
The fix is to not paste tokens together when the result would not be a valid Fortran or C token in the preprocessing context.