[clang] Revert "Pass LangOpts from CompilerInstance to DependencyScanningWork… (PR #94488)
https://github.com/nishithshah2211 created
https://github.com/llvm/llvm-project/pull/94488
…er (#93753)"
This reverts commit 9862080b1cbf685c0d462b29596e3f7206d24aa2.
>From 3bc45673476ae4fbd1166a2d6bff240077104f5e Mon Sep 17 00:00:00 2001
From: Nishith Shah
Date: Tue, 4 Jun 2024 09:47:55 -0700
Subject: [PATCH] Revert "Pass LangOpts from CompilerInstance to
DependencyScanningWorker (#93753)"
This reverts commit 9862080b1cbf685c0d462b29596e3f7206d24aa2.
---
.../clang/Lex/DependencyDirectivesScanner.h | 3 +-
.../DependencyScanningFilesystem.h| 3 +-
clang/lib/Frontend/FrontendActions.cpp| 4 +-
clang/lib/Lex/DependencyDirectivesScanner.cpp | 22 ++---
.../DependencyScanningFilesystem.cpp | 4 +-
.../DependencyScanningWorker.cpp | 5 +-
.../Lex/DependencyDirectivesScannerTest.cpp | 82 +++
.../Lex/PPDependencyDirectivesTest.cpp| 3 +-
8 files changed, 31 insertions(+), 95 deletions(-)
diff --git a/clang/include/clang/Lex/DependencyDirectivesScanner.h
b/clang/include/clang/Lex/DependencyDirectivesScanner.h
index 2f8354dec939f..0e115906fbfe5 100644
--- a/clang/include/clang/Lex/DependencyDirectivesScanner.h
+++ b/clang/include/clang/Lex/DependencyDirectivesScanner.h
@@ -17,7 +17,6 @@
#ifndef LLVM_CLANG_LEX_DEPENDENCYDIRECTIVESSCANNER_H
#define LLVM_CLANG_LEX_DEPENDENCYDIRECTIVESSCANNER_H
-#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
#include "llvm/ADT/ArrayRef.h"
@@ -118,7 +117,7 @@ struct Directive {
bool scanSourceForDependencyDirectives(
StringRef Input, SmallVectorImpl
&Tokens,
SmallVectorImpl &Directives,
-const LangOptions &LangOpts, DiagnosticsEngine *Diags = nullptr,
+DiagnosticsEngine *Diags = nullptr,
SourceLocation InputSourceLoc = SourceLocation());
/// Print the previously scanned dependency directives as minimized source
text.
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index 9dc20065a09a3..f7b4510d7f7be 100644
---
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -363,8 +363,7 @@ class DependencyScanningWorkerFilesystem
///
/// Returns true if the directive tokens are populated for this file entry,
/// false if not (i.e. this entry is not a file or its scan fails).
- bool ensureDirectiveTokensArePopulated(EntryRef Entry,
- const LangOptions &LangOpts);
+ bool ensureDirectiveTokensArePopulated(EntryRef Entry);
/// Check whether \p Path exists. By default checks cached result of \c
/// status(), and falls back on FS if unable to do so.
diff --git a/clang/lib/Frontend/FrontendActions.cpp
b/clang/lib/Frontend/FrontendActions.cpp
index 1812b85860f64..4f064321997a2 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -1169,8 +1169,8 @@ void
PrintDependencyDirectivesSourceMinimizerAction::ExecuteAction() {
llvm::SmallVector Tokens;
llvm::SmallVector Directives;
if (scanSourceForDependencyDirectives(
- FromFile.getBuffer(), Tokens, Directives, CI.getLangOpts(),
- &CI.getDiagnostics(), SM.getLocForStartOfFile(SM.getMainFileID( {
+ FromFile.getBuffer(), Tokens, Directives, &CI.getDiagnostics(),
+ SM.getLocForStartOfFile(SM.getMainFileID( {
assert(CI.getDiagnostics().hasErrorOccurred() &&
"no errors reported for failure");
diff --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp
b/clang/lib/Lex/DependencyDirectivesScanner.cpp
index fda54d314eef6..0971daa1f3666 100644
--- a/clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -62,17 +62,14 @@ struct DirectiveWithTokens {
struct Scanner {
Scanner(StringRef Input,
SmallVectorImpl &Tokens,
- DiagnosticsEngine *Diags, SourceLocation InputSourceLoc,
- const LangOptions &LangOpts)
+ DiagnosticsEngine *Diags, SourceLocation InputSourceLoc)
: Input(Input), Tokens(Tokens), Diags(Diags),
-InputSourceLoc(InputSourceLoc),
-LangOpts(getLangOptsForDepScanning(LangOpts)),
-TheLexer(InputSourceLoc, this->LangOpts, Input.begin(), Input.begin(),
+InputSourceLoc(InputSourceLoc), LangOpts(getLangOptsForDepScanning()),
+TheLexer(InputSourceLoc, LangOpts, Input.begin(), Input.begin(),
Input.end()) {}
- static LangOptions
- getLangOptsForDepScanning(const LangOptions &invocationLangOpts) {
-LangOptions LangOpts(invocationLangOpts);
+ static LangOptions getLangOptsForDepScanning() {
+LangOptions LangOpts;
// Set the lexer to use 'tok::at' for '@', instead of 'tok::unknown'.
LangOpts.ObjC = true;
LangOpts.LineComment = true;
@@
[clang] Pass LangOpts from CompilerInstance to DependencyScanningWorker (PR #93753)
nishithshah2211 wrote: Here is the revert PR: https://github.com/llvm/llvm-project/pull/94488 https://github.com/llvm/llvm-project/pull/93753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "Pass LangOpts from CompilerInstance to DependencyScanningWork… (PR #94488)
nishithshah2211 wrote: @jansvoboda11 - please merge this revert in when you get a chance, thanks! https://github.com/llvm/llvm-project/pull/94488 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] clang: Relax LangOpts checks when lexing quoted numbers during preprocessing (PR #95798)
https://github.com/nishithshah2211 created
https://github.com/llvm/llvm-project/pull/95798
When trying to lex numeric constants that use single quotes as separators, if
the lexer is parsing a preprocessor directive, we can relax the language
options check. These checks will be enforced in a later phase after
preprocessing/scanning.
This commit is a second attempt to fix
https://github.com/llvm/llvm-project/issues/88896. The first attempt to fix
this is here: https://github.com/llvm/llvm-project/pull/93753, which had to be
reverted because of the discussions in that same PR.
>From 9257132c272ab2774a6639af7215fc78f6118fda Mon Sep 17 00:00:00 2001
From: Nishith Shah
Date: Wed, 5 Jun 2024 08:27:40 -0700
Subject: [PATCH] clang: Relax LangOpts checks when lexing quoted numbers
during preprocessing
When trying to lex numeric constants that use single quotes as
separators, if the lexer is parsing a preprocessor directive, we can
relax the language options check. These checks will be enforced in a
later phase after preprocessing/scanning.
This commit is a second attempt to fix
https://github.com/llvm/llvm-project/issues/88896. The first attempt to
fix this is here: https://github.com/llvm/llvm-project/pull/93753, which
had to be reverted because of the discussions in that same PR.
---
clang/lib/Lex/Lexer.cpp | 3 +-
.../Lex/DependencyDirectivesScannerTest.cpp | 29 +++
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp
index e59c7805b3862..60c101871be75 100644
--- a/clang/lib/Lex/Lexer.cpp
+++ b/clang/lib/Lex/Lexer.cpp
@@ -2068,7 +2068,8 @@ bool Lexer::LexNumericConstant(Token &Result, const char
*CurPtr) {
}
// If we have a digit separator, continue.
- if (C == '\'' && (LangOpts.CPlusPlus14 || LangOpts.C23)) {
+ if (C == '\'' &&
+ (LangOpts.CPlusPlus14 || LangOpts.C23 || ParsingPreprocessorDirective)) {
auto [Next, NextSize] = getCharAndSizeNoWarn(CurPtr + Size, LangOpts);
if (isAsciiIdentifierContinue(Next)) {
if (!isLexingRawMode())
diff --git a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
index 59fef9ecbb9c9..0d2d98b16f350 100644
--- a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -7,6 +7,7 @@
//===--===//
#include "clang/Lex/DependencyDirectivesScanner.h"
+#include "clang/Basic/TokenKinds.h"
#include "llvm/ADT/SmallString.h"
#include "gtest/gtest.h"
@@ -1012,4 +1013,32 @@ TEST(MinimizeSourceToDependencyDirectivesTest,
TokensBeforeEOF) {
EXPECT_STREQ("#ifndef A\n#define A\n#endif\n\n", Out.data());
}
+TEST(MinimizeSourceToDependencyDirectivesTest, QuoteSeparatedNumber) {
+ SmallVector Out;
+ SmallVector Tokens;
+ SmallVector Directives;
+
+ StringRef Source = R"(
+#if 123'124
+#endif
+)";
+
+ ASSERT_FALSE(
+ minimizeSourceToDependencyDirectives(Source, Out, Tokens, Directives));
+ EXPECT_STREQ("#if 123'124\n#endif\n", Out.data());
+ ASSERT_EQ(Directives.size(), 3u);
+ EXPECT_EQ(Directives[0].Kind, dependency_directives_scan::pp_if);
+ EXPECT_EQ(Directives[1].Kind, dependency_directives_scan::pp_endif);
+ EXPECT_EQ(Directives[2].Kind, dependency_directives_scan::pp_eof);
+ ASSERT_EQ(Tokens.size(), 7u);
+
+ ASSERT_TRUE(Tokens[0].is(tok::hash));
+ ASSERT_TRUE(Tokens[1].is(tok::raw_identifier)); // "if"
+ ASSERT_TRUE(Tokens[2].is(tok::numeric_constant)); // 123'124
+ ASSERT_TRUE(Tokens[3].is(tok::eod));
+ ASSERT_TRUE(Tokens[4].is(tok::hash));
+ ASSERT_TRUE(Tokens[5].is(tok::raw_identifier)); // #endif
+ ASSERT_TRUE(Tokens[6].is(tok::eod));
+}
+
} // end anonymous namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] clang: Relax LangOpts checks when lexing quoted numbers during preprocessing (PR #95798)
nishithshah2211 wrote: @jansvoboda11 - I put up this PR to get your thoughts on addressing https://github.com/llvm/llvm-project/issues/88896, since the previous commit was reverted. Is this more along the lines of what you were thinking? I will push more changes to include a test for the Lexer too. https://github.com/llvm/llvm-project/pull/95798 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] clang: Relax LangOpts checks when lexing quoted numbers during preprocessing (PR #95798)
nishithshah2211 wrote: > The changes should come with a release note in clang/docs/ReleaseNotes.rst so > users know about the fix. Noted. Will include a release note, a lexer test and address any other comments for the next commit. https://github.com/llvm/llvm-project/pull/95798 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Pass LangOpts from CompilerInstance to DependencyScanningWorker (PR #93753)
https://github.com/nishithshah2211 created
https://github.com/llvm/llvm-project/pull/93753
This commit fixes https://github.com/llvm/llvm-project/issues/88896 by passing
LangOpts from the CompilerInstance to
DependencyScanningWorker so that the original LangOpts are preserved/respected.
This makes for more accurate parsing/lexing when certain language versions or
features specific to versions are to be used.
>From 01323e5675c5b13014b3b93e7f0c865d7163ed8e Mon Sep 17 00:00:00 2001
From: Nishith Shah
Date: Wed, 29 May 2024 12:34:52 -0700
Subject: [PATCH] Pass LangOpts from CompilerInstance to
DependencyScanningWorker
This commit fixes https://github.com/llvm/llvm-project/issues/88896
by passing LangOpts from the CompilerInstance to
DependencyScanningWorker so that the original LangOpts are
preserved/respected. This makes for more accurate parsing/lexing when
certain language versions or features specific to versions are to be used.
---
.../clang/Lex/DependencyDirectivesScanner.h | 3 ++-
.../DependencyScanningFilesystem.h| 3 ++-
clang/lib/Frontend/FrontendActions.cpp| 4 ++--
clang/lib/Lex/DependencyDirectivesScanner.cpp | 20 +++
.../DependencyScanningFilesystem.cpp | 4 ++--
.../DependencyScanningWorker.cpp | 5 +++--
6 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/clang/include/clang/Lex/DependencyDirectivesScanner.h
b/clang/include/clang/Lex/DependencyDirectivesScanner.h
index 0e115906fbfe5..2f8354dec939f 100644
--- a/clang/include/clang/Lex/DependencyDirectivesScanner.h
+++ b/clang/include/clang/Lex/DependencyDirectivesScanner.h
@@ -17,6 +17,7 @@
#ifndef LLVM_CLANG_LEX_DEPENDENCYDIRECTIVESSCANNER_H
#define LLVM_CLANG_LEX_DEPENDENCYDIRECTIVESSCANNER_H
+#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
#include "llvm/ADT/ArrayRef.h"
@@ -117,7 +118,7 @@ struct Directive {
bool scanSourceForDependencyDirectives(
StringRef Input, SmallVectorImpl
&Tokens,
SmallVectorImpl &Directives,
-DiagnosticsEngine *Diags = nullptr,
+const LangOptions &LangOpts, DiagnosticsEngine *Diags = nullptr,
SourceLocation InputSourceLoc = SourceLocation());
/// Print the previously scanned dependency directives as minimized source
text.
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index f7b4510d7f7be..9dc20065a09a3 100644
---
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -363,7 +363,8 @@ class DependencyScanningWorkerFilesystem
///
/// Returns true if the directive tokens are populated for this file entry,
/// false if not (i.e. this entry is not a file or its scan fails).
- bool ensureDirectiveTokensArePopulated(EntryRef Entry);
+ bool ensureDirectiveTokensArePopulated(EntryRef Entry,
+ const LangOptions &LangOpts);
/// Check whether \p Path exists. By default checks cached result of \c
/// status(), and falls back on FS if unable to do so.
diff --git a/clang/lib/Frontend/FrontendActions.cpp
b/clang/lib/Frontend/FrontendActions.cpp
index 454653a31534c..eddb2ac0c0834 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -1168,8 +1168,8 @@ void
PrintDependencyDirectivesSourceMinimizerAction::ExecuteAction() {
llvm::SmallVector Tokens;
llvm::SmallVector Directives;
if (scanSourceForDependencyDirectives(
- FromFile.getBuffer(), Tokens, Directives, &CI.getDiagnostics(),
- SM.getLocForStartOfFile(SM.getMainFileID( {
+ FromFile.getBuffer(), Tokens, Directives, CI.getLangOpts(),
+ &CI.getDiagnostics(), SM.getLocForStartOfFile(SM.getMainFileID( {
assert(CI.getDiagnostics().hasErrorOccurred() &&
"no errors reported for failure");
diff --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp
b/clang/lib/Lex/DependencyDirectivesScanner.cpp
index 0971daa1f3666..f7462aefa4f87 100644
--- a/clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -62,14 +62,17 @@ struct DirectiveWithTokens {
struct Scanner {
Scanner(StringRef Input,
SmallVectorImpl &Tokens,
- DiagnosticsEngine *Diags, SourceLocation InputSourceLoc)
+ DiagnosticsEngine *Diags, SourceLocation InputSourceLoc,
+ const LangOptions &LangOpts)
: Input(Input), Tokens(Tokens), Diags(Diags),
-InputSourceLoc(InputSourceLoc), LangOpts(getLangOptsForDepScanning()),
+InputSourceLoc(InputSourceLoc),
+LangOpts(getLangOptsForDepScanning(LangOpts)),
TheLexer(InputSourceLoc, LangOpts, Input.begin(), Input.begin(),
Input.end()) {}
- static LangOptions getLangOptsForDepScanning() {
[clang] Pass LangOpts from CompilerInstance to DependencyScanningWorker (PR #93753)
nishithshah2211 wrote: @cor3ntin @Sirraide I put up a draft PR to get some feedback and see if the changes here make sense or not. Also tagging @pogo59 since I received good feedback/pointers here: https://discourse.llvm.org/t/looking-for-help-with-accessing-langopts-from-the-actual-compiler-invocation/79228. My main concern is mostly around introducing `LangOpts` within `ensureDirectiveTokensArePopulated` API. I'll add new tests and fix existing tests in the next commit on this PR if the source changes seem reasonable. https://github.com/llvm/llvm-project/pull/93753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Pass LangOpts from CompilerInstance to DependencyScanningWorker (PR #93753)
https://github.com/nishithshah2211 updated
https://github.com/llvm/llvm-project/pull/93753
>From ae79ebec844a6a308bb370184eab892bd74e8fa1 Mon Sep 17 00:00:00 2001
From: Nishith Shah
Date: Wed, 29 May 2024 12:34:52 -0700
Subject: [PATCH] Pass LangOpts from CompilerInstance to
DependencyScanningWorker
This commit fixes https://github.com/llvm/llvm-project/issues/88896
by passing LangOpts from the CompilerInstance to
DependencyScanningWorker so that the original LangOpts are
preserved/respected. This makes for more accurate parsing/lexing when
certain language versions or features specific to versions are to be used.
---
.../clang/Lex/DependencyDirectivesScanner.h | 3 +-
.../DependencyScanningFilesystem.h| 3 +-
clang/lib/Frontend/FrontendActions.cpp| 4 +--
clang/lib/Lex/DependencyDirectivesScanner.cpp | 22 --
.../DependencyScanningFilesystem.cpp | 4 +--
.../DependencyScanningWorker.cpp | 5 ++--
.../Lex/DependencyDirectivesScannerTest.cpp | 30 +--
.../Lex/PPDependencyDirectivesTest.cpp| 3 +-
8 files changed, 47 insertions(+), 27 deletions(-)
diff --git a/clang/include/clang/Lex/DependencyDirectivesScanner.h
b/clang/include/clang/Lex/DependencyDirectivesScanner.h
index 0e115906fbfe5..2f8354dec939f 100644
--- a/clang/include/clang/Lex/DependencyDirectivesScanner.h
+++ b/clang/include/clang/Lex/DependencyDirectivesScanner.h
@@ -17,6 +17,7 @@
#ifndef LLVM_CLANG_LEX_DEPENDENCYDIRECTIVESSCANNER_H
#define LLVM_CLANG_LEX_DEPENDENCYDIRECTIVESSCANNER_H
+#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
#include "llvm/ADT/ArrayRef.h"
@@ -117,7 +118,7 @@ struct Directive {
bool scanSourceForDependencyDirectives(
StringRef Input, SmallVectorImpl
&Tokens,
SmallVectorImpl &Directives,
-DiagnosticsEngine *Diags = nullptr,
+const LangOptions &LangOpts, DiagnosticsEngine *Diags = nullptr,
SourceLocation InputSourceLoc = SourceLocation());
/// Print the previously scanned dependency directives as minimized source
text.
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index f7b4510d7f7be..9dc20065a09a3 100644
---
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -363,7 +363,8 @@ class DependencyScanningWorkerFilesystem
///
/// Returns true if the directive tokens are populated for this file entry,
/// false if not (i.e. this entry is not a file or its scan fails).
- bool ensureDirectiveTokensArePopulated(EntryRef Entry);
+ bool ensureDirectiveTokensArePopulated(EntryRef Entry,
+ const LangOptions &LangOpts);
/// Check whether \p Path exists. By default checks cached result of \c
/// status(), and falls back on FS if unable to do so.
diff --git a/clang/lib/Frontend/FrontendActions.cpp
b/clang/lib/Frontend/FrontendActions.cpp
index 454653a31534c..eddb2ac0c0834 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -1168,8 +1168,8 @@ void
PrintDependencyDirectivesSourceMinimizerAction::ExecuteAction() {
llvm::SmallVector Tokens;
llvm::SmallVector Directives;
if (scanSourceForDependencyDirectives(
- FromFile.getBuffer(), Tokens, Directives, &CI.getDiagnostics(),
- SM.getLocForStartOfFile(SM.getMainFileID( {
+ FromFile.getBuffer(), Tokens, Directives, CI.getLangOpts(),
+ &CI.getDiagnostics(), SM.getLocForStartOfFile(SM.getMainFileID( {
assert(CI.getDiagnostics().hasErrorOccurred() &&
"no errors reported for failure");
diff --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp
b/clang/lib/Lex/DependencyDirectivesScanner.cpp
index 0971daa1f3666..fda54d314eef6 100644
--- a/clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -62,14 +62,17 @@ struct DirectiveWithTokens {
struct Scanner {
Scanner(StringRef Input,
SmallVectorImpl &Tokens,
- DiagnosticsEngine *Diags, SourceLocation InputSourceLoc)
+ DiagnosticsEngine *Diags, SourceLocation InputSourceLoc,
+ const LangOptions &LangOpts)
: Input(Input), Tokens(Tokens), Diags(Diags),
-InputSourceLoc(InputSourceLoc), LangOpts(getLangOptsForDepScanning()),
-TheLexer(InputSourceLoc, LangOpts, Input.begin(), Input.begin(),
+InputSourceLoc(InputSourceLoc),
+LangOpts(getLangOptsForDepScanning(LangOpts)),
+TheLexer(InputSourceLoc, this->LangOpts, Input.begin(), Input.begin(),
Input.end()) {}
- static LangOptions getLangOptsForDepScanning() {
-LangOptions LangOpts;
+ static LangOptions
+ getLangOptsForDepScanning(const LangOptions &invocationLangOpts) {
+Lan
[clang] Pass LangOpts from CompilerInstance to DependencyScanningWorker (PR #93753)
nishithshah2211 wrote: @cor3ntin Thanks for reviewing. I updated the PR to get the tests/build passing. Please take a look when you get a chance. https://github.com/llvm/llvm-project/pull/93753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Pass LangOpts from CompilerInstance to DependencyScanningWorker (PR #93753)
https://github.com/nishithshah2211 updated
https://github.com/llvm/llvm-project/pull/93753
>From 46a25038abbcf5ab9eebded6813b4bbb71a44925 Mon Sep 17 00:00:00 2001
From: Nishith Shah
Date: Wed, 29 May 2024 12:34:52 -0700
Subject: [PATCH] Pass LangOpts from CompilerInstance to
DependencyScanningWorker
This commit fixes https://github.com/llvm/llvm-project/issues/88896
by passing LangOpts from the CompilerInstance to
DependencyScanningWorker so that the original LangOpts are
preserved/respected. This makes for more accurate parsing/lexing when
certain language versions or features specific to versions are to be used.
---
.../clang/Lex/DependencyDirectivesScanner.h | 3 +-
.../DependencyScanningFilesystem.h| 3 +-
clang/lib/Frontend/FrontendActions.cpp| 4 +-
clang/lib/Lex/DependencyDirectivesScanner.cpp | 22 +++--
.../DependencyScanningFilesystem.cpp | 4 +-
.../DependencyScanningWorker.cpp | 5 +-
.../Lex/DependencyDirectivesScannerTest.cpp | 82 ---
.../Lex/PPDependencyDirectivesTest.cpp| 3 +-
8 files changed, 95 insertions(+), 31 deletions(-)
diff --git a/clang/include/clang/Lex/DependencyDirectivesScanner.h
b/clang/include/clang/Lex/DependencyDirectivesScanner.h
index 0e115906fbfe5..2f8354dec939f 100644
--- a/clang/include/clang/Lex/DependencyDirectivesScanner.h
+++ b/clang/include/clang/Lex/DependencyDirectivesScanner.h
@@ -17,6 +17,7 @@
#ifndef LLVM_CLANG_LEX_DEPENDENCYDIRECTIVESSCANNER_H
#define LLVM_CLANG_LEX_DEPENDENCYDIRECTIVESSCANNER_H
+#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
#include "llvm/ADT/ArrayRef.h"
@@ -117,7 +118,7 @@ struct Directive {
bool scanSourceForDependencyDirectives(
StringRef Input, SmallVectorImpl
&Tokens,
SmallVectorImpl &Directives,
-DiagnosticsEngine *Diags = nullptr,
+const LangOptions &LangOpts, DiagnosticsEngine *Diags = nullptr,
SourceLocation InputSourceLoc = SourceLocation());
/// Print the previously scanned dependency directives as minimized source
text.
diff --git
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index f7b4510d7f7be..9dc20065a09a3 100644
---
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -363,7 +363,8 @@ class DependencyScanningWorkerFilesystem
///
/// Returns true if the directive tokens are populated for this file entry,
/// false if not (i.e. this entry is not a file or its scan fails).
- bool ensureDirectiveTokensArePopulated(EntryRef Entry);
+ bool ensureDirectiveTokensArePopulated(EntryRef Entry,
+ const LangOptions &LangOpts);
/// Check whether \p Path exists. By default checks cached result of \c
/// status(), and falls back on FS if unable to do so.
diff --git a/clang/lib/Frontend/FrontendActions.cpp
b/clang/lib/Frontend/FrontendActions.cpp
index 454653a31534c..eddb2ac0c0834 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -1168,8 +1168,8 @@ void
PrintDependencyDirectivesSourceMinimizerAction::ExecuteAction() {
llvm::SmallVector Tokens;
llvm::SmallVector Directives;
if (scanSourceForDependencyDirectives(
- FromFile.getBuffer(), Tokens, Directives, &CI.getDiagnostics(),
- SM.getLocForStartOfFile(SM.getMainFileID( {
+ FromFile.getBuffer(), Tokens, Directives, CI.getLangOpts(),
+ &CI.getDiagnostics(), SM.getLocForStartOfFile(SM.getMainFileID( {
assert(CI.getDiagnostics().hasErrorOccurred() &&
"no errors reported for failure");
diff --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp
b/clang/lib/Lex/DependencyDirectivesScanner.cpp
index 0971daa1f3666..fda54d314eef6 100644
--- a/clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -62,14 +62,17 @@ struct DirectiveWithTokens {
struct Scanner {
Scanner(StringRef Input,
SmallVectorImpl &Tokens,
- DiagnosticsEngine *Diags, SourceLocation InputSourceLoc)
+ DiagnosticsEngine *Diags, SourceLocation InputSourceLoc,
+ const LangOptions &LangOpts)
: Input(Input), Tokens(Tokens), Diags(Diags),
-InputSourceLoc(InputSourceLoc), LangOpts(getLangOptsForDepScanning()),
-TheLexer(InputSourceLoc, LangOpts, Input.begin(), Input.begin(),
+InputSourceLoc(InputSourceLoc),
+LangOpts(getLangOptsForDepScanning(LangOpts)),
+TheLexer(InputSourceLoc, this->LangOpts, Input.begin(), Input.begin(),
Input.end()) {}
- static LangOptions getLangOptsForDepScanning() {
-LangOptions LangOpts;
+ static LangOptions
+ getLangOptsForDepScanning(const LangOptions &invocationLangOpts) {
+LangOptions Lang
[clang] Pass LangOpts from CompilerInstance to DependencyScanningWorker (PR #93753)
nishithshah2211 wrote: Yes, sorry I missed committing that change. Let me know if there is a better way to test out the changes or add any additional changes. https://github.com/llvm/llvm-project/pull/93753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Pass LangOpts from CompilerInstance to DependencyScanningWorker (PR #93753)
nishithshah2211 wrote: Thanks for the pointers. What would be the process to merge this in? https://github.com/llvm/llvm-project/pull/93753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Pass LangOpts from CompilerInstance to DependencyScanningWorker (PR #93753)
nishithshah2211 wrote: Thanks for the comments @jansvoboda11 . I am new to all these different moving parts and want to understand better. I have a few questions. > If you concurrently scan the same file under two language standards with the > same scanning service, it becomes non-deterministic which one gets cached in > the filesystem cache. That is true. But until your comment, I did not even know that it is possible (and supported) to be able to invoke the same scanning service on the same file under two language options (say c++14 and c++11). How would someone do that? Asking so I can test this out locally and try to come up with a better solution. (Also, why would someone do that?) > You need to make the language standard a part of the cache key. This was kind of one of my concerns that I had called out here: https://discourse.llvm.org/t/looking-for-help-with-accessing-langopts-from-the-actual-compiler-invocation/79228/3. Specifically: > would it look a bit off to someone if they were to look at the header for > DependencyScanningWorkerFilesystem and see that the > ensureDirectiveTokensArePopulated API took a LangOpts specifically Given that `LangOpts` is kind of becoming a feature within `DependencyScanningWorkerFilesystem`'s APIs, I am kind of inclined towards having `LangOpts` as part of the cache key for disambiguation - but again, I am very very new to this. > An alternative solution (that I prefer) is to set up the scanner in a way > that always accepts ' in integer constants. Would this be considered "hacky"? Because if we go this way, the Scanner would technically be operating in a different language mode for integers, potentially overriding the language mode arg that was passed in during invocation. I am not opposed to it - just trying to understand the implications better. We do turn on specific `LangOpts` (like `Objc`) for the lexer during the Scanning phase as can be seen here - https://github.com/llvm/llvm-project/blob/83646590afe222cfdd792514854549077e17b005/clang/lib/Lex/DependencyDirectivesScanner.cpp#L71-L79. I guess the general question is - is it acceptable to have the Scanner operating in a language standard different than the passed in language mode and different than the compiler language standard? https://github.com/llvm/llvm-project/pull/93753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Pass LangOpts from CompilerInstance to DependencyScanningWorker (PR #93753)
nishithshah2211 wrote: > You can have a project that has both C and C++ implementation files that end > up including the same header files from the C standard library. One can be > compiled under C11 (without separator support), the other under C++14 (with > separator support). Thanks. I had considered this very lightly, and in my mind, I thought that this would result in two separate passes of scanning - and so, two separate scanning services. Maybe I am wrong? Thank you for the additional context and bits of knowledge, super helpful. I'll put up a separate PR that does the following: 1. reverts the changes within this PR 2. checks if the lexer has `ParsingPreprocessorDirective` property `true` or `false` and allow for the numeric lexing to happen in that case. Let me know if it is preferable for history etc. to have two separate PRs - one for the revert and one for the lexer to relax the CPP14/C23 constraint during the preprocessing phase. https://github.com/llvm/llvm-project/pull/93753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] clang: Relax LangOpts checks when lexing quoted numbers during preprocessing (PR #95798)
@@ -2068,7 +2068,8 @@ bool Lexer::LexNumericConstant(Token &Result, const char
*CurPtr) {
}
// If we have a digit separator, continue.
- if (C == '\'' && (LangOpts.CPlusPlus14 || LangOpts.C23)) {
+ if (C == '\'' &&
+ (LangOpts.CPlusPlus14 || LangOpts.C23 || ParsingPreprocessorDirective)) {
nishithshah2211 wrote:
@AaronBallman @jansvoboda11 @cor3ntin wanted to continue more on this after a
long gap I took. How can we reach a consensus? Unfortunately, I am pretty new
to Clang and do not understand the implications of making a choice one way or
another (which is what this thread is about, so thank you all for nice
discussion!).
https://github.com/llvm/llvm-project/pull/95798
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
