Skip to content

Reapply "[Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros" #102135

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Reapply "[Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros" #102135

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
efa62c3
[Clang] Implement P3034R1 Module Declarations Shouldn’t be Macros
yronglin Jul 23, 2024
e25019f
[Clang] Add peekNextPPToken, makes peek next token without side-effects
yronglin Aug 5, 2024
829b1c1
[Clang] Add IsCurrentLexingTokAtPhysicalStartOfLine in Lexer to avoid…
yronglin Aug 5, 2024
581160a
[Clang] Partially implement theP1857R3: Modules Dependency Discovery
yronglin Aug 5, 2024
fd71e8c
[Clang] Fix test and remove dead code
yronglin Aug 15, 2024
f0eb145
[Clang] format
yronglin Aug 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions clang/include/clang/Lex/Lexer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ class Lexer : public PreprocessorLexer {

bool IsAtPhysicalStartOfLine;

bool IsCurrentLexingTokAtPhysicalStartOfLine;
Comment on lines 137 to +139
Copy link
Contributor

@cor3ntin cor3ntin Aug 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have IsAtStartOfLine and IsAtPhysicalStartOfLine - Why do we need a third one?

Copy link
Contributor Author

@yronglin yronglin Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your review! This change introduced by 829b1c1. It's used to replace the bool TokAtPhysicalStartOfLine agurments in

llvm-project/clang/lib/Lex/Lexer.cpp

Line 3715 in fcefe95

bool Lexer::LexTokenInternal(Token &Result, bool TokAtPhysicalStartOfLine) {
, it's a bit different with Lexer::IsAtPhysicalStartOfLine , It represents the current token starts at the physical line. We may skip the '\n' or comment before the real token starts. with this change, we can avoid pass this value as a function argument.


bool HasLeadingSpace;

bool HasLeadingEmptyMacro;
Expand Down Expand Up @@ -609,7 +611,7 @@ class Lexer : public PreprocessorLexer {
/// LexTokenInternal - Internal interface to lex a preprocessing token. Called
/// by Lex.
///
bool LexTokenInternal(Token &Result, bool TokAtPhysicalStartOfLine);
bool LexTokenInternal(Token &Result);

bool CheckUnicodeWhitespace(Token &Result, uint32_t C, const char *CurPtr);

Expand Down Expand Up @@ -749,12 +751,9 @@ class Lexer : public PreprocessorLexer {
bool LexCharConstant (Token &Result, const char *CurPtr,
tok::TokenKind Kind);
bool LexEndOfFile (Token &Result, const char *CurPtr);
bool SkipWhitespace (Token &Result, const char *CurPtr,
bool &TokAtPhysicalStartOfLine);
bool SkipLineComment (Token &Result, const char *CurPtr,
bool &TokAtPhysicalStartOfLine);
bool SkipBlockComment (Token &Result, const char *CurPtr,
bool &TokAtPhysicalStartOfLine);
bool SkipWhitespace (Token &Result, const char *CurPtr);
bool SkipLineComment (Token &Result, const char *CurPtr);
bool SkipBlockComment (Token &Result, const char *CurPtr);
bool SaveLineComment (Token &Result, const char *CurPtr);

bool IsStartOfConflictMarker(const char *CurPtr);
Expand Down
47 changes: 22 additions & 25 deletions clang/lib/Lex/Lexer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2484,8 +2484,7 @@ bool Lexer::LexCharConstant(Token &Result, const char *CurPtr,
/// Update BufferPtr to point to the next non-whitespace character and return.
///
/// This method forms a token and returns true if KeepWhitespaceMode is enabled.
bool Lexer::SkipWhitespace(Token &Result, const char *CurPtr,
bool &TokAtPhysicalStartOfLine) {
bool Lexer::SkipWhitespace(Token &Result, const char *CurPtr) {
// Whitespace - Skip it, then return the token after the whitespace.
bool SawNewline = isVerticalWhitespace(CurPtr[-1]);

Expand Down Expand Up @@ -2541,7 +2540,7 @@ bool Lexer::SkipWhitespace(Token &Result, const char *CurPtr,
Result.setFlagValue(Token::LeadingSpace, HasLeadingSpace);
if (SawNewline) {
Result.setFlag(Token::StartOfLine);
TokAtPhysicalStartOfLine = true;
IsCurrentLexingTokAtPhysicalStartOfLine = true;

if (NewLinePtr && lastNewLine && NewLinePtr != lastNewLine && PP) {
if (auto *Handler = PP->getEmptylineHandler())
Expand All @@ -2560,8 +2559,7 @@ bool Lexer::SkipWhitespace(Token &Result, const char *CurPtr,
///
/// If we're in KeepCommentMode or any CommentHandler has inserted
/// some tokens, this will store the first token and return true.
bool Lexer::SkipLineComment(Token &Result, const char *CurPtr,
bool &TokAtPhysicalStartOfLine) {
bool Lexer::SkipLineComment(Token &Result, const char *CurPtr) {
// If Line comments aren't explicitly enabled for this language, emit an
// extension warning.
if (!LineComment) {
Expand Down Expand Up @@ -2717,7 +2715,7 @@ bool Lexer::SkipLineComment(Token &Result, const char *CurPtr,

// The next returned token is at the start of the line.
Result.setFlag(Token::StartOfLine);
TokAtPhysicalStartOfLine = true;
IsCurrentLexingTokAtPhysicalStartOfLine = true;
// No leading whitespace seen so far.
Result.clearFlag(Token::LeadingSpace);
BufferPtr = CurPtr;
Expand Down Expand Up @@ -2842,8 +2840,7 @@ static bool isEndOfBlockCommentWithEscapedNewLine(const char *CurPtr, Lexer *L,
///
/// If we're in KeepCommentMode or any CommentHandler has inserted
/// some tokens, this will store the first token and return true.
bool Lexer::SkipBlockComment(Token &Result, const char *CurPtr,
bool &TokAtPhysicalStartOfLine) {
bool Lexer::SkipBlockComment(Token &Result, const char *CurPtr) {
// Scan one character past where we should, looking for a '/' character. Once
// we find it, check to see if it was preceded by a *. This common
// optimization helps people who like to put a lot of * characters in their
Expand Down Expand Up @@ -3046,7 +3043,7 @@ bool Lexer::SkipBlockComment(Token &Result, const char *CurPtr,
// efficiently now. This is safe even in KeepWhitespaceMode because we would
// have already returned above with the comment as a token.
if (isHorizontalWhitespace(*CurPtr)) {
SkipWhitespace(Result, CurPtr+1, TokAtPhysicalStartOfLine);
SkipWhitespace(Result, CurPtr + 1);
return false;
}

Expand Down Expand Up @@ -3698,11 +3695,11 @@ bool Lexer::Lex(Token &Result) {
HasLeadingEmptyMacro = false;
}

bool atPhysicalStartOfLine = IsAtPhysicalStartOfLine;
IsCurrentLexingTokAtPhysicalStartOfLine = IsAtPhysicalStartOfLine;
IsAtPhysicalStartOfLine = false;
bool isRawLex = isLexingRawMode();
(void) isRawLex;
bool returnedToken = LexTokenInternal(Result, atPhysicalStartOfLine);
bool returnedToken = LexTokenInternal(Result);
// (After the LexTokenInternal call, the lexer might be destroyed.)
assert((returnedToken || !isRawLex) && "Raw lex must succeed");
return returnedToken;
Expand All @@ -3713,7 +3710,7 @@ bool Lexer::Lex(Token &Result) {
/// has a null character at the end of the file. This returns a preprocessing
/// token, not a normal token, as such, it is an internal interface. It assumes
/// that the Flags of result have been cleared before calling this.
bool Lexer::LexTokenInternal(Token &Result, bool TokAtPhysicalStartOfLine) {
bool Lexer::LexTokenInternal(Token &Result) {
LexStart:
assert(!Result.needsCleaning() && "Result needs cleaning");
assert(!Result.hasPtrData() && "Result has not been reset");
Expand Down Expand Up @@ -3766,7 +3763,7 @@ bool Lexer::LexTokenInternal(Token &Result, bool TokAtPhysicalStartOfLine) {
if (!isLexingRawMode())
Diag(CurPtr-1, diag::null_in_file);
Result.setFlag(Token::LeadingSpace);
if (SkipWhitespace(Result, CurPtr, TokAtPhysicalStartOfLine))
if (SkipWhitespace(Result, CurPtr))
return true; // KeepWhitespaceMode

// We know the lexer hasn't changed, so just try again with this lexer.
Expand Down Expand Up @@ -3812,7 +3809,7 @@ bool Lexer::LexTokenInternal(Token &Result, bool TokAtPhysicalStartOfLine) {
// No leading whitespace seen so far.
Result.clearFlag(Token::LeadingSpace);

if (SkipWhitespace(Result, CurPtr, TokAtPhysicalStartOfLine))
if (SkipWhitespace(Result, CurPtr))
return true; // KeepWhitespaceMode

// We only saw whitespace, so just try again with this lexer.
Expand All @@ -3824,7 +3821,7 @@ bool Lexer::LexTokenInternal(Token &Result, bool TokAtPhysicalStartOfLine) {
case '\v':
SkipHorizontalWhitespace:
Result.setFlag(Token::LeadingSpace);
if (SkipWhitespace(Result, CurPtr, TokAtPhysicalStartOfLine))
if (SkipWhitespace(Result, CurPtr))
return true; // KeepWhitespaceMode

SkipIgnoredUnits:
Expand All @@ -3834,11 +3831,11 @@ bool Lexer::LexTokenInternal(Token &Result, bool TokAtPhysicalStartOfLine) {
// too (without going through the big switch stmt).
if (CurPtr[0] == '/' && CurPtr[1] == '/' && !inKeepCommentMode() &&
LineComment && (LangOpts.CPlusPlus || !LangOpts.TraditionalCPP)) {
if (SkipLineComment(Result, CurPtr+2, TokAtPhysicalStartOfLine))
if (SkipLineComment(Result, CurPtr + 2))
return true; // There is a token to return.
goto SkipIgnoredUnits;
} else if (CurPtr[0] == '/' && CurPtr[1] == '*' && !inKeepCommentMode()) {
if (SkipBlockComment(Result, CurPtr+2, TokAtPhysicalStartOfLine))
if (SkipBlockComment(Result, CurPtr + 2))
return true; // There is a token to return.
goto SkipIgnoredUnits;
} else if (isHorizontalWhitespace(*CurPtr)) {
Expand Down Expand Up @@ -4150,8 +4147,7 @@ bool Lexer::LexTokenInternal(Token &Result, bool TokAtPhysicalStartOfLine) {
TreatAsComment = getCharAndSize(CurPtr+SizeTmp, SizeTmp2) != '*';

if (TreatAsComment) {
if (SkipLineComment(Result, ConsumeChar(CurPtr, SizeTmp, Result),
TokAtPhysicalStartOfLine))
if (SkipLineComment(Result, ConsumeChar(CurPtr, SizeTmp, Result)))
return true; // There is a token to return.

// It is common for the tokens immediately after a // comment to be
Expand All @@ -4162,8 +4158,7 @@ bool Lexer::LexTokenInternal(Token &Result, bool TokAtPhysicalStartOfLine) {
}

if (Char == '*') { // /**/ comment.
if (SkipBlockComment(Result, ConsumeChar(CurPtr, SizeTmp, Result),
TokAtPhysicalStartOfLine))
if (SkipBlockComment(Result, ConsumeChar(CurPtr, SizeTmp, Result)))
return true; // There is a token to return.

// We only saw whitespace, so just try again with this lexer.
Expand Down Expand Up @@ -4203,7 +4198,8 @@ bool Lexer::LexTokenInternal(Token &Result, bool TokAtPhysicalStartOfLine) {
// it's actually the start of a preprocessing directive. Callback to
// the preprocessor to handle it.
// TODO: -fpreprocessed mode??
if (TokAtPhysicalStartOfLine && !LexingRawMode && !Is_PragmaLexer)
if (IsCurrentLexingTokAtPhysicalStartOfLine && !LexingRawMode &&
!Is_PragmaLexer)
goto HandleDirective;

Kind = tok::hash;
Expand Down Expand Up @@ -4392,7 +4388,8 @@ bool Lexer::LexTokenInternal(Token &Result, bool TokAtPhysicalStartOfLine) {
// it's actually the start of a preprocessing directive. Callback to
// the preprocessor to handle it.
// TODO: -fpreprocessed mode??
if (TokAtPhysicalStartOfLine && !LexingRawMode && !Is_PragmaLexer)
if (IsCurrentLexingTokAtPhysicalStartOfLine && !LexingRawMode &&
!Is_PragmaLexer)
goto HandleDirective;

Kind = tok::hash;
Expand All @@ -4412,7 +4409,7 @@ bool Lexer::LexTokenInternal(Token &Result, bool TokAtPhysicalStartOfLine) {
if (!LangOpts.AsmPreprocessor) {
if (uint32_t CodePoint = tryReadUCN(CurPtr, BufferPtr, &Result)) {
if (CheckUnicodeWhitespace(Result, CodePoint, CurPtr)) {
if (SkipWhitespace(Result, CurPtr, TokAtPhysicalStartOfLine))
if (SkipWhitespace(Result, CurPtr))
return true; // KeepWhitespaceMode

// We only saw whitespace, so just try again with this lexer.
Expand Down Expand Up @@ -4445,7 +4442,7 @@ bool Lexer::LexTokenInternal(Token &Result, bool TokAtPhysicalStartOfLine) {
llvm::strictConversion);
if (Status == llvm::conversionOK) {
if (CheckUnicodeWhitespace(Result, CodePoint, CurPtr)) {
if (SkipWhitespace(Result, CurPtr, TokAtPhysicalStartOfLine))
if (SkipWhitespace(Result, CurPtr))
return true; // KeepWhitespaceMode

// We only saw whitespace, so just try again with this lexer.
Expand Down