Skip to content

[clang] Only propagate AtStartOfLine when expanding token stream and hit the end of TokenLexer#173052

Closed
yronglin wants to merge 1 commit intollvm:mainfrom
yronglin:fix_cxx_module_tok_flags
Closed

[clang] Only propagate AtStartOfLine when expanding token stream and hit the end of TokenLexer#173052
yronglin wants to merge 1 commit intollvm:mainfrom
yronglin:fix_cxx_module_tok_flags

Conversation

@yronglin
Copy link
Contributor

@yronglin yronglin commented Dec 19, 2025

This patch fix a bug introduced by #107168.

Consider the following code:

#define str(s) # s 
#define xstr(s) str(s) 
#define INCFILE(n) vers ## n 

include xstr(INCFILE(2).h) 

The period has an unexpected AtStartOfLine flag when it's lexed from TokenLexer, after phase 4, we expected include "vers2.h", but got include "vers2 .h", an unexpected whitespace occurred before ..

We only need to propagate AtStartOfLine flag when current TokenLexer expanding an token stream.

No new test added because this issue was covered by clang/test/Preprocessor/c99-6_10_3_4_p6.c.

… hit the end of TokenLexer

Signed-off-by: yronglin <yronglin777@gmail.com>
@yronglin yronglin requested review from cor3ntin and shafik December 19, 2025 17:33
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Dec 19, 2025
@llvmbot
Copy link
Member

llvmbot commented Dec 19, 2025

@llvm/pr-subscribers-clang

Author: None (yronglin)

Changes

This patch fix a bug introduced by #107168.

Consider the following code:

#define str(s) # s 
#define xstr(s) str(s) 
#define INCFILE(n) vers ## n 

include xstr(INCFILE(2).h) 

The period has an unexpected AtStartOfLine flag when it's lexed from TokenLexer, after phase 4, we expected include "vers2.h", but got include "vers2 .h", an unexpected whitespace occurred before ..

We only need to propagate AtStartOfLine flag when current TokenLexer expanding an token stream.


Full diff: https://github.com/llvm/llvm-project/pull/173052.diff

1 Files Affected:

  • (modified) clang/lib/Lex/TokenLexer.cpp (+1-1)
diff --git a/clang/lib/Lex/TokenLexer.cpp b/clang/lib/Lex/TokenLexer.cpp
index e9531ee9794d2..ce072de79b6af 100644
--- a/clang/lib/Lex/TokenLexer.cpp
+++ b/clang/lib/Lex/TokenLexer.cpp
@@ -632,7 +632,7 @@ bool TokenLexer::Lex(Token &Tok) {
     //
     // The 'extern' token should has 'StartOfLine' flag when current TokenLexer
     // exits and propagate line start/leading space info.
-    if (isLexingCXXModuleDirective()) {
+    if (!Macro && isLexingCXXModuleDirective()) {
       AtStartOfLine = true;
       setLexingCXXModuleDirective(false);
     }

@yronglin
Copy link
Contributor Author

yronglin commented Dec 19, 2025

Hmm, it's weird to me why this issue was not catched in premerge CI. I have verified this fix in my local.

@yronglin yronglin changed the title [clang] Only propagated AtStartOfLine when expanding token stream and hit the end of TokenLexer [clang] Only propagate AtStartOfLine when expanding token stream and hit the end of TokenLexer Dec 19, 2025
@dyung
Copy link
Collaborator

dyung commented Dec 19, 2025

Hmm, it's weird to me why this issue was not catched in premerge CI. I have verified this fix in my local.

The failure seems to be intermittently happening for some reason, at least on one of my bots:
https://lab.llvm.org/buildbot/#/builders/144

@ilovepi
Copy link
Contributor

ilovepi commented Dec 19, 2025

@yronglin
Copy link
Contributor Author

https://lab.llvm.org/buildbot/#/builders/144

The failure and current PR is not the same issue. This PR fixes extra whitespace before token. I think I have found the root cause of the crash.

@yronglin yronglin closed this Dec 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants