[clang-tools-extra] [clangd] Avoid libFormat's objective-c guessing heuristic where possible (PR #84133)
https://github.com/HighCommander4 updated
https://github.com/llvm/llvm-project/pull/84133
>From 9c3a4ab81e931f43104fa0ac26de56595f43c6b4 Mon Sep 17 00:00:00 2001
From: Nathan Ridge
Date: Sun, 25 Feb 2024 19:55:10 -0500
Subject: [PATCH] [clangd] Avoid libFormat's objective-c guessing heuristic
where possible
This avoids a known libFormat bug where the heuristic can OOM on certain
large files (particularly single-header libraries such as miniaudio.h).
The OOM will still happen on affected files if you actually try to
format them (this is harder to avoid since the underlyting issue affects
the actual formatting logic, not just the language-guessing heuristic),
but at least it's avoided during non-modifying operations like hover, and
modifying operations that do local formatting like code completion.
Fixes https://github.com/clangd/clangd/issues/719
Fixes https://github.com/clangd/clangd/issues/1384
Fixes https://github.com/llvm/llvm-project/issues/70945
---
clang-tools-extra/clangd/ClangdServer.cpp | 10 ++---
clang-tools-extra/clangd/CodeComplete.cpp | 4 +-
clang-tools-extra/clangd/IncludeCleaner.cpp | 2 +-
clang-tools-extra/clangd/ParsedAST.cpp| 2 +-
clang-tools-extra/clangd/SourceCode.cpp | 16 +++-
clang-tools-extra/clangd/SourceCode.h | 6 ++-
clang-tools-extra/clangd/tool/Check.cpp | 2 +-
.../clangd/unittests/SourceCodeTests.cpp | 38 +++
8 files changed, 68 insertions(+), 12 deletions(-)
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp
b/clang-tools-extra/clangd/ClangdServer.cpp
index 2907e3ba3c303c..5790273d625ef1 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -523,7 +523,7 @@ void ClangdServer::formatFile(PathRef File,
std::optional Rng,
auto Action = [File = File.str(), Code = std::move(*Code),
Ranges = std::vector{RequestedRange},
CB = std::move(CB), this]() mutable {
-format::FormatStyle Style = getFormatStyleForFile(File, Code, TFS);
+format::FormatStyle Style = getFormatStyleForFile(File, Code, TFS, true);
tooling::Replacements IncludeReplaces =
format::sortIncludes(Style, Code, Ranges, File);
auto Changed = tooling::applyAllReplacements(Code, IncludeReplaces);
@@ -551,7 +551,7 @@ void ClangdServer::formatOnType(PathRef File, Position Pos,
auto Action = [File = File.str(), Code = std::move(*Code),
TriggerText = TriggerText.str(), CursorPos = *CursorPos,
CB = std::move(CB), this]() mutable {
-auto Style = getFormatStyleForFile(File, Code, TFS);
+auto Style = getFormatStyleForFile(File, Code, TFS, false);
std::vector Result;
for (const tooling::Replacement &R :
formatIncremental(Code, CursorPos, TriggerText, Style))
@@ -605,7 +605,7 @@ void ClangdServer::rename(PathRef File, Position Pos,
llvm::StringRef NewName,
if (Opts.WantFormat) {
auto Style = getFormatStyleForFile(File, InpAST->Inputs.Contents,
- *InpAST->Inputs.TFS);
+ *InpAST->Inputs.TFS, false);
llvm::Error Err = llvm::Error::success();
for (auto &E : R->GlobalChanges)
Err =
@@ -762,7 +762,7 @@ void ClangdServer::applyTweak(PathRef File, Range Sel,
StringRef TweakID,
for (auto &It : (*Effect)->ApplyEdits) {
Edit &E = It.second;
format::FormatStyle Style =
-getFormatStyleForFile(File, E.InitialCode, TFS);
+getFormatStyleForFile(File, E.InitialCode, TFS, false);
if (llvm::Error Err = reformatEdit(E, Style))
elog("Failed to format {0}: {1}", It.first(), std::move(Err));
}
@@ -825,7 +825,7 @@ void ClangdServer::findHover(PathRef File, Position Pos,
if (!InpAST)
return CB(InpAST.takeError());
format::FormatStyle Style = getFormatStyleForFile(
-File, InpAST->Inputs.Contents, *InpAST->Inputs.TFS);
+File, InpAST->Inputs.Contents, *InpAST->Inputs.TFS, false);
CB(clangd::getHover(InpAST->AST, Pos, std::move(Style), Index));
};
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp
b/clang-tools-extra/clangd/CodeComplete.cpp
index 0e5f08cec440ce..036eb9808ea082 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -1628,7 +1628,7 @@ class CodeCompleteFlow {
IsUsingDeclaration = Recorder->CCContext.isUsingDeclaration();
auto Style = getFormatStyleForFile(SemaCCInput.FileName,
SemaCCInput.ParseInput.Contents,
- *SemaCCInput.ParseInput.TFS);
+ *SemaCCInput.ParseInput.TFS, false);
const auto NextToken = findTokenAfterCompletionPoint(
Recorder->CCSema->getPreprocessor().getCodeCompletionLoc(),
Recorder->CCSema->getSourceManager(),
[clang-tools-extra] [clangd] Avoid libFormat's objective-c guessing heuristic where possible (PR #84133)
@@ -166,14 +166,29 @@ TextEdit toTextEdit(const FixItHint &FixIt, const
SourceManager &M,
std::optional getCanonicalPath(const FileEntryRef F,
FileManager &FileMgr);
+/// A flag passed to getFormatStyleForFile() that specifies what kind of
+/// formatting operation the returned FormatStyle will be used for.
+enum class FormatKind {
+ // Formatting a snippet of synthesized code (e.g. a code snippet
+ // shown in a hover) that's not part of the main file.
+ Snippet,
+ // Formatting edits made by an editor action such as code completion
+ // or rename.
+ Replacements,
+ // Formatting the entire main file (or a range selected by the user,
+ // which can be arbitrarily long).
+ EntireFileOrRange
HighCommander4 wrote:
Done, patch uses a bool flag now
https://github.com/llvm/llvm-project/pull/84133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Avoid libFormat's objective-c guessing heuristic where possible (PR #84133)
@@ -166,14 +166,29 @@ TextEdit toTextEdit(const FixItHint &FixIt, const
SourceManager &M,
std::optional getCanonicalPath(const FileEntryRef F,
FileManager &FileMgr);
+/// A flag passed to getFormatStyleForFile() that specifies what kind of
+/// formatting operation the returned FormatStyle will be used for.
+enum class FormatKind {
+ // Formatting a snippet of synthesized code (e.g. a code snippet
+ // shown in a hover) that's not part of the main file.
+ Snippet,
+ // Formatting edits made by an editor action such as code completion
+ // or rename.
+ Replacements,
+ // Formatting the entire main file (or a range selected by the user,
+ // which can be arbitrarily long).
+ EntireFileOrRange
+};
+
/// Choose the clang-format style we should apply to a certain file.
/// This will usually use FS to look for .clang-format directories.
/// FIXME: should we be caching the .clang-format file search?
/// This uses format::DefaultFormatStyle and format::DefaultFallbackStyle,
/// though the latter may have been overridden in main()!
format::FormatStyle getFormatStyleForFile(llvm::StringRef File,
HighCommander4 wrote:
Sure, I added a unit test which checks the detected language in various cases,
including checking that the new `FormatFile` flag successfully bypasses the
language guessing heuristic
https://github.com/llvm/llvm-project/pull/84133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] 3093d73 - [clangd] Avoid libFormat's objective-c guessing heuristic where possible (#84133)
Author: Nathan Ridge
Date: 2024-03-11T04:16:45-04:00
New Revision: 3093d731dff93df02899dcc62f5e7ba02461ff2a
URL:
https://github.com/llvm/llvm-project/commit/3093d731dff93df02899dcc62f5e7ba02461ff2a
DIFF:
https://github.com/llvm/llvm-project/commit/3093d731dff93df02899dcc62f5e7ba02461ff2a.diff
LOG: [clangd] Avoid libFormat's objective-c guessing heuristic where possible
(#84133)
This avoids a known libFormat bug where the heuristic can OOM on certain
large files (particularly single-header libraries such as miniaudio.h).
The OOM will still happen on affected files if you actually try to
format them (this is harder to avoid since the underlyting issue affects
the actual formatting logic, not just the language-guessing heuristic),
but at least it's avoided during non-modifying operations like hover,
and modifying operations that do local formatting like code completion.
Fixes https://github.com/clangd/clangd/issues/719
Fixes https://github.com/clangd/clangd/issues/1384
Fixes https://github.com/llvm/llvm-project/issues/70945
Added:
Modified:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/tool/Check.cpp
clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp
b/clang-tools-extra/clangd/ClangdServer.cpp
index 2907e3ba3c303c..5790273d625ef1 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -523,7 +523,7 @@ void ClangdServer::formatFile(PathRef File,
std::optional Rng,
auto Action = [File = File.str(), Code = std::move(*Code),
Ranges = std::vector{RequestedRange},
CB = std::move(CB), this]() mutable {
-format::FormatStyle Style = getFormatStyleForFile(File, Code, TFS);
+format::FormatStyle Style = getFormatStyleForFile(File, Code, TFS, true);
tooling::Replacements IncludeReplaces =
format::sortIncludes(Style, Code, Ranges, File);
auto Changed = tooling::applyAllReplacements(Code, IncludeReplaces);
@@ -551,7 +551,7 @@ void ClangdServer::formatOnType(PathRef File, Position Pos,
auto Action = [File = File.str(), Code = std::move(*Code),
TriggerText = TriggerText.str(), CursorPos = *CursorPos,
CB = std::move(CB), this]() mutable {
-auto Style = getFormatStyleForFile(File, Code, TFS);
+auto Style = getFormatStyleForFile(File, Code, TFS, false);
std::vector Result;
for (const tooling::Replacement &R :
formatIncremental(Code, CursorPos, TriggerText, Style))
@@ -605,7 +605,7 @@ void ClangdServer::rename(PathRef File, Position Pos,
llvm::StringRef NewName,
if (Opts.WantFormat) {
auto Style = getFormatStyleForFile(File, InpAST->Inputs.Contents,
- *InpAST->Inputs.TFS);
+ *InpAST->Inputs.TFS, false);
llvm::Error Err = llvm::Error::success();
for (auto &E : R->GlobalChanges)
Err =
@@ -762,7 +762,7 @@ void ClangdServer::applyTweak(PathRef File, Range Sel,
StringRef TweakID,
for (auto &It : (*Effect)->ApplyEdits) {
Edit &E = It.second;
format::FormatStyle Style =
-getFormatStyleForFile(File, E.InitialCode, TFS);
+getFormatStyleForFile(File, E.InitialCode, TFS, false);
if (llvm::Error Err = reformatEdit(E, Style))
elog("Failed to format {0}: {1}", It.first(), std::move(Err));
}
@@ -825,7 +825,7 @@ void ClangdServer::findHover(PathRef File, Position Pos,
if (!InpAST)
return CB(InpAST.takeError());
format::FormatStyle Style = getFormatStyleForFile(
-File, InpAST->Inputs.Contents, *InpAST->Inputs.TFS);
+File, InpAST->Inputs.Contents, *InpAST->Inputs.TFS, false);
CB(clangd::getHover(InpAST->AST, Pos, std::move(Style), Index));
};
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp
b/clang-tools-extra/clangd/CodeComplete.cpp
index 0e5f08cec440ce..036eb9808ea082 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -1628,7 +1628,7 @@ class CodeCompleteFlow {
IsUsingDeclaration = Recorder->CCContext.isUsingDeclaration();
auto Style = getFormatStyleForFile(SemaCCInput.FileName,
SemaCCInput.ParseInput.Contents,
- *SemaCCInput.ParseInput.TFS);
+ *SemaCCInput.ParseInput.TFS, false);
const auto NextToken = findTokenAfterCompletionPoint(
Recorder->CCSema->getPreprocessor().getCodeCompletionLoc(),
[clang-tools-extra] [clangd] Avoid libFormat's objective-c guessing heuristic where possible (PR #84133)
https://github.com/HighCommander4 closed https://github.com/llvm/llvm-project/pull/84133 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Comment parsing: add argument parsing for @throw @throws @exception (PR #84726)
https://github.com/hdoc created https://github.com/llvm/llvm-project/pull/84726
Doxygen allows for the @throw, @throws, and @exception commands to have an
attached argument indicating the type being thrown. Currently, Clang's AST
parsing doesn't support parsing out this argument from doc comments. The result
is missing compatibility with Doxygen.
We would find it helpful if the AST exposed these thrown types as
BlockCommandComment arguments so that we could generate better documentation.
This PR implements parsing of arguments for the @throw, @throws, and @exception
commands. Each command can only have one argument, matching the semantics of
Doxygen. We have also added unit tests to validate the functionality.
>From ec3f444913d9162de4494cdb09b336b1b00380fa Mon Sep 17 00:00:00 2001
From: hdoc
Date: Mon, 11 Mar 2024 01:13:25 -0700
Subject: [PATCH] Comment parsing: add argument parsing for @throw @throws
@exception
Doxygen allows for the @throw, @throws, and @exception commands to
have an attached argument indicating the type being thrown. Currently,
Clang's AST parsing doesn't support parsing out this argument from doc
comments. The result is missing compatibility with Doxygen.
We would find it helpful if the AST exposed these thrown types as
BlockCommandComment arguments so that we could generate better
documentation.
This PR implements parsing of arguments for the @throw, @throws, and
@exception commands. Each command can only have one argument, matching
the semantics of Doxygen. We have also added unit tests to validate
the functionality.
---
clang/include/clang/AST/CommentCommands.td | 6 +-
clang/include/clang/AST/CommentParser.h| 3 +
clang/lib/AST/CommentParser.cpp| 133
clang/unittests/AST/CommentParser.cpp | 235 -
4 files changed, 373 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/AST/CommentCommands.td
b/clang/include/clang/AST/CommentCommands.td
index e839031752cdd8..06b2fa9b5531c6 100644
--- a/clang/include/clang/AST/CommentCommands.td
+++ b/clang/include/clang/AST/CommentCommands.td
@@ -132,9 +132,9 @@ def Tparam : BlockCommand<"tparam"> { let IsTParamCommand =
1; }
// HeaderDoc command for template parameter documentation.
def Templatefield : BlockCommand<"templatefield"> { let IsTParamCommand = 1; }
-def Throws: BlockCommand<"throws"> { let IsThrowsCommand = 1; }
-def Throw : BlockCommand<"throw"> { let IsThrowsCommand = 1; }
-def Exception : BlockCommand<"exception"> { let IsThrowsCommand = 1; }
+def Throws: BlockCommand<"throws"> { let IsThrowsCommand = 1; let NumArgs
= 1; }
+def Throw : BlockCommand<"throw"> { let IsThrowsCommand = 1; let NumArgs =
1; }
+def Exception : BlockCommand<"exception"> { let IsThrowsCommand = 1; let
NumArgs = 1;}
def Deprecated : BlockCommand<"deprecated"> {
let IsEmptyParagraphAllowed = 1;
diff --git a/clang/include/clang/AST/CommentParser.h
b/clang/include/clang/AST/CommentParser.h
index e11e818b1af0a1..5884a25d007851 100644
--- a/clang/include/clang/AST/CommentParser.h
+++ b/clang/include/clang/AST/CommentParser.h
@@ -100,6 +100,9 @@ class Parser {
ArrayRef
parseCommandArgs(TextTokenRetokenizer &Retokenizer, unsigned NumArgs);
+ ArrayRef
+ parseThrowCommandArgs(TextTokenRetokenizer &Retokenizer, unsigned NumArgs);
+
BlockCommandComment *parseBlockCommand();
InlineCommandComment *parseInlineCommand();
diff --git a/clang/lib/AST/CommentParser.cpp b/clang/lib/AST/CommentParser.cpp
index 8adfd85d0160c3..c70fa1b05cb241 100644
--- a/clang/lib/AST/CommentParser.cpp
+++ b/clang/lib/AST/CommentParser.cpp
@@ -75,6 +75,25 @@ class TextTokenRetokenizer {
return *Pos.BufferPtr;
}
+ char peekNext(unsigned offset) const {
+assert(!isEnd());
+assert(Pos.BufferPtr != Pos.BufferEnd);
+if (Pos.BufferPtr + offset <= Pos.BufferEnd) {
+ return *(Pos.BufferPtr + offset);
+} else {
+ return '\0';
+}
+ }
+
+ void peekNextToken(SmallString<32> &WordText) const {
+unsigned offset = 1;
+char C = peekNext(offset++);
+while (!isWhitespace(C) && C != '\0') {
+ WordText.push_back(C);
+ C = peekNext(offset++);
+}
+ }
+
void consumeChar() {
assert(!isEnd());
assert(Pos.BufferPtr != Pos.BufferEnd);
@@ -89,6 +108,29 @@ class TextTokenRetokenizer {
}
}
+ /// Extract a template type
+ bool lexTemplateType(SmallString<32> &WordText) {
+unsigned IncrementCounter = 0;
+while (!isEnd()) {
+ const char C = peek();
+ WordText.push_back(C);
+ consumeChar();
+ switch (C) {
+ default:
+break;
+ case '<': {
+IncrementCounter++;
+ } break;
+ case '>': {
+IncrementCounter--;
+if (!IncrementCounter)
+ return true;
+ } break;
+ }
+}
+return false;
+ }
+
/// Add a token.
/// Returns true on success, false if there are no interesting tokens to
/// fetch from lexer.
[clang] Comment parsing: add argument parsing for @throw @throws @exception (PR #84726)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/84726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Limit how much work guessLanguage() can do (PR #78925)
https://github.com/HighCommander4 closed https://github.com/llvm/llvm-project/pull/78925 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Limit how much work guessLanguage() can do (PR #78925)
HighCommander4 wrote: > Thanks. I've written the clangd patch and posted it for review at #84133. > > If that gets approved, I will abandon this PR. #84133 has merged now. I'll accordingly close this. Thank you @owenca for your feedback and helping reach a solution that accomplished the goals of this patch in a claner way! https://github.com/llvm/llvm-project/pull/78925 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Comment parsing: add argument parsing for @throw @throws @exception (PR #84726)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: hdoc (hdoc)
Changes
Doxygen allows for the @throw, @throws, and @exception
commands to have an attached argument indicating the type being thrown.
Currently, Clang's AST parsing doesn't support parsing out this argument from
doc comments. The result is missing compatibility with Doxygen.
We would find it helpful if the AST exposed these thrown types as
BlockCommandComment arguments so that we could generate better documentation.
This PR implements parsing of arguments for the @throw, @throws, and @exception commands. Each command can only have one
argument, matching the semantics of Doxygen. We have also added unit tests to
validate the functionality.
---
Full diff: https://github.com/llvm/llvm-project/pull/84726.diff
4 Files Affected:
- (modified) clang/include/clang/AST/CommentCommands.td (+3-3)
- (modified) clang/include/clang/AST/CommentParser.h (+3)
- (modified) clang/lib/AST/CommentParser.cpp (+133)
- (modified) clang/unittests/AST/CommentParser.cpp (+234-1)
``diff
diff --git a/clang/include/clang/AST/CommentCommands.td
b/clang/include/clang/AST/CommentCommands.td
index e839031752cdd8..06b2fa9b5531c6 100644
--- a/clang/include/clang/AST/CommentCommands.td
+++ b/clang/include/clang/AST/CommentCommands.td
@@ -132,9 +132,9 @@ def Tparam : BlockCommand<"tparam"> { let IsTParamCommand =
1; }
// HeaderDoc command for template parameter documentation.
def Templatefield : BlockCommand<"templatefield"> { let IsTParamCommand = 1; }
-def Throws: BlockCommand<"throws"> { let IsThrowsCommand = 1; }
-def Throw : BlockCommand<"throw"> { let IsThrowsCommand = 1; }
-def Exception : BlockCommand<"exception"> { let IsThrowsCommand = 1; }
+def Throws: BlockCommand<"throws"> { let IsThrowsCommand = 1; let NumArgs
= 1; }
+def Throw : BlockCommand<"throw"> { let IsThrowsCommand = 1; let NumArgs =
1; }
+def Exception : BlockCommand<"exception"> { let IsThrowsCommand = 1; let
NumArgs = 1;}
def Deprecated : BlockCommand<"deprecated"> {
let IsEmptyParagraphAllowed = 1;
diff --git a/clang/include/clang/AST/CommentParser.h
b/clang/include/clang/AST/CommentParser.h
index e11e818b1af0a1..5884a25d007851 100644
--- a/clang/include/clang/AST/CommentParser.h
+++ b/clang/include/clang/AST/CommentParser.h
@@ -100,6 +100,9 @@ class Parser {
ArrayRef
parseCommandArgs(TextTokenRetokenizer &Retokenizer, unsigned NumArgs);
+ ArrayRef
+ parseThrowCommandArgs(TextTokenRetokenizer &Retokenizer, unsigned NumArgs);
+
BlockCommandComment *parseBlockCommand();
InlineCommandComment *parseInlineCommand();
diff --git a/clang/lib/AST/CommentParser.cpp b/clang/lib/AST/CommentParser.cpp
index 8adfd85d0160c3..c70fa1b05cb241 100644
--- a/clang/lib/AST/CommentParser.cpp
+++ b/clang/lib/AST/CommentParser.cpp
@@ -75,6 +75,25 @@ class TextTokenRetokenizer {
return *Pos.BufferPtr;
}
+ char peekNext(unsigned offset) const {
+assert(!isEnd());
+assert(Pos.BufferPtr != Pos.BufferEnd);
+if (Pos.BufferPtr + offset <= Pos.BufferEnd) {
+ return *(Pos.BufferPtr + offset);
+} else {
+ return '\0';
+}
+ }
+
+ void peekNextToken(SmallString<32> &WordText) const {
+unsigned offset = 1;
+char C = peekNext(offset++);
+while (!isWhitespace(C) && C != '\0') {
+ WordText.push_back(C);
+ C = peekNext(offset++);
+}
+ }
+
void consumeChar() {
assert(!isEnd());
assert(Pos.BufferPtr != Pos.BufferEnd);
@@ -89,6 +108,29 @@ class TextTokenRetokenizer {
}
}
+ /// Extract a template type
+ bool lexTemplateType(SmallString<32> &WordText) {
+unsigned IncrementCounter = 0;
+while (!isEnd()) {
+ const char C = peek();
+ WordText.push_back(C);
+ consumeChar();
+ switch (C) {
+ default:
+break;
+ case '<': {
+IncrementCounter++;
+ } break;
+ case '>': {
+IncrementCounter--;
+if (!IncrementCounter)
+ return true;
+ } break;
+ }
+}
+return false;
+ }
+
/// Add a token.
/// Returns true on success, false if there are no interesting tokens to
/// fetch from lexer.
@@ -149,6 +191,76 @@ class TextTokenRetokenizer {
addToken();
}
+ /// Extract a type argument
+ bool lexDataType(Token &Tok) {
+if (isEnd())
+ return false;
+Position SavedPos = Pos;
+consumeWhitespace();
+SmallString<32> NextToken;
+SmallString<32> WordText;
+const char *WordBegin = Pos.BufferPtr;
+SourceLocation Loc = getSourceLocation();
+StringRef ConstVal = StringRef("const");
+bool ConstPointer = false;
+
+while (!isEnd()) {
+ const char C = peek();
+ if (!isWhitespace(C)) {
+if (C == '<') {
+ if (!lexTemplateType(WordText))
+return false;
+} else {
+ WordText.push_back(C);
+ consumeChar();
+}
+ } else {
+if (WordText.equals(ConstVal)) {
+ Wo
[clang] [Clang][Comments] Add argument parsing for @throw @throws @exception (PR #84726)
https://github.com/hdoc edited https://github.com/llvm/llvm-project/pull/84726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format][NFC] Eliminate the IsCpp parameter in all functions (PR #84599)
https://github.com/mydeveloperday approved this pull request. This looks good to me https://github.com/llvm/llvm-project/pull/84599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)
ChuanqiXu9 wrote: This commit breaks our coroutines library async_simple `https://github.com/alibaba/async_simple` and here is a (relative) minimal reproducer: https://godbolt.org/z/sG5jzcGEz The reproducer comes from an implementation for async_simple::Generator (https://github.com/alibaba/async_simple/blob/main/async_simple/coro/Generator.h), which is basically the same implementation from the paper of `std::generator` in c++23. So possibly this may make that crash too. I'll revert this patch to avoid the regression now. Sorry for not testing it sufficiently. https://github.com/llvm/llvm-project/pull/84193 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] placement new initializes typedef array with correct size (PR #83124)
mahtohappy wrote: Ping @AaronBallman @cor3ntin @Endilll https://github.com/llvm/llvm-project/pull/83124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 0f501c3 - Revert "[C++20][Coroutines] Lambda-coroutine with operator new in promise_type (#84193)"
Author: Chuanqi Xu
Date: 2024-03-11T17:02:43+08:00
New Revision: 0f501c30b9601627c236f9abca8a3befba5dc161
URL:
https://github.com/llvm/llvm-project/commit/0f501c30b9601627c236f9abca8a3befba5dc161
DIFF:
https://github.com/llvm/llvm-project/commit/0f501c30b9601627c236f9abca8a3befba5dc161.diff
LOG: Revert "[C++20][Coroutines] Lambda-coroutine with operator new in
promise_type (#84193)"
This reverts commit 35d3b33ba5c9b90443ac985f2521b78f84b611fe.
See the comments in https://github.com/llvm/llvm-project/pull/84193 for
details
Added:
Modified:
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaCoroutine.cpp
clang/lib/Sema/SemaExprCXX.cpp
Removed:
clang/test/SemaCXX/gh84064-1.cpp
clang/test/SemaCXX/gh84064-2.cpp
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 00b3f53f5c1c66..267c79cc057cba 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6752,18 +6752,10 @@ class Sema final {
SourceLocation RParenLoc);
ActOnCXXThis - Parse 'this' pointer.
- ///
- /// \param ThisRefersToClosureObject Whether to skip the 'this' check for a
- /// lambda because 'this' refers to the closure object.
- ExprResult ActOnCXXThis(SourceLocation loc,
- bool ThisRefersToClosureObject = false);
+ ExprResult ActOnCXXThis(SourceLocation loc);
/// Build a CXXThisExpr and mark it referenced in the current context.
- ///
- /// \param ThisRefersToClosureObject Whether to skip the 'this' check for a
- /// lambda because 'this' refers to the closure object.
- Expr *BuildCXXThisExpr(SourceLocation Loc, QualType Type, bool IsImplicit,
- bool ThisRefersToClosureObject = false);
+ Expr *BuildCXXThisExpr(SourceLocation Loc, QualType Type, bool IsImplicit);
void MarkThisReferenced(CXXThisExpr *This);
/// Try to retrieve the type of the 'this' pointer.
diff --git a/clang/lib/Sema/SemaCoroutine.cpp
b/clang/lib/Sema/SemaCoroutine.cpp
index 5206fc7621c7cd..736632857efc36 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -25,7 +25,6 @@
#include "clang/Sema/Initialization.h"
#include "clang/Sema/Overload.h"
#include "clang/Sema/ScopeInfo.h"
-#include "clang/Sema/Sema.h"
#include "clang/Sema/SemaInternal.h"
#include "llvm/ADT/SmallSet.h"
@@ -1291,21 +1290,8 @@ bool CoroutineStmtBuilder::makeReturnOnAllocFailure() {
static bool collectPlacementArgs(Sema &S, FunctionDecl &FD, SourceLocation Loc,
SmallVectorImpl &PlacementArgs) {
if (auto *MD = dyn_cast(&FD)) {
-if (MD->isImplicitObjectMemberFunction()) {
- ExprResult ThisExpr{};
-
- if (isLambdaCallOperator(MD) && !MD->isStatic()) {
-Qualifiers ThisQuals = MD->getMethodQualifiers();
-CXXRecordDecl *Record = MD->getParent();
-
-Sema::CXXThisScopeRAII ThisScope(S, Record, ThisQuals,
- Record != nullptr);
-
-ThisExpr = S.ActOnCXXThis(Loc, /*ThisRefersToClosureObject=*/true);
- } else {
-ThisExpr = S.ActOnCXXThis(Loc);
- }
-
+if (MD->isImplicitObjectMemberFunction() && !isLambdaCallOperator(MD)) {
+ ExprResult ThisExpr = S.ActOnCXXThis(Loc);
if (ThisExpr.isInvalid())
return false;
ThisExpr = S.CreateBuiltinUnaryOp(Loc, UO_Deref, ThisExpr.get());
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 88e3d9ced044cb..c34a40fa7c81ac 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1414,8 +1414,7 @@ bool Sema::CheckCXXThisCapture(SourceLocation Loc, const
bool Explicit,
return false;
}
-ExprResult Sema::ActOnCXXThis(SourceLocation Loc,
- bool ThisRefersToClosureObject) {
+ExprResult Sema::ActOnCXXThis(SourceLocation Loc) {
/// C++ 9.3.2: In the body of a non-static member function, the keyword this
/// is a non-lvalue expression whose value is the address of the object for
/// which the function is called.
@@ -1435,18 +1434,13 @@ ExprResult Sema::ActOnCXXThis(SourceLocation Loc,
return Diag(Loc, diag::err_invalid_this_use) << 0;
}
- return BuildCXXThisExpr(Loc, ThisTy, /*IsImplicit=*/false,
- ThisRefersToClosureObject);
+ return BuildCXXThisExpr(Loc, ThisTy, /*IsImplicit=*/false);
}
-Expr *Sema::BuildCXXThisExpr(SourceLocation Loc, QualType Type, bool
IsImplicit,
- bool ThisRefersToClosureObject) {
+Expr *Sema::BuildCXXThisExpr(SourceLocation Loc, QualType Type,
+ bool IsImplicit) {
auto *This = CXXThisExpr::Create(Context, Loc, Type, IsImplicit);
-
- if (!ThisRefersToClosureObject) {
-MarkThisReferenced(This);
- }
-
+ MarkThisReferenced(This);
return This;
}
diff --git a/clang/
[clang] [Clang][C++23] Implement P2448R2: Relaxing some constexpr restrictions (PR #77753)
Fznamznon wrote: > to fail, which I think is not intentional? No, it is not. Fails only for c++23 now. I'm looking. https://github.com/llvm/llvm-project/pull/77753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)
andreasfertig wrote: Interesting! I'll see what I can do. https://github.com/llvm/llvm-project/pull/84193 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Add argument parsing for @throw @throws @exception (PR #84726)
https://github.com/hdoc edited https://github.com/llvm/llvm-project/pull/84726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Mention possibility of underflow in array overflow errors (PR #84201)
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
@@ -83,6 +83,8 @@ class StateUpdateReporter {
AssumedUpperBound = UpperBoundVal;
}
+ bool assumedNonNegative() { return AssumedNonNegative; }
NagyDonat wrote:
I'd prefer to keep the current name because it's more natural if we're looking
at this from the outside (the get/has variants are awkward because
`AssumedNonNegative` is not a noun phrase); and it's just an implementation
detail that this is a getter for a private data member.
https://github.com/llvm/llvm-project/pull/84201
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Mention possibility of underflow in array overflow errors (PR #84201)
=?utf-8?q?Donát?= Nagy ,NagyDonat
Message-ID:
In-Reply-To:
https://github.com/NagyDonat updated
https://github.com/llvm/llvm-project/pull/84201
>From 74c7c04cb456c0a058ca36a2ca4ffa4d70d576e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?=
Date: Wed, 6 Mar 2024 18:09:02 +0100
Subject: [PATCH 1/3] [analyzer] Mention possibility of underflow in array
overflow errors
The checker alpha.security.ArrayBoundV2 performs bounds checking in two
steps: first it checks for underflow, and if it isn't guaranteed then it
assumes that there is no underflow. After this, it checks for overflow,
and if it's guaranteed or the index is tainted then it reports it.
This meant that in situations where overflow and underflow are both
possible (but the index is either tainted or guaranteed to be invalid),
the checker was reporting an overflow error.
This commit modifies the messages printed in these cases to mention the
possibility of an underflow.
---
.../Checkers/ArrayBoundCheckerV2.cpp | 37 +++-
.../test/Analysis/out-of-bounds-diagnostics.c | 56 ++-
.../test/Analysis/taint-diagnostic-visitor.c | 2 +-
3 files changed, 80 insertions(+), 15 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
index 29eb932584027d..664d26b00a3cab 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -83,6 +83,8 @@ class StateUpdateReporter {
AssumedUpperBound = UpperBoundVal;
}
+ bool assumedNonNegative() { return AssumedNonNegative; }
+
const NoteTag *createNoteTag(CheckerContext &C) const;
private:
@@ -402,7 +404,8 @@ static bool tryDividePair(std::optional &Val1,
}
static Messages getExceedsMsgs(ASTContext &ACtx, const SubRegion *Region,
- NonLoc Offset, NonLoc Extent, SVal Location) {
+ NonLoc Offset, NonLoc Extent, SVal Location,
+ bool AssumedNonNegative) {
std::string RegName = getRegionName(Region);
const auto *EReg = Location.getAsRegion()->getAs();
assert(EReg && "this checker only handles element access");
@@ -414,6 +417,7 @@ static Messages getExceedsMsgs(ASTContext &ACtx, const
SubRegion *Region,
int64_t ElemSize = ACtx.getTypeSizeInChars(ElemType).getQuantity();
bool UseByteOffsets = !tryDividePair(OffsetN, ExtentN, ElemSize);
+ const char *OffsetOrIndex = UseByteOffsets ? "byte offset" : "index";
SmallString<256> Buf;
llvm::raw_svector_ostream Out(Buf);
@@ -421,10 +425,12 @@ static Messages getExceedsMsgs(ASTContext &ACtx, const
SubRegion *Region,
if (!ExtentN && !UseByteOffsets)
Out << "'" << ElemType.getAsString() << "' element in ";
Out << RegName << " at ";
- if (OffsetN) {
-Out << (UseByteOffsets ? "byte offset " : "index ") << *OffsetN;
+ if (AssumedNonNegative) {
+Out << "a negative or overflowing " << OffsetOrIndex;
+ } else if (OffsetN) {
+Out << OffsetOrIndex << " " << *OffsetN;
} else {
-Out << "an overflowing " << (UseByteOffsets ? "byte offset" : "index");
+Out << "an overflowing " << OffsetOrIndex;
}
if (ExtentN) {
Out << ", while it holds only ";
@@ -441,17 +447,19 @@ static Messages getExceedsMsgs(ASTContext &ACtx, const
SubRegion *Region,
Out << "s";
}
- return {
- formatv("Out of bound access to memory after the end of {0}", RegName),
- std::string(Buf)};
+ return {formatv("Out of bound access to memory {0} {1}",
+ AssumedNonNegative ? "around" : "after the end of", RegName),
+ std::string(Buf)};
}
-static Messages getTaintMsgs(const SubRegion *Region, const char *OffsetName) {
+static Messages getTaintMsgs(const SubRegion *Region, const char *OffsetName,
+ bool AssumedNonNegative) {
std::string RegName = getRegionName(Region);
return {formatv("Potential out of bound access to {0} with tainted {1}",
RegName, OffsetName),
- formatv("Access of {0} with a tainted {1} that may be too large",
- RegName, OffsetName)};
+ formatv("Access of {0} with a tainted {1} that may be {2}too large",
+ RegName, OffsetName,
+ AssumedNonNegative ? "negative or " : "")};
}
const NoteTag *StateUpdateReporter::createNoteTag(CheckerContext &C) const {
@@ -603,6 +611,8 @@ void ArrayBoundCheckerV2::performCheck(const Expr *E,
CheckerContext &C) const {
auto [WithinUpperBound, ExceedsUpperBound] =
compareValueToThreshold(State, ByteOffset, *KnownSize, SVB);
+bool AssumedNonNegative = SUR.assumedNonNegative();
+
if (ExceedsUpperBound) {
// The offset may be invalid (>= Size)...
if (!WithinUpperBound) {
@@ -615,8 +625,9 @@ void ArrayBoundCheckerV2::performCheck(const Expr *E,
CheckerContext &C) const {
[clang] [Clang][Sema] Allow access to a public template alias declaration that refers to friend's private nested type (PR #83847)
jcsxky wrote: gently ping~ https://github.com/llvm/llvm-project/pull/83847 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [WebAssembly] Implement an alternative translation for -wasm-enable-sjlj (PR #84137)
yamt wrote: > > Given that we don't need `setjmpTableSize` anymore and `setjmpTable` > > doesn't change, we don't need the whole block here from line 1463 ~ line > > 1503 doing SSA updates anymore: > > i get errors like the following if i simply put the SSA update things in > `!EnableWasmAltSjLj` block. i need to investigate. > > ``` > spacetanuki% /Volumes/PortableSSD/llvm/build/bin/clang --sysroot > /opt/wasi-sdk-21.0/share/wasi-sysroot -resource-dir > /Volumes/PortableSSD/llvm/llvm/lib/clang/17 --target=wasm32-wasi -Os -c > -mllvm -wasm-enable-sjlj -mllvm -experimental-wasm-enable-alt-sjlj a.c > Instruction does not dominate all uses! > %val = load i32, ptr %val_gep, align 4 > %setjmp.ret = phi i32 [ 0, %entry.split ], [ %val, %setjmp.dispatch ] > in function f > fatal error: error in backend: Broken function found, compilation aborted! > PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ > and include the crash backtrace, preprocessed source, and associated run > script. > Stack dump: > 0. Program arguments: /Volumes/PortableSSD/llvm/build/bin/clang > --sysroot /opt/wasi-sdk-21.0/share/wasi-sysroot -resource-dir > /Volumes/PortableSSD/llvm/llvm/lib/clang/17 --target=wasm32-wasi -Os -c > -mllvm -wasm-enable-sjlj -mllvm -experimental-wasm-enable-alt-sjlj a.c > 1. parser at end of file > 2. Code generation > 3. Running pass 'Function Pass Manager' on module 'a.c'. > 4. Running pass 'Module Verifier' on function '@f' > ``` i think i found the cause. rebuildSSA is still necessary to propagate "val". https://github.com/llvm/llvm-project/pull/84137 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add clang-tidy check readability-math-missing-parentheses (PR #84481)
https://github.com/11happy updated
https://github.com/llvm/llvm-project/pull/84481
>From 8fdf6306085ed4cf0f77b7e718e374e9f65fedf9 Mon Sep 17 00:00:00 2001
From: 11happy
Date: Fri, 8 Mar 2024 19:02:47 +0530
Subject: [PATCH 1/2] add clang-tidy check readability-math-missing-parentheses
Signed-off-by: 11happy
---
.../clang-tidy/readability/CMakeLists.txt | 1 +
.../MathMissingParenthesesCheck.cpp | 167 ++
.../readability/MathMissingParenthesesCheck.h | 31
.../readability/ReadabilityTidyModule.cpp | 3 +
clang-tools-extra/docs/ReleaseNotes.rst | 6 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../readability/math-missing-parentheses.rst | 19 ++
.../readability/math-missing-parentheses.cpp | 42 +
8 files changed, 270 insertions(+)
create mode 100644
clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/readability/math-missing-parentheses.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index a6c8cbd8eb448a..0d4fa095501dfb 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -27,6 +27,7 @@ add_clang_library(clangTidyReadabilityModule
IsolateDeclarationCheck.cpp
MagicNumbersCheck.cpp
MakeMemberFunctionConstCheck.cpp
+ MathMissingParenthesesCheck.cpp
MisleadingIndentationCheck.cpp
MisplacedArrayIndexCheck.cpp
NamedParameterCheck.cpp
diff --git
a/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp
b/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp
new file mode 100644
index 00..d9574a9fb7a476
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp
@@ -0,0 +1,167 @@
+//===--- MathMissingParenthesesCheck.cpp - clang-tidy
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MathMissingParenthesesCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+void MathMissingParenthesesCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(binaryOperator(unless(hasParent(binaryOperator())),
+hasDescendant(binaryOperator()))
+ .bind("binOp"),
+ this);
+}
+static int precedenceCheck(const char op) {
+ if (op == '/' || op == '*' || op == '%')
+return 5;
+
+ else if (op == '+' || op == '-')
+return 4;
+
+ else if (op == '&')
+return 3;
+ else if (op == '^')
+return 2;
+
+ else if (op == '|')
+return 1;
+
+ else
+return 0;
+}
+static bool isOperand(const char c) {
+ if (c >= 'a' && c <= 'z')
+return true;
+ else if (c >= 'A' && c <= 'Z')
+return true;
+ else if (c >= '0' && c <= '9')
+return true;
+ else if (c == '$')
+return true;
+ else
+return false;
+}
+static bool conditionForNegative(const std::string s, int i,
+ const std::string CurStr) {
+ if (CurStr[0] == '-') {
+if (i == 0) {
+ return true;
+} else {
+ while (s[i - 1] == ' ') {
+i--;
+ }
+ if (!isOperand(s[i - 1])) {
+return true;
+ } else {
+return false;
+ }
+}
+ } else {
+return false;
+ }
+}
+static std::string getOperationOrder(std::string s, std::set &Operators)
{
+ std::stack StackOne;
+ std::string TempStr = "";
+ for (int i = 0; i < s.length(); i++) {
+std::string CurStr = "";
+CurStr += s[i];
+if (CurStr == " ")
+ continue;
+else {
+ if (isOperand(CurStr[0]) || conditionForNegative(s, i, CurStr)) {
+while (i < s.length() && (isOperand(s[i]) || s[i] == '-')) {
+ if (s[i] == '-') {
+TempStr += "$";
+ } else {
+TempStr += CurStr;
+ }
+ i++;
+ CurStr = s[i];
+}
+TempStr += " ";
+ } else if (CurStr == "(") {
+StackOne.push("(");
+ } else if (CurStr == ")") {
+while (StackOne.top() != "(") {
+ TempStr += StackOne.top();
+ StackOne.pop();
+}
+StackOne.pop();
+ } else {
+while (!StackOne.empty() && precedenceCheck
[clang] Add new flag -Wreturn-mismatch (PR #82872)
11happy wrote: Are there any more changes for this PR required from my end? Thank you https://github.com/llvm/llvm-project/pull/82872 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add clang-tidy check readability-math-missing-parentheses (PR #84481)
@@ -0,0 +1,167 @@
+//===--- MathMissingParenthesesCheck.cpp - clang-tidy
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MathMissingParenthesesCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+void MathMissingParenthesesCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(binaryOperator(unless(hasParent(binaryOperator())),
+hasDescendant(binaryOperator()))
+ .bind("binOp"),
+ this);
+}
+static int precedenceCheck(const char op) {
+ if (op == '/' || op == '*' || op == '%')
+return 5;
+
+ else if (op == '+' || op == '-')
+return 4;
+
+ else if (op == '&')
+return 3;
+ else if (op == '^')
+return 2;
+
+ else if (op == '|')
+return 1;
+
+ else
+return 0;
+}
+static bool isOperand(const char c) {
11happy wrote:
done
https://github.com/llvm/llvm-project/pull/84481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add clang-tidy check readability-math-missing-parentheses (PR #84481)
@@ -0,0 +1,167 @@
+//===--- MathMissingParenthesesCheck.cpp - clang-tidy
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MathMissingParenthesesCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+void MathMissingParenthesesCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(binaryOperator(unless(hasParent(binaryOperator())),
+hasDescendant(binaryOperator()))
+ .bind("binOp"),
+ this);
+}
+static int precedenceCheck(const char op) {
+ if (op == '/' || op == '*' || op == '%')
+return 5;
+
+ else if (op == '+' || op == '-')
+return 4;
+
+ else if (op == '&')
+return 3;
+ else if (op == '^')
+return 2;
+
+ else if (op == '|')
+return 1;
+
+ else
+return 0;
+}
11happy wrote:
done
https://github.com/llvm/llvm-project/pull/84481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add clang-tidy check readability-math-missing-parentheses (PR #84481)
@@ -0,0 +1,167 @@
+//===--- MathMissingParenthesesCheck.cpp - clang-tidy
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MathMissingParenthesesCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+void MathMissingParenthesesCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(binaryOperator(unless(hasParent(binaryOperator())),
+hasDescendant(binaryOperator()))
+ .bind("binOp"),
+ this);
+}
+static int precedenceCheck(const char op) {
+ if (op == '/' || op == '*' || op == '%')
+return 5;
+
+ else if (op == '+' || op == '-')
+return 4;
+
+ else if (op == '&')
+return 3;
+ else if (op == '^')
+return 2;
+
+ else if (op == '|')
+return 1;
+
+ else
+return 0;
+}
+static bool isOperand(const char c) {
+ if (c >= 'a' && c <= 'z')
+return true;
+ else if (c >= 'A' && c <= 'Z')
+return true;
+ else if (c >= '0' && c <= '9')
+return true;
+ else if (c == '$')
+return true;
+ else
+return false;
+}
+static bool conditionForNegative(const std::string s, int i,
+ const std::string CurStr) {
+ if (CurStr[0] == '-') {
+if (i == 0) {
+ return true;
+} else {
+ while (s[i - 1] == ' ') {
+i--;
+ }
+ if (!isOperand(s[i - 1])) {
+return true;
+ } else {
+return false;
+ }
+}
+ } else {
+return false;
+ }
+}
+static std::string getOperationOrder(std::string s, std::set &Operators)
{
11happy wrote:
done
https://github.com/llvm/llvm-project/pull/84481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add clang-tidy check readability-math-missing-parentheses (PR #84481)
@@ -0,0 +1,167 @@
+//===--- MathMissingParenthesesCheck.cpp - clang-tidy
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MathMissingParenthesesCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+void MathMissingParenthesesCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(binaryOperator(unless(hasParent(binaryOperator())),
+hasDescendant(binaryOperator()))
+ .bind("binOp"),
+ this);
+}
+static int precedenceCheck(const char op) {
+ if (op == '/' || op == '*' || op == '%')
+return 5;
+
+ else if (op == '+' || op == '-')
+return 4;
+
+ else if (op == '&')
+return 3;
+ else if (op == '^')
+return 2;
+
+ else if (op == '|')
+return 1;
+
+ else
+return 0;
+}
+static bool isOperand(const char c) {
+ if (c >= 'a' && c <= 'z')
+return true;
+ else if (c >= 'A' && c <= 'Z')
+return true;
+ else if (c >= '0' && c <= '9')
+return true;
+ else if (c == '$')
+return true;
+ else
+return false;
+}
+static bool conditionForNegative(const std::string s, int i,
+ const std::string CurStr) {
+ if (CurStr[0] == '-') {
+if (i == 0) {
+ return true;
+} else {
+ while (s[i - 1] == ' ') {
+i--;
+ }
+ if (!isOperand(s[i - 1])) {
+return true;
+ } else {
+return false;
+ }
+}
+ } else {
+return false;
+ }
+}
+static std::string getOperationOrder(std::string s, std::set &Operators)
{
+ std::stack StackOne;
+ std::string TempStr = "";
+ for (int i = 0; i < s.length(); i++) {
+std::string CurStr = "";
+CurStr += s[i];
+if (CurStr == " ")
+ continue;
+else {
+ if (isOperand(CurStr[0]) || conditionForNegative(s, i, CurStr)) {
+while (i < s.length() && (isOperand(s[i]) || s[i] == '-')) {
+ if (s[i] == '-') {
+TempStr += "$";
+ } else {
+TempStr += CurStr;
+ }
+ i++;
+ CurStr = s[i];
+}
+TempStr += " ";
+ } else if (CurStr == "(") {
+StackOne.push("(");
+ } else if (CurStr == ")") {
+while (StackOne.top() != "(") {
+ TempStr += StackOne.top();
+ StackOne.pop();
+}
+StackOne.pop();
+ } else {
+while (!StackOne.empty() && precedenceCheck(CurStr[0]) <=
+precedenceCheck((StackOne.top())[0])) {
+ TempStr += StackOne.top();
+ StackOne.pop();
+}
+StackOne.push(CurStr);
+ }
+}
+ }
+ while (!StackOne.empty()) {
+TempStr += StackOne.top();
+StackOne.pop();
+ }
+ std::stack StackTwo;
+ for (int i = 0; i < TempStr.length(); i++) {
+if (TempStr[i] == ' ')
+ continue;
+else if (isOperand(TempStr[i])) {
+ std::string CurStr = "";
+ while (i < TempStr.length() && isOperand(TempStr[i])) {
+if (TempStr[i] == '$') {
+ CurStr += "-";
+} else {
+ CurStr += TempStr[i];
+}
+i++;
+ }
+ StackTwo.push(CurStr);
+} else {
+ std::string OperandOne = StackTwo.top();
+ StackTwo.pop();
+ std::string OperandTwo = StackTwo.top();
+ StackTwo.pop();
+ Operators.insert(TempStr[i]);
+ StackTwo.push("(" + OperandTwo + " " + TempStr[i] + " " + OperandOne +
+")");
+}
+ }
+ return StackTwo.top();
+}
+void MathMissingParenthesesCheck::check(
+const MatchFinder::MatchResult &Result) {
+ const auto *BinOp = Result.Nodes.getNodeAs("binOp");
+ if (!BinOp)
+return;
+ clang::SourceManager &SM = *Result.SourceManager;
+ clang::LangOptions LO = Result.Context->getLangOpts();
+ clang::CharSourceRange Range =
+ clang::CharSourceRange::getTokenRange(BinOp->getSourceRange());
+ std::string Expression = clang::Lexer::getSourceText(Range, SM, LO).str();
11happy wrote:
done
https://github.com/llvm/llvm-project/pull/84481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add clang-tidy check readability-math-missing-parentheses (PR #84481)
@@ -100,6 +100,12 @@ Improvements to clang-tidy New checks ^^ +- New :doc:`readability-math-missing-parentheses + ` check. + + Checks for mathematical expressions that involve operators + of different priorities. 11happy wrote: done https://github.com/llvm/llvm-project/pull/84481 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add clang-tidy check readability-math-missing-parentheses (PR #84481)
@@ -0,0 +1,42 @@
+// RUN: %check_clang_tidy %s readability-math-missing-parentheses %t
+
+// FIXME: Add something that triggers the check here.
+void f(){
+//CHECK-MESSAGES: :[[@LINE+2]]:13: warning: add parantheses to clarify the
precedence of operations [readability-math-missing-parentheses]
+//CHECK-FIXES: int a = 1 + (2 * 3);
+int a = 1 + 2 * 3;
+
+int b = 1 + 2 + 3; // No warning
+
+int c = 1 * 2 * 3; // No warning
+
+//CHECK-MESSAGES: :[[@LINE+2]]:13: warning: add parantheses to clarify the
precedence of operations [readability-math-missing-parentheses]
+//CHECK-FIXES: int d = (1 + (2 * 3)) - (4 / 5);
+int d = 1 + 2 * 3 - 4 / 5;
+
+//CHECK-MESSAGES: :[[@LINE+2]]:13: warning: add parantheses to clarify the
precedence of operations [readability-math-missing-parentheses]
+//CHECK-FIXES: int e = (1 & (2 + 3)) | (4 * 5);
+int e = 1 & 2 + 3 | 4 * 5;
+
+//CHECK-MESSAGES: :[[@LINE+2]]:13: warning: add parantheses to clarify the
precedence of operations [readability-math-missing-parentheses]
+//CHECK-FIXES: int f = (1 * -2) + 4;
+int f = 1 * -2 + 4;
+
+//CHECK-MESSAGES: :[[@LINE+2]]:13: warning: add parantheses to clarify the
precedence of operations [readability-math-missing-parentheses]
+//CHECK-FIXES: int g = (((1 * 2) * 3) + 4) + 5;
+int g = 1 * 2 * 3 + 4 + 5;
+
+// CHECK-MESSAGES: :[[@LINE+2]]:13: warning: add parantheses to clarify
the precedence of operations [readability-math-missing-parentheses]
+// CHECK-FIXES: int h = (120 & (2 + 3)) | (22 * 5);
+int h = 120 & 2 + 3 | 22 * 5;
+
+int i = 1 & 2 & 3; // No warning
+
+int j = 1 | 2 | 3; // No warning
+
+int k = 1 ^ 2 ^ 3; // No warning
+
+// CHECK-MESSAGES: :[[@LINE+2]]:13: warning: add parantheses to clarify
the precedence of operations [readability-math-missing-parentheses]
+// CHECK-FIXES: int l = (1 + 2) ^ 3;
+int l = 1 + 2 ^ 3;
+}
11happy wrote:
added tests
https://github.com/llvm/llvm-project/pull/84481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix OMPT ident flag in combined distribute parallel for pragma (PR #80987)
mikaoP wrote: Tests passing now https://github.com/llvm/llvm-project/pull/80987 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][C++23] Implement P2448R2: Relaxing some constexpr restrictions (PR #77753)
Fznamznon wrote:
Ok, now I looked at it a bit more, and I think this is expected due to the
change and how clang behaved before this patch. After this patch implicit
destructor of S, i.e. `~S` becomes `constexpr`. It used to be not `constexpr`
because neither `P` nor `B` had `constexpr` destructors. clang defines and
emits errors for constexpr defaulted members due to this bit here:
https://github.com/llvm/llvm-project/blob/0ef61ed54dca2e974928c55b2144b57d4c4ff621/clang/lib/Sema/SemaDeclCXX.cpp#L7225
So, once I manually change this example to force `~S()` to be `constexpr`,
older clang produces the same error:
```
template struct P {
cosntexpr ~P() { p->f(); } // add constexpr
T * p;
};
struct X;
struct B { virtual constexpr ~B(); }; // add constexpr
struct S: B { P x; }; // now all bases and members have constexpr
destructors, ~S() is constexpr
```
Demo https://godbolt.org/z/rT3bWMdqz (note I took 18.1.0 clang which doesn't
have P2448R2 implementation).
I'm not quite sure the error itself is correct, I've got to educate myself
about standard matters a bit more to understand this, but no other compiler
errors on this https://godbolt.org/z/Ehhn5sno7.
cc @AaronBallman , @cor3ntin, @zygoloid in case you can weigh in here
https://github.com/llvm/llvm-project/pull/77753
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [WebAssembly] Implement an alternative translation for -wasm-enable-sjlj (PR #84137)
https://github.com/yamt updated https://github.com/llvm/llvm-project/pull/84137
>From 1283ae6b5536810f8fbe183eda80997aa9f5cdc3 Mon Sep 17 00:00:00 2001
From: YAMAMOTO Takashi
Date: Fri, 9 Feb 2024 15:49:55 +0900
Subject: [PATCH 1/7] [WebAssembly] Implement an alternative translation for
-wasm-enable-sjlj
Instead of maintaining per-function-invocation malloc()'ed tables to
track which functions each label belongs to, store the equivalent info
in jump buffers (jmp_buf) themselves.
Also, use a less emscripten-looking ABI symbols:
saveSetjmp -> __wasm_sjlj_setjmp
testSetjmp -> __wasm_sjlj_test
getTempRet0-> (removed)
__wasm_longjmp -> __wasm_sjlj_longjmp
Enabled with:
-mllvm -wasm-enable-sjlj -mllvm -experimental-wasm-enable-alt-sjlj
(-experimental-wasm-enable-alt-sjlj is the new option this change
introduces.)
While I want to use this for WASI, it should work for emscripten as well.
An example runtime and a few tests:
https://github.com/yamt/garbage/tree/wasm-sjlj-alt2/wasm/longjmp
Discussion:
https://docs.google.com/document/d/1ZvTPT36K5jjiedF8MCXbEmYjULJjI723aOAks1IdLLg/edit
---
clang/lib/Driver/ToolChains/WebAssembly.cpp | 14 ++
.../MCTargetDesc/WebAssemblyMCTargetDesc.cpp | 3 +
.../MCTargetDesc/WebAssemblyMCTargetDesc.h| 1 +
.../WebAssemblyLowerEmscriptenEHSjLj.cpp | 174 +++---
.../WebAssembly/WebAssemblyTargetMachine.cpp | 4 +
5 files changed, 131 insertions(+), 65 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/WebAssembly.cpp
b/clang/lib/Driver/ToolChains/WebAssembly.cpp
index b8c2573d6265fb..2e7c8e6e8d13f7 100644
--- a/clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ b/clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -386,6 +386,20 @@ void WebAssembly::addClangTargetOptions(const ArgList
&DriverArgs,
// Backend needs '-exception-model=wasm' to use Wasm EH instructions
CC1Args.push_back("-exception-model=wasm");
}
+
+if (Opt.starts_with("-experimental-wasm-enable-alt-sjlj")) {
+ // '-mllvm -experimental-wasm-enable-alt-sjlj' should be used with
+ // '-mllvm -wasm-enable-sjlj'
+ bool HasWasmEnableSjlj = false;
+ for (const Arg *A : DriverArgs.filtered(options::OPT_mllvm)) {
+if (StringRef(A->getValue(0)) == "-wasm-enable-sjlj")
+ HasWasmEnableSjlj = true;
+ }
+ if (!HasWasmEnableSjlj)
+getDriver().Diag(diag::err_drv_argument_only_allowed_with)
+<< "-mllvm -experimental-wasm-enable-alt-sjlj"
+<< "-mllvm -wasm-enable-sjlj";
+}
}
}
diff --git
a/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.cpp
b/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.cpp
index e8f58a19d25e3b..7f15742367be09 100644
--- a/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.cpp
+++ b/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.cpp
@@ -54,6 +54,9 @@ cl::opt
// setjmp/longjmp handling using wasm EH instrutions
cl::opt WebAssembly::WasmEnableSjLj(
"wasm-enable-sjlj", cl::desc("WebAssembly setjmp/longjmp handling"));
+cl::opt WebAssembly::WasmEnableAltSjLj(
+"experimental-wasm-enable-alt-sjlj",
+cl::desc("Use experimental alternate ABI for --wasm-enable-sjlj"));
static MCAsmInfo *createMCAsmInfo(const MCRegisterInfo & /*MRI*/,
const Triple &TT,
diff --git a/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h
b/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h
index 15aeaaeb8c4a4e..d23de9d407d894 100644
--- a/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h
+++ b/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h
@@ -44,6 +44,7 @@ extern cl::opt WasmEnableEmEH; // asm.js-style EH
extern cl::opt WasmEnableEmSjLj; // asm.js-style SjLJ
extern cl::opt WasmEnableEH; // EH using Wasm EH instructions
extern cl::opt WasmEnableSjLj; // SjLj using Wasm EH instructions
+extern cl::opt WasmEnableAltSjLj; // Alt ABI for WasmEnableSjLj
enum OperandType {
/// Basic block label in a branch construct.
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
b/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
index 77e6640d5a8224..fc76757011f5d8 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
@@ -300,6 +300,7 @@ class WebAssemblyLowerEmscriptenEHSjLj final : public
ModulePass {
bool EnableEmEH; // Enable Emscripten exception handling
bool EnableEmSjLj; // Enable Emscripten setjmp/longjmp handling
bool EnableWasmSjLj; // Enable Wasm setjmp/longjmp handling
+ bool EnableWasmAltSjLj; // Alt ABI for EnableWasmSjLj
bool DoSjLj; // Whether we actually perform setjmp/longjmp handling
GlobalVariable *ThrewGV = nullptr; // __THREW__ (Emscripten)
@@ -368,7 +369,8 @@ class WebAssemblyLowerEm
[clang] [clang-format] adds a space after not inside macros (PR #78176)
ilya-biryukov wrote: I just wanted to bring up that using `WhitespaceSensitiveMacros` here is a little tricky because the macro name one would need to put there is `V` (the name is taken directly from the code that hit this). Having a more sophisticated option to say "parameter `V` of macro `MyComplicatedMacro` is a string-sensitive macro" should work better, but seems like an overkill. https://github.com/llvm/llvm-project/pull/78176 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] ec2875c - [clang-repl] Expose RuntimeInterfaceBuilder to allow customization (#83126)
Author: Stefan Gränitz
Date: 2024-03-11T12:15:11+01:00
New Revision: ec2875ce2690010f7dd894c9b56802297dd6cb84
URL:
https://github.com/llvm/llvm-project/commit/ec2875ce2690010f7dd894c9b56802297dd6cb84
DIFF:
https://github.com/llvm/llvm-project/commit/ec2875ce2690010f7dd894c9b56802297dd6cb84.diff
LOG: [clang-repl] Expose RuntimeInterfaceBuilder to allow customization (#83126)
RuntimeInterfaceBuilder wires up JITed expressions with the hardcoded
Interpreter runtime. It's used only for value printing right now, but it
is not limited to that. The default implementation focuses on an
evaluation process where the Interpreter has direct access to the memory
of JITed expressions (in-process execution or shared memory).
We need a different approach to support out-of-process evaluation or
variations of the runtime. It seems reasonable to expose a minimal
interface for it. The new RuntimeInterfaceBuilder is an abstract base
class in the public header. For that, the TypeVisitor had to become a
component (instead of inheriting from it). FindRuntimeInterface() was
adjusted to return an instance of the RuntimeInterfaceBuilder and it can
be overridden from derived classes.
Added:
clang/unittests/Interpreter/InterpreterExtensionsTest.cpp
Modified:
clang/include/clang/Interpreter/Interpreter.h
clang/lib/Interpreter/Interpreter.cpp
clang/unittests/Interpreter/CMakeLists.txt
Removed:
diff --git a/clang/include/clang/Interpreter/Interpreter.h
b/clang/include/clang/Interpreter/Interpreter.h
index c8f932e95c4798..d972d960dcb7cd 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -18,6 +18,7 @@
#include "clang/AST/GlobalDecl.h"
#include "clang/Interpreter/PartialTranslationUnit.h"
#include "clang/Interpreter/Value.h"
+#include "clang/Sema/Ownership.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ExecutionEngine/JITSymbol.h"
@@ -75,17 +76,26 @@ class IncrementalCompilerBuilder {
llvm::StringRef CudaSDKPath;
};
+/// Generate glue code between the Interpreter's built-in runtime and user
code.
+class RuntimeInterfaceBuilder {
+public:
+ virtual ~RuntimeInterfaceBuilder() = default;
+
+ using TransformExprFunction = ExprResult(RuntimeInterfaceBuilder *Builder,
+ Expr *, ArrayRef);
+ virtual TransformExprFunction *getPrintValueTransformer() = 0;
+};
+
/// Provides top-level interfaces for incremental compilation and execution.
class Interpreter {
std::unique_ptr TSCtx;
std::unique_ptr IncrParser;
std::unique_ptr IncrExecutor;
+ std::unique_ptr RuntimeIB;
// An optional parser for CUDA offloading
std::unique_ptr DeviceParser;
- Interpreter(std::unique_ptr CI, llvm::Error &Err);
-
llvm::Error CreateExecutor();
unsigned InitPTUSize = 0;
@@ -94,8 +104,25 @@ class Interpreter {
// printing happens, it's in an invalid state.
Value LastValue;
+ // Add a call to an Expr to report its result. We query the function from
+ // RuntimeInterfaceBuilder once and store it as a function pointer to avoid
+ // frequent virtual function calls.
+ RuntimeInterfaceBuilder::TransformExprFunction *AddPrintValueCall = nullptr;
+
+protected:
+ // Derived classes can make use an extended interface of the Interpreter.
+ // That's useful for testing and out-of-tree clients.
+ Interpreter(std::unique_ptr CI, llvm::Error &Err);
+
+ // Lazily construct the RuntimeInterfaceBuilder. The provided instance will
be
+ // used for the entire lifetime of the interpreter. The default
implementation
+ // targets the in-process __clang_Interpreter runtime. Override this to use a
+ // custom runtime.
+ virtual std::unique_ptr FindRuntimeInterface();
+
public:
- ~Interpreter();
+ virtual ~Interpreter();
+
static llvm::Expected>
create(std::unique_ptr CI);
static llvm::Expected>
@@ -143,8 +170,6 @@ class Interpreter {
private:
size_t getEffectivePTUSize() const;
- bool FindRuntimeInterface();
-
llvm::DenseMap Dtors;
llvm::SmallVector ValuePrintingInfo;
diff --git a/clang/lib/Interpreter/Interpreter.cpp
b/clang/lib/Interpreter/Interpreter.cpp
index 37696b28976428..3485da8196683a 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -507,9 +507,13 @@ static constexpr llvm::StringRef MagicRuntimeInterface[] =
{
"__clang_Interpreter_SetValueWithAlloc",
"__clang_Interpreter_SetValueCopyArr", "__ci_newtag"};
-bool Interpreter::FindRuntimeInterface() {
+static std::unique_ptr
+createInProcessRuntimeInterfaceBuilder(Interpreter &Interp, ASTContext &Ctx,
+ Sema &S);
+
+std::unique_ptr Interpreter::FindRuntimeInterface() {
if (llvm::all_of(ValuePrintingInfo, [](Expr *E) { return E != nullptr; }))
-return true;
+return nullptr;
Sema &S = getCompilerInstance()->getSem
[clang] [clang-repl] Expose RuntimeInterfaceBuilder to allow customization (PR #83126)
https://github.com/weliveindetail closed https://github.com/llvm/llvm-project/pull/83126 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] adds a space after not inside macros (PR #78176)
@@ -4842,7 +4842,7 @@ bool TokenAnnotator::spaceRequiredBefore(const
AnnotatedLine &Line,
return true;
}
if (Left.is(TT_UnaryOperator)) {
-if (Right.isNot(tok::l_paren)) {
+if (!Right.isOneOf(tok::r_paren, tok::l_paren, tok::exclaim)) {
ilya-biryukov wrote:
Adding a space makes total sense in code that **has an argument** to `not`.
But this case seems special because `V(not!)` or `V(not)` clearly can't be a
full expression by themselves. So it feels preferable to not add any extra
spaces.
https://github.com/llvm/llvm-project/pull/78176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Add argument parsing for @throw @throws @exception (PR #84726)
@@ -149,6 +191,76 @@ class TextTokenRetokenizer {
addToken();
}
+ /// Extract a type argument
+ bool lexDataType(Token &Tok) {
+if (isEnd())
+ return false;
+Position SavedPos = Pos;
+consumeWhitespace();
+SmallString<32> NextToken;
+SmallString<32> WordText;
+const char *WordBegin = Pos.BufferPtr;
+SourceLocation Loc = getSourceLocation();
+StringRef ConstVal = StringRef("const");
sdkrystian wrote:
Shouldn't we support `volatile` as well?
https://github.com/llvm/llvm-project/pull/84726
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Fix crash when using name of UnresolvedUsingValueDecl with template arguments (PR #83842)
sdkrystian wrote:
@wlei-llvm Thank you! I've reduced the repro to this:
```cpp
struct A { };
template
void f(A);
struct B {
void f();
void g() {
f(A());
}
};
```
https://github.com/llvm/llvm-project/pull/83842
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [APINotes] Fix failing tests after a PCM logic change (PR #84556)
egorzhdan wrote: The clang-format CI failure is unrelated to this patch. https://github.com/llvm/llvm-project/pull/84556 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 878097d - [APINotes] Fix failing tests after a PCM logic change
Author: Egor Zhdan
Date: 2024-03-11T12:02:29Z
New Revision: 878097dff3ea4bad6b7f50017224a84bbf2af406
URL:
https://github.com/llvm/llvm-project/commit/878097dff3ea4bad6b7f50017224a84bbf2af406
DIFF:
https://github.com/llvm/llvm-project/commit/878097dff3ea4bad6b7f50017224a84bbf2af406.diff
LOG: [APINotes] Fix failing tests after a PCM logic change
This fixes tests that are going to be upstreamed in the near future.
Currently they are failing downstream in the Apple open source fork.
Failing tests
Clang :: APINotes/retain-count-convention.m
Clang :: APINotes/types.m
Clang :: APINotes/versioned-multi.c
Clang :: APINotes/versioned.m
Since 2e5af56 got merged, Clang now enables `LangOpts.APINotesModules`
when reading a precompiled module that was built with API Notes enabled.
This is correct. The logic in APINotesManager needs to be adjusted to
handle this.
rdar://123526142
Added:
Modified:
clang/lib/APINotes/APINotesManager.cpp
Removed:
diff --git a/clang/lib/APINotes/APINotesManager.cpp
b/clang/lib/APINotes/APINotesManager.cpp
index d3aef09dac9105..f60f09e2b3c231 100644
--- a/clang/lib/APINotes/APINotesManager.cpp
+++ b/clang/lib/APINotes/APINotesManager.cpp
@@ -224,7 +224,7 @@ APINotesManager::getCurrentModuleAPINotes(Module *M, bool
LookInModule,
llvm::SmallVector APINotes;
// First, look relative to the module itself.
- if (LookInModule) {
+ if (LookInModule && M->Directory) {
// Local function to try loading an API notes file in the given directory.
auto tryAPINotes = [&](DirectoryEntryRef Dir, bool WantPublic) {
if (auto File = findAPINotesFile(Dir, ModuleName, WantPublic)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [APINotes] Fix failing tests after a PCM logic change (PR #84556)
https://github.com/egorzhdan closed https://github.com/llvm/llvm-project/pull/84556 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Fix JSON conversion for symbol tags (PR #84747)
https://github.com/ckandeler created
https://github.com/llvm/llvm-project/pull/84747
The wrong constructor of json::Value got called, making every tag an array
instead of a number.
>From f67994902314acd8ae0f0c561b07b8c014172e17 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Mon, 11 Mar 2024 13:04:21 +0100
Subject: [PATCH] [clangd] Fix JSON conversion for symbol tags
The wrong constructor of json::Value got called, making every tag an
array instead of a number.
---
clang-tools-extra/clangd/Protocol.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/Protocol.cpp
b/clang-tools-extra/clangd/Protocol.cpp
index 8aa18bb0058abe..c6553e00dcae28 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -1412,7 +1412,7 @@ bool fromJSON(const llvm::json::Value &Params,
ReferenceParams &R,
}
llvm::json::Value toJSON(SymbolTag Tag) {
- return llvm::json::Value{static_cast(Tag)};
+ return llvm::json::Value(static_cast(Tag));
}
llvm::json::Value toJSON(const CallHierarchyItem &I) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Fix JSON conversion for symbol tags (PR #84747)
llvmbot wrote:
@llvm/pr-subscribers-clangd
@llvm/pr-subscribers-clang-tools-extra
Author: Christian Kandeler (ckandeler)
Changes
The wrong constructor of json::Value got called, making every tag an array
instead of a number.
---
Full diff: https://github.com/llvm/llvm-project/pull/84747.diff
1 Files Affected:
- (modified) clang-tools-extra/clangd/Protocol.cpp (+1-1)
``diff
diff --git a/clang-tools-extra/clangd/Protocol.cpp
b/clang-tools-extra/clangd/Protocol.cpp
index 8aa18bb0058abe..c6553e00dcae28 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -1412,7 +1412,7 @@ bool fromJSON(const llvm::json::Value &Params,
ReferenceParams &R,
}
llvm::json::Value toJSON(SymbolTag Tag) {
- return llvm::json::Value{static_cast(Tag)};
+ return llvm::json::Value(static_cast(Tag));
}
llvm::json::Value toJSON(const CallHierarchyItem &I) {
``
https://github.com/llvm/llvm-project/pull/84747
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Fix JSON conversion for symbol tags (PR #84747)
https://github.com/sam-mccall closed https://github.com/llvm/llvm-project/pull/84747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] 9b2386e - [clangd] Fix JSON conversion for symbol tags (#84747)
Author: Christian Kandeler
Date: 2024-03-11T13:16:58+01:00
New Revision: 9b2386e82dedafade233c8871637ee76da9ebe0e
URL:
https://github.com/llvm/llvm-project/commit/9b2386e82dedafade233c8871637ee76da9ebe0e
DIFF:
https://github.com/llvm/llvm-project/commit/9b2386e82dedafade233c8871637ee76da9ebe0e.diff
LOG: [clangd] Fix JSON conversion for symbol tags (#84747)
The wrong constructor of json::Value got called, making every tag an
array instead of a number.
Added:
Modified:
clang-tools-extra/clangd/Protocol.cpp
Removed:
diff --git a/clang-tools-extra/clangd/Protocol.cpp
b/clang-tools-extra/clangd/Protocol.cpp
index 8aa18bb0058abe..c6553e00dcae28 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -1412,7 +1412,7 @@ bool fromJSON(const llvm::json::Value &Params,
ReferenceParams &R,
}
llvm::json::Value toJSON(SymbolTag Tag) {
- return llvm::json::Value{static_cast(Tag)};
+ return llvm::json::Value(static_cast(Tag));
}
llvm::json::Value toJSON(const CallHierarchyItem &I) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Fix JSON conversion for symbol tags (PR #84747)
sam-mccall wrote: Thanks! (And sorry, that json::Value constructor is my fault too...) https://github.com/llvm/llvm-project/pull/84747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Improve BlockInCriticalSectionsChecker (PR #80029)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/80029 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Improve BlockInCriticalSectionsChecker (PR #80029)
https://github.com/NagyDonat commented: Seems to be a good change overall and I like that you implemented this with some nice helper classes. However, there is some code duplication that could be reduced (see inline comments). Also, I feel that (especially after these extensions) this checker duplicates some logic that also appears in `PthreadLockChecker.cpp`. I didn't check the exact details but that checker is also modeling the locking and unlocking calls in order to report bad locking/unlocking order. In theory it would be to ensure that we don't develop and maintain two similar but different solutions that both track lock handling code -- but PthreadLockChecker is 700 lines long and I understand that it would be difficult to unify it with this checker. https://github.com/llvm/llvm-project/pull/80029 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Improve BlockInCriticalSectionsChecker (PR #80029)
@@ -20,48 +20,180 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringExtras.h"
+
+#include
+#include
+#include
using namespace clang;
using namespace ento;
namespace {
+
+struct CritSectionMarker {
+ const Expr *LockExpr{};
+ const MemRegion *LockReg{};
+
+ void Profile(llvm::FoldingSetNodeID &ID) const {
+ID.Add(LockExpr);
+ID.Add(LockReg);
+ }
+
+ [[nodiscard]] constexpr bool
+ operator==(const CritSectionMarker &Other) const noexcept {
+return LockExpr == Other.LockExpr && LockReg == Other.LockReg;
+ }
+ [[nodiscard]] constexpr bool
+ operator!=(const CritSectionMarker &Other) const noexcept {
+return !(*this == Other);
+ }
+};
+
+class FirstArgMutexDescriptor {
+ CallDescription LockFn;
+ CallDescription UnlockFn;
+
+public:
+ FirstArgMutexDescriptor(CallDescription &&LockFn, CallDescription &&UnlockFn)
+ : LockFn(std::move(LockFn)), UnlockFn(std::move(UnlockFn)) {}
+ [[nodiscard]] bool matchesLock(const CallEvent &Call) const {
+return LockFn.matches(Call) && Call.getNumArgs() > 0;
+ }
+ [[nodiscard]] bool matchesUnlock(const CallEvent &Call) const {
+return UnlockFn.matches(Call) && Call.getNumArgs() > 0;
+ }
+ [[nodiscard]] const MemRegion *getLockRegion(const CallEvent &Call) const {
+return Call.getArgSVal(0).getAsRegion();
+ }
+ [[nodiscard]] const MemRegion *getUnlockRegion(const CallEvent &Call) const {
+return Call.getArgSVal(0).getAsRegion();
+ }
+};
+
+class MemberMutexDescriptor {
+ CallDescription LockFn;
+ CallDescription UnlockFn;
+
+public:
+ MemberMutexDescriptor(CallDescription &&LockFn, CallDescription &&UnlockFn)
+ : LockFn(std::move(LockFn)), UnlockFn(std::move(UnlockFn)) {}
+ [[nodiscard]] bool matchesLock(const CallEvent &Call) const {
+return LockFn.matches(Call);
+ }
+ bool matchesUnlock(const CallEvent &Call) const {
+return UnlockFn.matches(Call);
+ }
+ [[nodiscard]] const MemRegion *getLockRegion(const CallEvent &Call) const {
+return cast(Call).getCXXThisVal().getAsRegion();
+ }
+ [[nodiscard]] const MemRegion *getUnlockRegion(const CallEvent &Call) const {
+return cast(Call).getCXXThisVal().getAsRegion();
+ }
+};
NagyDonat wrote:
Would it be possible to reduce the code duplication between these by
introducing a common ancestor class?
https://github.com/llvm/llvm-project/pull/80029
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Improve BlockInCriticalSectionsChecker (PR #80029)
@@ -20,48 +20,180 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringExtras.h"
+
+#include
+#include
+#include
using namespace clang;
using namespace ento;
namespace {
+
+struct CritSectionMarker {
+ const Expr *LockExpr{};
+ const MemRegion *LockReg{};
+
+ void Profile(llvm::FoldingSetNodeID &ID) const {
+ID.Add(LockExpr);
+ID.Add(LockReg);
+ }
+
+ [[nodiscard]] constexpr bool
+ operator==(const CritSectionMarker &Other) const noexcept {
+return LockExpr == Other.LockExpr && LockReg == Other.LockReg;
+ }
+ [[nodiscard]] constexpr bool
+ operator!=(const CritSectionMarker &Other) const noexcept {
+return !(*this == Other);
+ }
+};
+
+class FirstArgMutexDescriptor {
+ CallDescription LockFn;
+ CallDescription UnlockFn;
+
+public:
+ FirstArgMutexDescriptor(CallDescription &&LockFn, CallDescription &&UnlockFn)
+ : LockFn(std::move(LockFn)), UnlockFn(std::move(UnlockFn)) {}
+ [[nodiscard]] bool matchesLock(const CallEvent &Call) const {
+return LockFn.matches(Call) && Call.getNumArgs() > 0;
+ }
+ [[nodiscard]] bool matchesUnlock(const CallEvent &Call) const {
+return UnlockFn.matches(Call) && Call.getNumArgs() > 0;
+ }
+ [[nodiscard]] const MemRegion *getLockRegion(const CallEvent &Call) const {
+return Call.getArgSVal(0).getAsRegion();
+ }
+ [[nodiscard]] const MemRegion *getUnlockRegion(const CallEvent &Call) const {
+return Call.getArgSVal(0).getAsRegion();
+ }
+};
+
+class MemberMutexDescriptor {
+ CallDescription LockFn;
+ CallDescription UnlockFn;
+
+public:
+ MemberMutexDescriptor(CallDescription &&LockFn, CallDescription &&UnlockFn)
+ : LockFn(std::move(LockFn)), UnlockFn(std::move(UnlockFn)) {}
+ [[nodiscard]] bool matchesLock(const CallEvent &Call) const {
+return LockFn.matches(Call);
+ }
+ bool matchesUnlock(const CallEvent &Call) const {
+return UnlockFn.matches(Call);
+ }
+ [[nodiscard]] const MemRegion *getLockRegion(const CallEvent &Call) const {
+return cast(Call).getCXXThisVal().getAsRegion();
+ }
+ [[nodiscard]] const MemRegion *getUnlockRegion(const CallEvent &Call) const {
+return cast(Call).getCXXThisVal().getAsRegion();
+ }
+};
+
+class RAIIMutexDescriptor {
+ mutable const IdentifierInfo *Guard{};
+ mutable bool IdentifierInfoInitialized{};
+ mutable llvm::SmallString<32> GuardName{};
+
+ void initIdentifierInfo(const CallEvent &Call) const {
+if (!IdentifierInfoInitialized) {
+ // In case of checking C code, or when the corresponding headers are not
+ // included, we might end up query the identifier table every time when
+ // this function is called instead of early returning it. To avoid this,
a
+ // bool variable (IdentifierInfoInitialized) is used and the function
will
+ // be run only once.
+ Guard = &Call.getCalleeAnalysisDeclContext()->getASTContext().Idents.get(
+ GuardName);
+ IdentifierInfoInitialized = true;
+}
+ }
+
+public:
+ RAIIMutexDescriptor(StringRef GuardName) : GuardName(GuardName) {}
+ [[nodiscard]] bool matchesLock(const CallEvent &Call) const {
+initIdentifierInfo(Call);
+const auto *Ctor = dyn_cast(&Call);
+if (!Ctor)
+ return false;
+auto *IdentifierInfo = Ctor->getDecl()->getParent()->getIdentifier();
+return IdentifierInfo == Guard;
+ }
+ [[nodiscard]] bool matchesUnlock(const CallEvent &Call) const {
+initIdentifierInfo(Call);
+const auto *Dtor = dyn_cast(&Call);
+if (!Dtor)
+ return false;
+auto *IdentifierInfo =
+cast(Dtor->getDecl()->getParent())->getIdentifier();
+return IdentifierInfo == Guard;
+ }
NagyDonat wrote:
The methods `matchesLock()` and `matchesUnlock()` are a bit complex and almost
identical, consider implementing them as calling
`matchesImpl(Call)` and
`matchesImpl(Call)` with a template function that contains
the shared logic.
https://github.com/llvm/llvm-project/pull/80029
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Improve BlockInCriticalSectionsChecker (PR #80029)
@@ -70,73 +202,121 @@ class BlockInCriticalSectionChecker : public
Checker {
} // end anonymous namespace
-REGISTER_TRAIT_WITH_PROGRAMSTATE(MutexCounter, unsigned)
+REGISTER_LIST_WITH_PROGRAMSTATE(ActiveCritSections, CritSectionMarker)
-void BlockInCriticalSectionChecker::initIdentifierInfo(ASTContext &Ctx) const {
- if (!IdentifierInfoInitialized) {
-/* In case of checking C code, or when the corresponding headers are not
- * included, we might end up query the identifier table every time when
this
- * function is called instead of early returning it. To avoid this, a bool
- * variable (IdentifierInfoInitialized) is used and the function will be
run
- * only once. */
-IILockGuard = &Ctx.Idents.get(ClassLockGuard);
-IIUniqueLock = &Ctx.Idents.get(ClassUniqueLock);
-IdentifierInfoInitialized = true;
- }
-}
+namespace std {
+// Iterator traits for ImmutableList data structure
+// that enable the use of STL algorithms.
+// TODO: Move these to llvm::ImmutableList when overhauling immutable data
+// structures for proper iterator concept support.
+template <>
+struct iterator_traits<
+typename llvm::ImmutableList::iterator> {
+ using iterator_category = std::forward_iterator_tag;
+ using value_type = CritSectionMarker;
+ using difference_type = std::ptrdiff_t;
+ using reference = CritSectionMarker &;
+ using pointer = CritSectionMarker *;
+};
+} // namespace std
-bool BlockInCriticalSectionChecker::isBlockingFunction(const CallEvent &Call)
const {
- return matchesAny(Call, SleepFn, GetcFn, FgetsFn, ReadFn, RecvFn);
+std::optional
+BlockInCriticalSectionChecker::checkLock(const CallEvent &Call,
+ CheckerContext &C) const {
+ const auto *LockDescriptor =
+ llvm::find_if(MutexDescriptors, [&Call](auto &&LockFn) {
+return std::visit(
+[&Call](auto &&Descriptor) { return Descriptor.matchesLock(Call);
},
+LockFn);
+ });
+ if (LockDescriptor != MutexDescriptors.end())
+return *LockDescriptor;
+ return std::nullopt;
}
-bool BlockInCriticalSectionChecker::isLockFunction(const CallEvent &Call)
const {
- if (const auto *Ctor = dyn_cast(&Call)) {
-auto IdentifierInfo = Ctor->getDecl()->getParent()->getIdentifier();
-if (IdentifierInfo == IILockGuard || IdentifierInfo == IIUniqueLock)
- return true;
- }
+void BlockInCriticalSectionChecker::handleLock(
+const MutexDescriptor &LockDescriptor, const CallEvent &Call,
+CheckerContext &C) const {
+ const auto *MutexRegion = std::visit(
+ [&Call](auto &&Descriptor) { return Descriptor.getLockRegion(Call); },
+ LockDescriptor);
+ if (!MutexRegion)
+return;
+
+ const auto MarkToAdd = CritSectionMarker{Call.getOriginExpr(), MutexRegion};
+ ProgramStateRef StateWithLockEvent =
+ C.getState()->add(MarkToAdd);
+ C.addTransition(StateWithLockEvent, createCritSectionNote(MarkToAdd, C));
+}
- return matchesAny(Call, LockFn, PthreadLockFn, PthreadTryLockFn, MtxLock,
-MtxTimedLock, MtxTryLock);
+std::optional
+BlockInCriticalSectionChecker::checkUnlock(const CallEvent &Call,
+ CheckerContext &C) const {
+ const auto *UnlockDescriptor =
+ llvm::find_if(MutexDescriptors, [&Call](auto &&UnlockFn) {
+return std::visit(
+[&Call](auto &&Descriptor) {
+ return Descriptor.matchesUnlock(Call);
+},
+UnlockFn);
+ });
+ if (UnlockDescriptor != MutexDescriptors.end())
+return *UnlockDescriptor;
+ return std::nullopt;
NagyDonat wrote:
This is an almost exact duplicate of `checkLock()`. It would be better to unify
them into a single function (e.g. `checkLockUnlock` that takes a boolean
argument `IsLock` (and then also unify `matchesLock`/`matchesUnlock` into a
single function that takes a boolean).
I know that boolean arguments can be a code smell, but in this particular case
it would be a much better solution than this code duplication.
https://github.com/llvm/llvm-project/pull/80029
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Improve BlockInCriticalSectionsChecker (PR #80029)
@@ -20,48 +20,180 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringExtras.h"
+
+#include
+#include
+#include
using namespace clang;
using namespace ento;
namespace {
+
+struct CritSectionMarker {
+ const Expr *LockExpr{};
+ const MemRegion *LockReg{};
+
+ void Profile(llvm::FoldingSetNodeID &ID) const {
+ID.Add(LockExpr);
+ID.Add(LockReg);
+ }
+
+ [[nodiscard]] constexpr bool
+ operator==(const CritSectionMarker &Other) const noexcept {
+return LockExpr == Other.LockExpr && LockReg == Other.LockReg;
+ }
+ [[nodiscard]] constexpr bool
+ operator!=(const CritSectionMarker &Other) const noexcept {
+return !(*this == Other);
+ }
+};
+
+class FirstArgMutexDescriptor {
+ CallDescription LockFn;
+ CallDescription UnlockFn;
+
+public:
+ FirstArgMutexDescriptor(CallDescription &&LockFn, CallDescription &&UnlockFn)
+ : LockFn(std::move(LockFn)), UnlockFn(std::move(UnlockFn)) {}
+ [[nodiscard]] bool matchesLock(const CallEvent &Call) const {
+return LockFn.matches(Call) && Call.getNumArgs() > 0;
+ }
+ [[nodiscard]] bool matchesUnlock(const CallEvent &Call) const {
+return UnlockFn.matches(Call) && Call.getNumArgs() > 0;
+ }
+ [[nodiscard]] const MemRegion *getLockRegion(const CallEvent &Call) const {
+return Call.getArgSVal(0).getAsRegion();
+ }
+ [[nodiscard]] const MemRegion *getUnlockRegion(const CallEvent &Call) const {
+return Call.getArgSVal(0).getAsRegion();
+ }
+};
+
+class MemberMutexDescriptor {
+ CallDescription LockFn;
+ CallDescription UnlockFn;
+
+public:
+ MemberMutexDescriptor(CallDescription &&LockFn, CallDescription &&UnlockFn)
+ : LockFn(std::move(LockFn)), UnlockFn(std::move(UnlockFn)) {}
+ [[nodiscard]] bool matchesLock(const CallEvent &Call) const {
+return LockFn.matches(Call);
+ }
+ bool matchesUnlock(const CallEvent &Call) const {
+return UnlockFn.matches(Call);
+ }
+ [[nodiscard]] const MemRegion *getLockRegion(const CallEvent &Call) const {
+return cast(Call).getCXXThisVal().getAsRegion();
+ }
+ [[nodiscard]] const MemRegion *getUnlockRegion(const CallEvent &Call) const {
+return cast(Call).getCXXThisVal().getAsRegion();
+ }
+};
+
+class RAIIMutexDescriptor {
+ mutable const IdentifierInfo *Guard{};
+ mutable bool IdentifierInfoInitialized{};
+ mutable llvm::SmallString<32> GuardName{};
+
+ void initIdentifierInfo(const CallEvent &Call) const {
+if (!IdentifierInfoInitialized) {
+ // In case of checking C code, or when the corresponding headers are not
+ // included, we might end up query the identifier table every time when
+ // this function is called instead of early returning it. To avoid this,
a
+ // bool variable (IdentifierInfoInitialized) is used and the function
will
+ // be run only once.
+ Guard = &Call.getCalleeAnalysisDeclContext()->getASTContext().Idents.get(
+ GuardName);
+ IdentifierInfoInitialized = true;
+}
+ }
+
+public:
+ RAIIMutexDescriptor(StringRef GuardName) : GuardName(GuardName) {}
+ [[nodiscard]] bool matchesLock(const CallEvent &Call) const {
+initIdentifierInfo(Call);
+const auto *Ctor = dyn_cast(&Call);
+if (!Ctor)
+ return false;
+auto *IdentifierInfo = Ctor->getDecl()->getParent()->getIdentifier();
+return IdentifierInfo == Guard;
+ }
+ [[nodiscard]] bool matchesUnlock(const CallEvent &Call) const {
+initIdentifierInfo(Call);
+const auto *Dtor = dyn_cast(&Call);
+if (!Dtor)
+ return false;
+auto *IdentifierInfo =
+cast(Dtor->getDecl()->getParent())->getIdentifier();
NagyDonat wrote:
Why do you need a cast to `CXXRecordDecl` here if it isn't needed for the
constructor?
https://github.com/llvm/llvm-project/pull/80029
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] aec9283 - [clang-repl] Refactor locking of runtime PTU stack (NFC) (#84176)
Author: Stefan Gränitz
Date: 2024-03-11T13:39:23+01:00
New Revision: aec92830b79a8c49cdce0d592627d5f18bb6370b
URL:
https://github.com/llvm/llvm-project/commit/aec92830b79a8c49cdce0d592627d5f18bb6370b
DIFF:
https://github.com/llvm/llvm-project/commit/aec92830b79a8c49cdce0d592627d5f18bb6370b.diff
LOG: [clang-repl] Refactor locking of runtime PTU stack (NFC) (#84176)
The Interpreter locks PTUs that originate from implicit runtime code and
initialization to prevent users from undoing them accidentally.
The previous implementation seemed hacky, because it required the reader
to be familiar with the internal workings of the PTU stack. The concept
itself is a pragmatic solution and not very surprising. This patch
introduces a function for it and adds a comment.
Added:
Modified:
clang/include/clang/Interpreter/Interpreter.h
clang/lib/Interpreter/Interpreter.cpp
Removed:
diff --git a/clang/include/clang/Interpreter/Interpreter.h
b/clang/include/clang/Interpreter/Interpreter.h
index d972d960dcb7cd..469ce1fd75bf84 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -169,6 +169,7 @@ class Interpreter {
private:
size_t getEffectivePTUSize() const;
+ void markUserCodeStart();
llvm::DenseMap Dtors;
diff --git a/clang/lib/Interpreter/Interpreter.cpp
b/clang/lib/Interpreter/Interpreter.cpp
index 3485da8196683a..e293fefb524963 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -280,15 +280,14 @@ Interpreter::create(std::unique_ptr CI)
{
if (Err)
return std::move(Err);
+ // Add runtime code and set a marker to hide it from user code. Undo will not
+ // go through that.
auto PTU = Interp->Parse(Runtimes);
if (!PTU)
return PTU.takeError();
+ Interp->markUserCodeStart();
Interp->ValuePrintingInfo.resize(4);
- // FIXME: This is a ugly hack. Undo command checks its availability by
looking
- // at the size of the PTU list. However we have parsed something in the
- // beginning of the REPL so we have to mark them as 'Irrevocable'.
- Interp->InitPTUSize = Interp->IncrParser->getPTUs().size();
return std::move(Interp);
}
@@ -345,6 +344,11 @@ const ASTContext &Interpreter::getASTContext() const {
return getCompilerInstance()->getASTContext();
}
+void Interpreter::markUserCodeStart() {
+ assert(!InitPTUSize && "We only do this once");
+ InitPTUSize = IncrParser->getPTUs().size();
+}
+
size_t Interpreter::getEffectivePTUSize() const {
std::list &PTUs = IncrParser->getPTUs();
assert(PTUs.size() >= InitPTUSize && "empty PTU list?");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Refactor locking of runtime PTU stack (NFC) (PR #84176)
https://github.com/weliveindetail closed https://github.com/llvm/llvm-project/pull/84176 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [clang][UBSan] Add implicit conversion check for bitfields (PR #75481)
@@ -555,13 +555,11 @@ static void
handleImplicitConversion(ImplicitConversionData *Data,
ReportOptions Opts, ValueHandle Src,
ValueHandle Dst) {
SourceLocation Loc = Data->Loc.acquire();
- ErrorType ET = ErrorType::GenericUB;
-
const TypeDescriptor &SrcTy = Data->FromType;
const TypeDescriptor &DstTy = Data->ToType;
-
bool SrcSigned = SrcTy.isSignedIntegerTy();
bool DstSigned = DstTy.isSignedIntegerTy();
+ ErrorType ET = ErrorType::GenericUB;
Zonotora wrote:
I have only moved this line... I think changing the error type is for a future
PR as you comment further down.
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [clang][UBSan] Add implicit conversion check for bitfields (PR #75481)
Zonotora wrote: @zygoloid Thanks for your feedback! Fixed all comments except the last one. I totally agree with you about the distinction of UB checks, and that should be part of a different future PR! https://github.com/llvm/llvm-project/pull/75481 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add bit-precise overloads for builtin operators (PR #84755)
https://github.com/AaronBallman created
https://github.com/llvm/llvm-project/pull/84755
We previously were not adding them to the candidate set and so use of a
bit-precise integer as a class member could lead to ambiguous overload sets.
Fixes https://github.com/llvm/llvm-project/issues/82998
>From 1f57de0e2e11a4bd834871e7d033ec53c43dc42e Mon Sep 17 00:00:00 2001
From: Aaron Ballman
Date: Mon, 11 Mar 2024 08:41:37 -0400
Subject: [PATCH] Add bit-precise overloads for builtin operators
We previously were not adding them to the candidate set and so use of a
bit-precise integer as a class member could lead to ambiguous overload
sets.
Fixes #82998
---
clang/docs/ReleaseNotes.rst| 3 ++
clang/lib/Sema/SemaOverload.cpp| 23 ++
clang/test/SemaCXX/overload-bitint.cpp | 42 ++
3 files changed, 68 insertions(+)
create mode 100644 clang/test/SemaCXX/overload-bitint.cpp
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0ee2801766a9cc..ab95d74d4356f5 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -254,6 +254,9 @@ Bug Fixes in This Version
operator.
Fixes (#GH83267).
+- Clang now correctly generates overloads for bit-precise integer types for
+ builtin operators in C++. Fixes #GH82998.
+
Bug Fixes to Compiler Builtins
^^
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index b0c693f078efe2..675f6f9ebcda5f 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -8516,6 +8516,9 @@ class BuiltinCandidateTypeSet {
/// candidates.
TypeSet MatrixTypes;
+ /// The set of _BitInt types that will be used in the built-in candidates.
+ TypeSet BitIntTypes;
+
/// A flag indicating non-record types are viable candidates
bool HasNonRecordTypes;
@@ -8564,6 +8567,7 @@ class BuiltinCandidateTypeSet {
}
llvm::iterator_range vector_types() { return VectorTypes; }
llvm::iterator_range matrix_types() { return MatrixTypes; }
+ llvm::iterator_range bitint_types() { return BitIntTypes; }
bool containsMatrixType(QualType Ty) const { return MatrixTypes.count(Ty); }
bool hasNonRecordTypes() { return HasNonRecordTypes; }
@@ -8735,6 +8739,9 @@ BuiltinCandidateTypeSet::AddTypesConvertedFrom(QualType
Ty,
} else if (Ty->isEnumeralType()) {
HasArithmeticOrEnumeralTypes = true;
EnumerationTypes.insert(Ty);
+ } else if (Ty->isBitIntType()) {
+HasArithmeticOrEnumeralTypes = true;
+BitIntTypes.insert(Ty);
} else if (Ty->isVectorType()) {
// We treat vector types as arithmetic types in many contexts as an
// extension.
@@ -8955,6 +8962,22 @@ class BuiltinOperatorOverloadBuilder {
(S.Context.getAuxTargetInfo() &&
S.Context.getAuxTargetInfo()->hasInt128Type()))
ArithmeticTypes.push_back(S.Context.UnsignedInt128Ty);
+
+/// _BitInt overloads are a bit special. We don't want to add candidates
+/// for the entire family of _BitInt types, so instead we only add
+/// candidates for the unique, unqualified _BitInt types present in the
+/// candidate type set. The candidate set already handled ensuring the type
+/// is unqualified and canonical, but because we're adding from N different
+/// sets, we need to do some extra work to unique things. Copy the
+/// candidates into a unique set, then copy from that set into the list of
+/// arithmetic types.
+llvm::SmallSetVector BitIntCandidates;
+llvm::for_each(CandidateTypes, [&BitIntCandidates](
+ BuiltinCandidateTypeSet &Candidate) {
+ for (QualType BitTy : Candidate.bitint_types())
+BitIntCandidates.insert(CanQualType::CreateUnsafe(BitTy));
+});
+llvm::copy(BitIntCandidates, std::back_inserter(ArithmeticTypes));
LastPromotedIntegralType = ArithmeticTypes.size();
LastPromotedArithmeticType = ArithmeticTypes.size();
// End of promoted types.
diff --git a/clang/test/SemaCXX/overload-bitint.cpp
b/clang/test/SemaCXX/overload-bitint.cpp
new file mode 100644
index 00..b834a3b01fede6
--- /dev/null
+++ b/clang/test/SemaCXX/overload-bitint.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+
+#include "Inputs/std-compare.h"
+
+struct S {
+ _BitInt(12) a;
+
+ constexpr operator _BitInt(12)() const { return a; }
+};
+
+// None of these used to compile because we weren't adding _BitInt types to the
+// overload set for builtin operators. See GH82998.
+static_assert(S{10} < 11);
+static_assert(S{10} <= 11);
+static_assert(S{12} > 11);
+static_assert(S{12} >= 11);
+static_assert(S{10} == 10);
+static_assert((S{10} <=> 10) == 0);
+static_assert(S{10} != 11);
+static_assert(S{10} + 0 == 10);
+static_assert(S{10} - 0 == 10);
+static_assert(S{10} * 1 == 10);
+static_assert(S{10} / 1 == 10);
+static_assert(S{10} % 1 == 0);
+static_assert(S{10} << 0 == 10
[clang] Add bit-precise overloads for builtin operators (PR #84755)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Aaron Ballman (AaronBallman)
Changes
We previously were not adding them to the candidate set and so use of a
bit-precise integer as a class member could lead to ambiguous overload sets.
Fixes https://github.com/llvm/llvm-project/issues/82998
---
Full diff: https://github.com/llvm/llvm-project/pull/84755.diff
3 Files Affected:
- (modified) clang/docs/ReleaseNotes.rst (+3)
- (modified) clang/lib/Sema/SemaOverload.cpp (+23)
- (added) clang/test/SemaCXX/overload-bitint.cpp (+42)
``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index bce27dc8c4a996..5b3a5b6c9bf23e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -262,6 +262,9 @@ Bug Fixes in This Version
operator.
Fixes (#GH83267).
+- Clang now correctly generates overloads for bit-precise integer types for
+ builtin operators in C++. Fixes #GH82998.
+
Bug Fixes to Compiler Builtins
^^
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index b0c693f078efe2..675f6f9ebcda5f 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -8516,6 +8516,9 @@ class BuiltinCandidateTypeSet {
/// candidates.
TypeSet MatrixTypes;
+ /// The set of _BitInt types that will be used in the built-in candidates.
+ TypeSet BitIntTypes;
+
/// A flag indicating non-record types are viable candidates
bool HasNonRecordTypes;
@@ -8564,6 +8567,7 @@ class BuiltinCandidateTypeSet {
}
llvm::iterator_range vector_types() { return VectorTypes; }
llvm::iterator_range matrix_types() { return MatrixTypes; }
+ llvm::iterator_range bitint_types() { return BitIntTypes; }
bool containsMatrixType(QualType Ty) const { return MatrixTypes.count(Ty); }
bool hasNonRecordTypes() { return HasNonRecordTypes; }
@@ -8735,6 +8739,9 @@ BuiltinCandidateTypeSet::AddTypesConvertedFrom(QualType
Ty,
} else if (Ty->isEnumeralType()) {
HasArithmeticOrEnumeralTypes = true;
EnumerationTypes.insert(Ty);
+ } else if (Ty->isBitIntType()) {
+HasArithmeticOrEnumeralTypes = true;
+BitIntTypes.insert(Ty);
} else if (Ty->isVectorType()) {
// We treat vector types as arithmetic types in many contexts as an
// extension.
@@ -8955,6 +8962,22 @@ class BuiltinOperatorOverloadBuilder {
(S.Context.getAuxTargetInfo() &&
S.Context.getAuxTargetInfo()->hasInt128Type()))
ArithmeticTypes.push_back(S.Context.UnsignedInt128Ty);
+
+/// _BitInt overloads are a bit special. We don't want to add candidates
+/// for the entire family of _BitInt types, so instead we only add
+/// candidates for the unique, unqualified _BitInt types present in the
+/// candidate type set. The candidate set already handled ensuring the type
+/// is unqualified and canonical, but because we're adding from N different
+/// sets, we need to do some extra work to unique things. Copy the
+/// candidates into a unique set, then copy from that set into the list of
+/// arithmetic types.
+llvm::SmallSetVector BitIntCandidates;
+llvm::for_each(CandidateTypes, [&BitIntCandidates](
+ BuiltinCandidateTypeSet &Candidate) {
+ for (QualType BitTy : Candidate.bitint_types())
+BitIntCandidates.insert(CanQualType::CreateUnsafe(BitTy));
+});
+llvm::copy(BitIntCandidates, std::back_inserter(ArithmeticTypes));
LastPromotedIntegralType = ArithmeticTypes.size();
LastPromotedArithmeticType = ArithmeticTypes.size();
// End of promoted types.
diff --git a/clang/test/SemaCXX/overload-bitint.cpp
b/clang/test/SemaCXX/overload-bitint.cpp
new file mode 100644
index 00..b834a3b01fede6
--- /dev/null
+++ b/clang/test/SemaCXX/overload-bitint.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+
+#include "Inputs/std-compare.h"
+
+struct S {
+ _BitInt(12) a;
+
+ constexpr operator _BitInt(12)() const { return a; }
+};
+
+// None of these used to compile because we weren't adding _BitInt types to the
+// overload set for builtin operators. See GH82998.
+static_assert(S{10} < 11);
+static_assert(S{10} <= 11);
+static_assert(S{12} > 11);
+static_assert(S{12} >= 11);
+static_assert(S{10} == 10);
+static_assert((S{10} <=> 10) == 0);
+static_assert(S{10} != 11);
+static_assert(S{10} + 0 == 10);
+static_assert(S{10} - 0 == 10);
+static_assert(S{10} * 1 == 10);
+static_assert(S{10} / 1 == 10);
+static_assert(S{10} % 1 == 0);
+static_assert(S{10} << 0 == 10);
+static_assert(S{10} >> 0 == 10);
+static_assert((S{10} | 0) == 10);
+static_assert((S{10} & 10) == 10);
+static_assert((S{10} ^ 0) == 10);
+static_assert(-S{10} == -10);
+static_assert(+S{10} == +10);
+static_assert(~S{10} == ~10);
+
+struct A {
+ _BitInt(12) a;
+
+ bool operator==(const A&) const = default;
+ bool operator!=(const A&) const = default;
+ std::str
[clang] [analyzer] Turn NodeBuilderContext into a class (PR #84638)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/84638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Turn NodeBuilderContext into a class (PR #84638)
https://github.com/steakhal commented: Alright. So we can simply mark all the members private. I was surprised a bit. LGTM, but remove the TODO comment you fix. https://github.com/llvm/llvm-project/pull/84638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Turn NodeBuilderContext into a class (PR #84638)
@@ -194,11 +194,12 @@ class CoreEngine {
};
// TODO: Turn into a class.
steakhal wrote:
Remove this TODO, as its completed.
https://github.com/llvm/llvm-project/pull/84638
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Mention possibility of underflow in array overflow errors (PR #84201)
=?utf-8?q?Don=C3=A1t?= Nagy ,NagyDonat Message-ID: In-Reply-To: https://github.com/steakhal approved this pull request. I scrolled over, and it looks harmless, so LGTM. https://github.com/llvm/llvm-project/pull/84201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Model more getline/getdelim pre and postconditions (PR #83027)
Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID: In-Reply-To: steakhal wrote: > I want to avoid that some functions have null pointer checks in > `StreamChecker`, some not. If this change is merged then it would be good to > add null pointer checks to other functions like `fread` and `fwrite`. (Until > now only the NULL stream pointer was checked.) I can really think of having these checks at both checkers, so that both parties are sort of happy - except for of course having this precondition checked at both places. But to me, the question is really as follow: is it better with this, or not? I'm also open for suggestions for resolving this conflict, so let me know if you have ideas. https://github.com/llvm/llvm-project/pull/83027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [RemoveDIs] Update Clang front end to handle DbgRecords (PR #84756)
https://github.com/OCHyams created
https://github.com/llvm/llvm-project/pull/84756
This patch fixes problems that pop up when clang emits DbgRecords instead of
debug intrinsics.
Note: this doesn't mean clang is emitting DbgRecords yet, because the modules
it creates are still always in the old debug mode. That will come in a future
patch.
>From 8b37020a48b8145ad9d7deb288b9f8a3ee3a9a9b Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams
Date: Sat, 9 Mar 2024 13:10:05 +
Subject: [PATCH 1/2] clang: insertbefore non-debug for removeDIs stability
fixes debug-info-blocks.m in both modes
---
clang/lib/CodeGen/CGBlocks.cpp | 5 -
clang/test/CodeGenObjC/debug-info-blocks.m | 2 +-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 0cbace7b7f7bbd..768b786c6426df 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -1540,7 +1540,10 @@ llvm::Function *CodeGenFunction::GenerateBlockFunction(
llvm::BasicBlock *resume = Builder.GetInsertBlock();
// Go back to the entry.
- ++entry_ptr;
+ if (entry_ptr->getNextNonDebugInstruction())
+entry_ptr = entry_ptr->getNextNonDebugInstruction()->getIterator();
+ else
+entry_ptr = entry->end();
Builder.SetInsertPoint(entry, entry_ptr);
// Emit debug information for all the DeclRefExprs.
diff --git a/clang/test/CodeGenObjC/debug-info-blocks.m
b/clang/test/CodeGenObjC/debug-info-blocks.m
index 14b29f222fbe8e..59171da016da1e 100644
--- a/clang/test/CodeGenObjC/debug-info-blocks.m
+++ b/clang/test/CodeGenObjC/debug-info-blocks.m
@@ -5,8 +5,8 @@
// CHECK: define {{.*}}_block_invoke
// CHECK: store ptr %.block_descriptor, ptr %[[ALLOCA:block.addr]], align
-// CHECK-NEXT: call void @llvm.dbg.declare(metadata ptr %[[ALLOCA]], metadata
![[SELF:[0-9]+]], metadata !{{.*}})
// CHECK-NEXT: call void @llvm.dbg.declare(metadata ptr %d, metadata
![[D:[0-9]+]], metadata !{{.*}})
+// CHECK-NEXT: call void @llvm.dbg.declare(metadata ptr %[[ALLOCA]], metadata
![[SELF:[0-9]+]], metadata !{{.*}})
// Test that we do emit scope info for the helper functions, and that the
// parameters to these functions are marked as artificial (so the debugger
>From efabe4b06f38aaf3aed8cf02c97f566652ba5f15 Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams
Date: Sat, 9 Mar 2024 13:40:30 +
Subject: [PATCH 2/2] openmp update DbgRecords
Fixed OpenMP/debug_task_shared.c
---
clang/lib/CodeGen/CGStmtOpenMP.cpp | 22 +-
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp
b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index 3fbd2e03eb61df..5c6a3b8e001730 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -4746,10 +4746,10 @@ void CodeGenFunction::EmitOMPTaskBasedDirective(
if (CGF.CGM.getCodeGenOpts().hasReducedDebugInfo())
(void)DI->EmitDeclareOfAutoVariable(SharedVar, ContextValue,
CGF.Builder, false);
- llvm::Instruction &Last = CGF.Builder.GetInsertBlock()->back();
// Get the call dbg.declare instruction we just created and update
// its DIExpression to add offset to base address.
- if (auto DDI = dyn_cast(&Last)) {
+ auto UpdateExpr = [](llvm::LLVMContext &Ctx, auto *DDI,
+ unsigned Offset) {
SmallVector Ops;
// Add offset to the base address if non zero.
if (Offset) {
@@ -4757,9 +4757,21 @@ void CodeGenFunction::EmitOMPTaskBasedDirective(
Ops.push_back(Offset);
}
Ops.push_back(llvm::dwarf::DW_OP_deref);
-auto &Ctx = DDI->getContext();
-llvm::DIExpression *DIExpr = llvm::DIExpression::get(Ctx, Ops);
-Last.setOperand(2, llvm::MetadataAsValue::get(Ctx, DIExpr));
+DDI->setExpression(llvm::DIExpression::get(Ctx, Ops));
+ };
+ llvm::Instruction &Last = CGF.Builder.GetInsertBlock()->back();
+ if (auto DDI = dyn_cast(&Last))
+UpdateExpr(DDI->getContext(), DDI, Offset);
+ // If we're emitting using the new debug info format into a block
+ // without a terminator, the record will be "trailing".
+ assert(!Last.isTerminator() && "unexpected terminator");
+ if (auto *Marker =
+ CGF.Builder.GetInsertBlock()->getTrailingDPValues()) {
+for (llvm::DPValue &DPV : llvm::reverse(
+ llvm::DPValue::filter(Marker->getDbgValueRange( {
+ UpdateExpr(Last.getContext(), &DPV, Offset);
+ break;
+}
}
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [RemoveDIs] Update Clang front end to handle DbgRecords (PR #84756)
https://github.com/OCHyams edited https://github.com/llvm/llvm-project/pull/84756 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [RemoveDIs] Update Clang front end to handle DbgRecords (PR #84756)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Orlando Cazalet-Hyams (OCHyams)
Changes
This patch fixes problems that pop up when clang emits DbgRecords instead of
debug intrinsics.
Note: this doesn't mean clang is emitting DbgRecords yet, because the modules
it creates are still always in the old debug mode. That will come in a future
patch.
Depends on #84739
---
Full diff: https://github.com/llvm/llvm-project/pull/84756.diff
3 Files Affected:
- (modified) clang/lib/CodeGen/CGBlocks.cpp (+4-1)
- (modified) clang/lib/CodeGen/CGStmtOpenMP.cpp (+17-5)
- (modified) clang/test/CodeGenObjC/debug-info-blocks.m (+1-1)
``diff
diff --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 0cbace7b7f7bbd..768b786c6426df 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -1540,7 +1540,10 @@ llvm::Function *CodeGenFunction::GenerateBlockFunction(
llvm::BasicBlock *resume = Builder.GetInsertBlock();
// Go back to the entry.
- ++entry_ptr;
+ if (entry_ptr->getNextNonDebugInstruction())
+entry_ptr = entry_ptr->getNextNonDebugInstruction()->getIterator();
+ else
+entry_ptr = entry->end();
Builder.SetInsertPoint(entry, entry_ptr);
// Emit debug information for all the DeclRefExprs.
diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp
b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index 3fbd2e03eb61df..5c6a3b8e001730 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -4746,10 +4746,10 @@ void CodeGenFunction::EmitOMPTaskBasedDirective(
if (CGF.CGM.getCodeGenOpts().hasReducedDebugInfo())
(void)DI->EmitDeclareOfAutoVariable(SharedVar, ContextValue,
CGF.Builder, false);
- llvm::Instruction &Last = CGF.Builder.GetInsertBlock()->back();
// Get the call dbg.declare instruction we just created and update
// its DIExpression to add offset to base address.
- if (auto DDI = dyn_cast(&Last)) {
+ auto UpdateExpr = [](llvm::LLVMContext &Ctx, auto *DDI,
+ unsigned Offset) {
SmallVector Ops;
// Add offset to the base address if non zero.
if (Offset) {
@@ -4757,9 +4757,21 @@ void CodeGenFunction::EmitOMPTaskBasedDirective(
Ops.push_back(Offset);
}
Ops.push_back(llvm::dwarf::DW_OP_deref);
-auto &Ctx = DDI->getContext();
-llvm::DIExpression *DIExpr = llvm::DIExpression::get(Ctx, Ops);
-Last.setOperand(2, llvm::MetadataAsValue::get(Ctx, DIExpr));
+DDI->setExpression(llvm::DIExpression::get(Ctx, Ops));
+ };
+ llvm::Instruction &Last = CGF.Builder.GetInsertBlock()->back();
+ if (auto DDI = dyn_cast(&Last))
+UpdateExpr(DDI->getContext(), DDI, Offset);
+ // If we're emitting using the new debug info format into a block
+ // without a terminator, the record will be "trailing".
+ assert(!Last.isTerminator() && "unexpected terminator");
+ if (auto *Marker =
+ CGF.Builder.GetInsertBlock()->getTrailingDPValues()) {
+for (llvm::DPValue &DPV : llvm::reverse(
+ llvm::DPValue::filter(Marker->getDbgValueRange( {
+ UpdateExpr(Last.getContext(), &DPV, Offset);
+ break;
+}
}
}
}
diff --git a/clang/test/CodeGenObjC/debug-info-blocks.m
b/clang/test/CodeGenObjC/debug-info-blocks.m
index 14b29f222fbe8e..59171da016da1e 100644
--- a/clang/test/CodeGenObjC/debug-info-blocks.m
+++ b/clang/test/CodeGenObjC/debug-info-blocks.m
@@ -5,8 +5,8 @@
// CHECK: define {{.*}}_block_invoke
// CHECK: store ptr %.block_descriptor, ptr %[[ALLOCA:block.addr]], align
-// CHECK-NEXT: call void @llvm.dbg.declare(metadata ptr %[[ALLOCA]], metadata
![[SELF:[0-9]+]], metadata !{{.*}})
// CHECK-NEXT: call void @llvm.dbg.declare(metadata ptr %d, metadata
![[D:[0-9]+]], metadata !{{.*}})
+// CHECK-NEXT: call void @llvm.dbg.declare(metadata ptr %[[ALLOCA]], metadata
![[SELF:[0-9]+]], metadata !{{.*}})
// Test that we do emit scope info for the helper functions, and that the
// parameters to these functions are marked as artificial (so the debugger
``
https://github.com/llvm/llvm-project/pull/84756
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [RemoveDIs] Update Clang front end to handle DbgRecords (PR #84756)
llvmbot wrote:
@llvm/pr-subscribers-clang-codegen
Author: Orlando Cazalet-Hyams (OCHyams)
Changes
This patch fixes problems that pop up when clang emits DbgRecords instead of
debug intrinsics.
Note: this doesn't mean clang is emitting DbgRecords yet, because the modules
it creates are still always in the old debug mode. That will come in a future
patch.
Depends on #84739
---
Full diff: https://github.com/llvm/llvm-project/pull/84756.diff
3 Files Affected:
- (modified) clang/lib/CodeGen/CGBlocks.cpp (+4-1)
- (modified) clang/lib/CodeGen/CGStmtOpenMP.cpp (+17-5)
- (modified) clang/test/CodeGenObjC/debug-info-blocks.m (+1-1)
``diff
diff --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 0cbace7b7f7bbd..768b786c6426df 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -1540,7 +1540,10 @@ llvm::Function *CodeGenFunction::GenerateBlockFunction(
llvm::BasicBlock *resume = Builder.GetInsertBlock();
// Go back to the entry.
- ++entry_ptr;
+ if (entry_ptr->getNextNonDebugInstruction())
+entry_ptr = entry_ptr->getNextNonDebugInstruction()->getIterator();
+ else
+entry_ptr = entry->end();
Builder.SetInsertPoint(entry, entry_ptr);
// Emit debug information for all the DeclRefExprs.
diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp
b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index 3fbd2e03eb61df..5c6a3b8e001730 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -4746,10 +4746,10 @@ void CodeGenFunction::EmitOMPTaskBasedDirective(
if (CGF.CGM.getCodeGenOpts().hasReducedDebugInfo())
(void)DI->EmitDeclareOfAutoVariable(SharedVar, ContextValue,
CGF.Builder, false);
- llvm::Instruction &Last = CGF.Builder.GetInsertBlock()->back();
// Get the call dbg.declare instruction we just created and update
// its DIExpression to add offset to base address.
- if (auto DDI = dyn_cast(&Last)) {
+ auto UpdateExpr = [](llvm::LLVMContext &Ctx, auto *DDI,
+ unsigned Offset) {
SmallVector Ops;
// Add offset to the base address if non zero.
if (Offset) {
@@ -4757,9 +4757,21 @@ void CodeGenFunction::EmitOMPTaskBasedDirective(
Ops.push_back(Offset);
}
Ops.push_back(llvm::dwarf::DW_OP_deref);
-auto &Ctx = DDI->getContext();
-llvm::DIExpression *DIExpr = llvm::DIExpression::get(Ctx, Ops);
-Last.setOperand(2, llvm::MetadataAsValue::get(Ctx, DIExpr));
+DDI->setExpression(llvm::DIExpression::get(Ctx, Ops));
+ };
+ llvm::Instruction &Last = CGF.Builder.GetInsertBlock()->back();
+ if (auto DDI = dyn_cast(&Last))
+UpdateExpr(DDI->getContext(), DDI, Offset);
+ // If we're emitting using the new debug info format into a block
+ // without a terminator, the record will be "trailing".
+ assert(!Last.isTerminator() && "unexpected terminator");
+ if (auto *Marker =
+ CGF.Builder.GetInsertBlock()->getTrailingDPValues()) {
+for (llvm::DPValue &DPV : llvm::reverse(
+ llvm::DPValue::filter(Marker->getDbgValueRange( {
+ UpdateExpr(Last.getContext(), &DPV, Offset);
+ break;
+}
}
}
}
diff --git a/clang/test/CodeGenObjC/debug-info-blocks.m
b/clang/test/CodeGenObjC/debug-info-blocks.m
index 14b29f222fbe8e..59171da016da1e 100644
--- a/clang/test/CodeGenObjC/debug-info-blocks.m
+++ b/clang/test/CodeGenObjC/debug-info-blocks.m
@@ -5,8 +5,8 @@
// CHECK: define {{.*}}_block_invoke
// CHECK: store ptr %.block_descriptor, ptr %[[ALLOCA:block.addr]], align
-// CHECK-NEXT: call void @llvm.dbg.declare(metadata ptr %[[ALLOCA]], metadata
![[SELF:[0-9]+]], metadata !{{.*}})
// CHECK-NEXT: call void @llvm.dbg.declare(metadata ptr %d, metadata
![[D:[0-9]+]], metadata !{{.*}})
+// CHECK-NEXT: call void @llvm.dbg.declare(metadata ptr %[[ALLOCA]], metadata
![[SELF:[0-9]+]], metadata !{{.*}})
// Test that we do emit scope info for the helper functions, and that the
// parameters to these functions are marked as artificial (so the debugger
``
https://github.com/llvm/llvm-project/pull/84756
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [RemoveDIs] Update Clang front end to handle DbgRecords (PR #84756)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff e77f5fe889909df3508fd929f2636a0ac211877a efabe4b06f38aaf3aed8cf02c97f566652ba5f15 -- clang/lib/CodeGen/CGBlocks.cpp clang/lib/CodeGen/CGStmtOpenMP.cpp `` View the diff from clang-format here. ``diff diff --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp index 768b786c64..ad0b50d799 100644 --- a/clang/lib/CodeGen/CGBlocks.cpp +++ b/clang/lib/CodeGen/CGBlocks.cpp @@ -1542,7 +1542,7 @@ llvm::Function *CodeGenFunction::GenerateBlockFunction( // Go back to the entry. if (entry_ptr->getNextNonDebugInstruction()) entry_ptr = entry_ptr->getNextNonDebugInstruction()->getIterator(); - else + else entry_ptr = entry->end(); Builder.SetInsertPoint(entry, entry_ptr); `` https://github.com/llvm/llvm-project/pull/84756 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [analyzer] Mention possibility of underflow in array overflow errors (PR #84201)
=?utf-8?q?Donát?= Nagy ,NagyDonat
,Balazs Benics
Message-ID:
In-Reply-To:
https://github.com/NagyDonat updated
https://github.com/llvm/llvm-project/pull/84201
>From 74c7c04cb456c0a058ca36a2ca4ffa4d70d576e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?=
Date: Wed, 6 Mar 2024 18:09:02 +0100
Subject: [PATCH 1/4] [analyzer] Mention possibility of underflow in array
overflow errors
The checker alpha.security.ArrayBoundV2 performs bounds checking in two
steps: first it checks for underflow, and if it isn't guaranteed then it
assumes that there is no underflow. After this, it checks for overflow,
and if it's guaranteed or the index is tainted then it reports it.
This meant that in situations where overflow and underflow are both
possible (but the index is either tainted or guaranteed to be invalid),
the checker was reporting an overflow error.
This commit modifies the messages printed in these cases to mention the
possibility of an underflow.
---
.../Checkers/ArrayBoundCheckerV2.cpp | 37 +++-
.../test/Analysis/out-of-bounds-diagnostics.c | 56 ++-
.../test/Analysis/taint-diagnostic-visitor.c | 2 +-
3 files changed, 80 insertions(+), 15 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
index 29eb932584027d..664d26b00a3cab 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -83,6 +83,8 @@ class StateUpdateReporter {
AssumedUpperBound = UpperBoundVal;
}
+ bool assumedNonNegative() { return AssumedNonNegative; }
+
const NoteTag *createNoteTag(CheckerContext &C) const;
private:
@@ -402,7 +404,8 @@ static bool tryDividePair(std::optional &Val1,
}
static Messages getExceedsMsgs(ASTContext &ACtx, const SubRegion *Region,
- NonLoc Offset, NonLoc Extent, SVal Location) {
+ NonLoc Offset, NonLoc Extent, SVal Location,
+ bool AssumedNonNegative) {
std::string RegName = getRegionName(Region);
const auto *EReg = Location.getAsRegion()->getAs();
assert(EReg && "this checker only handles element access");
@@ -414,6 +417,7 @@ static Messages getExceedsMsgs(ASTContext &ACtx, const
SubRegion *Region,
int64_t ElemSize = ACtx.getTypeSizeInChars(ElemType).getQuantity();
bool UseByteOffsets = !tryDividePair(OffsetN, ExtentN, ElemSize);
+ const char *OffsetOrIndex = UseByteOffsets ? "byte offset" : "index";
SmallString<256> Buf;
llvm::raw_svector_ostream Out(Buf);
@@ -421,10 +425,12 @@ static Messages getExceedsMsgs(ASTContext &ACtx, const
SubRegion *Region,
if (!ExtentN && !UseByteOffsets)
Out << "'" << ElemType.getAsString() << "' element in ";
Out << RegName << " at ";
- if (OffsetN) {
-Out << (UseByteOffsets ? "byte offset " : "index ") << *OffsetN;
+ if (AssumedNonNegative) {
+Out << "a negative or overflowing " << OffsetOrIndex;
+ } else if (OffsetN) {
+Out << OffsetOrIndex << " " << *OffsetN;
} else {
-Out << "an overflowing " << (UseByteOffsets ? "byte offset" : "index");
+Out << "an overflowing " << OffsetOrIndex;
}
if (ExtentN) {
Out << ", while it holds only ";
@@ -441,17 +447,19 @@ static Messages getExceedsMsgs(ASTContext &ACtx, const
SubRegion *Region,
Out << "s";
}
- return {
- formatv("Out of bound access to memory after the end of {0}", RegName),
- std::string(Buf)};
+ return {formatv("Out of bound access to memory {0} {1}",
+ AssumedNonNegative ? "around" : "after the end of", RegName),
+ std::string(Buf)};
}
-static Messages getTaintMsgs(const SubRegion *Region, const char *OffsetName) {
+static Messages getTaintMsgs(const SubRegion *Region, const char *OffsetName,
+ bool AssumedNonNegative) {
std::string RegName = getRegionName(Region);
return {formatv("Potential out of bound access to {0} with tainted {1}",
RegName, OffsetName),
- formatv("Access of {0} with a tainted {1} that may be too large",
- RegName, OffsetName)};
+ formatv("Access of {0} with a tainted {1} that may be {2}too large",
+ RegName, OffsetName,
+ AssumedNonNegative ? "negative or " : "")};
}
const NoteTag *StateUpdateReporter::createNoteTag(CheckerContext &C) const {
@@ -603,6 +611,8 @@ void ArrayBoundCheckerV2::performCheck(const Expr *E,
CheckerContext &C) const {
auto [WithinUpperBound, ExceedsUpperBound] =
compareValueToThreshold(State, ByteOffset, *KnownSize, SVB);
+bool AssumedNonNegative = SUR.assumedNonNegative();
+
if (ExceedsUpperBound) {
// The offset may be invalid (>= Size)...
if (!WithinUpperBound) {
@@ -615,8 +625,9 @@ void ArrayBoundCheckerV2::performCheck(const Expr *E,
CheckerContext &C) c
[clang] Clang repl implicit executor create (PR #84758)
https://github.com/weliveindetail created
https://github.com/llvm/llvm-project/pull/84758
Until now the IncrExecutor is created lazily on the first execution request. In
order to process the PTUs that come from initialization, we have to do it
upfront implicitly.
From 2f7b523472543a92a97701880f8cd3c928061f53 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?=
Date: Sun, 10 Mar 2024 18:17:48 +0100
Subject: [PATCH 1/2] [clang-repl] Set up executor implicitly to account for
init PTUs
---
clang/lib/Interpreter/Interpreter.cpp | 30 +++
clang/test/Interpreter/execute.cpp| 4 ++--
2 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Interpreter/Interpreter.cpp
b/clang/lib/Interpreter/Interpreter.cpp
index e293fefb524963..5e1161ca472b36 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -229,12 +229,30 @@ IncrementalCompilerBuilder::CreateCudaHost() {
}
Interpreter::Interpreter(std::unique_ptr CI,
- llvm::Error &Err) {
- llvm::ErrorAsOutParameter EAO(&Err);
+ llvm::Error &ErrOut) {
+ llvm::ErrorAsOutParameter EAO(&ErrOut);
auto LLVMCtx = std::make_unique();
TSCtx = std::make_unique(std::move(LLVMCtx));
- IncrParser = std::make_unique(*this, std::move(CI),
- *TSCtx->getContext(), Err);
+ IncrParser = std::make_unique(
+ *this, std::move(CI), *TSCtx->getContext(), ErrOut);
+ if (ErrOut)
+return;
+
+ // Not all frontends support code-generation, e.g. ast-dump actions don't
+ if (IncrParser->getCodeGen()) {
+if (llvm::Error Err = CreateExecutor()) {
+ ErrOut = joinErrors(std::move(ErrOut), std::move(Err));
+ return;
+}
+
+// Process the PTUs that came from initialization. For example -include
will
+// give us a header that's processed at initialization of the preprocessor.
+for (PartialTranslationUnit &PTU : IncrParser->getPTUs())
+ if (llvm::Error Err = Execute(PTU)) {
+ErrOut = joinErrors(std::move(ErrOut), std::move(Err));
+return;
+ }
+ }
}
Interpreter::~Interpreter() {
@@ -375,6 +393,10 @@ Interpreter::Parse(llvm::StringRef Code) {
llvm::Error Interpreter::CreateExecutor() {
const clang::TargetInfo &TI =
getCompilerInstance()->getASTContext().getTargetInfo();
+ if (!IncrParser->getCodeGen())
+return llvm::make_error("Operation failed. "
+ "No code generator available",
+ std::error_code());
llvm::Error Err = llvm::Error::success();
auto Executor = std::make_unique(*TSCtx, Err, TI);
if (!Err)
diff --git a/clang/test/Interpreter/execute.cpp
b/clang/test/Interpreter/execute.cpp
index 6e73ed3927e815..534a54ed94fba2 100644
--- a/clang/test/Interpreter/execute.cpp
+++ b/clang/test/Interpreter/execute.cpp
@@ -7,6 +7,8 @@
// RUN: cat %s | clang-repl | FileCheck %s
// RUN: cat %s | clang-repl -Xcc -O2 | FileCheck %s
+// RUN: clang-repl -Xcc -include -Xcc %s | FileCheck %s
+// RUN: clang-repl -Xcc -fsyntax-only -Xcc -include -Xcc %s
extern "C" int printf(const char *, ...);
int i = 42;
auto r1 = printf("i = %d\n", i);
@@ -19,5 +21,3 @@ auto r2 = printf("S[f=%f, m=0x%llx]\n", s.f,
reinterpret_cast
Date: Mon, 11 Mar 2024 14:10:58 +0100
Subject: [PATCH 2/2] [tmp] Add crash note
---
clang/test/Interpreter/inline-virtual.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/test/Interpreter/inline-virtual.cpp
b/clang/test/Interpreter/inline-virtual.cpp
index 79ab8ed337ffea..d862b3354f61fe 100644
--- a/clang/test/Interpreter/inline-virtual.cpp
+++ b/clang/test/Interpreter/inline-virtual.cpp
@@ -14,7 +14,7 @@ struct A { int a; A(int a) : a(a) {} virtual ~A(); };
// PartialTranslationUnit.
inline A::~A() { printf("~A(%d)\n", a); }
-// Create one instance with new and delete it.
+// Create one instance with new and delete it. We crash here now:
A *a1 = new A(1);
delete a1;
// CHECK: ~A(1)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Clang repl implicit executor create (PR #84758)
Stefan =?utf-8?q?Gränitz?=
Message-ID:
In-Reply-To:
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Stefan Gränitz (weliveindetail)
Changes
Until now the IncrExecutor is created lazily on the first execution request. In
order to process the PTUs that come from initialization, we have to do it
upfront implicitly.
---
Full diff: https://github.com/llvm/llvm-project/pull/84758.diff
3 Files Affected:
- (modified) clang/lib/Interpreter/Interpreter.cpp (+26-4)
- (modified) clang/test/Interpreter/execute.cpp (+2-2)
- (modified) clang/test/Interpreter/inline-virtual.cpp (+1-1)
``diff
diff --git a/clang/lib/Interpreter/Interpreter.cpp
b/clang/lib/Interpreter/Interpreter.cpp
index e293fefb524963..5e1161ca472b36 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -229,12 +229,30 @@ IncrementalCompilerBuilder::CreateCudaHost() {
}
Interpreter::Interpreter(std::unique_ptr CI,
- llvm::Error &Err) {
- llvm::ErrorAsOutParameter EAO(&Err);
+ llvm::Error &ErrOut) {
+ llvm::ErrorAsOutParameter EAO(&ErrOut);
auto LLVMCtx = std::make_unique();
TSCtx = std::make_unique(std::move(LLVMCtx));
- IncrParser = std::make_unique(*this, std::move(CI),
- *TSCtx->getContext(), Err);
+ IncrParser = std::make_unique(
+ *this, std::move(CI), *TSCtx->getContext(), ErrOut);
+ if (ErrOut)
+return;
+
+ // Not all frontends support code-generation, e.g. ast-dump actions don't
+ if (IncrParser->getCodeGen()) {
+if (llvm::Error Err = CreateExecutor()) {
+ ErrOut = joinErrors(std::move(ErrOut), std::move(Err));
+ return;
+}
+
+// Process the PTUs that came from initialization. For example -include
will
+// give us a header that's processed at initialization of the preprocessor.
+for (PartialTranslationUnit &PTU : IncrParser->getPTUs())
+ if (llvm::Error Err = Execute(PTU)) {
+ErrOut = joinErrors(std::move(ErrOut), std::move(Err));
+return;
+ }
+ }
}
Interpreter::~Interpreter() {
@@ -375,6 +393,10 @@ Interpreter::Parse(llvm::StringRef Code) {
llvm::Error Interpreter::CreateExecutor() {
const clang::TargetInfo &TI =
getCompilerInstance()->getASTContext().getTargetInfo();
+ if (!IncrParser->getCodeGen())
+return llvm::make_error("Operation failed. "
+ "No code generator available",
+ std::error_code());
llvm::Error Err = llvm::Error::success();
auto Executor = std::make_unique(*TSCtx, Err, TI);
if (!Err)
diff --git a/clang/test/Interpreter/execute.cpp
b/clang/test/Interpreter/execute.cpp
index 6e73ed3927e815..534a54ed94fba2 100644
--- a/clang/test/Interpreter/execute.cpp
+++ b/clang/test/Interpreter/execute.cpp
@@ -7,6 +7,8 @@
// RUN: cat %s | clang-repl | FileCheck %s
// RUN: cat %s | clang-repl -Xcc -O2 | FileCheck %s
+// RUN: clang-repl -Xcc -include -Xcc %s | FileCheck %s
+// RUN: clang-repl -Xcc -fsyntax-only -Xcc -include -Xcc %s
extern "C" int printf(const char *, ...);
int i = 42;
auto r1 = printf("i = %d\n", i);
@@ -19,5 +21,3 @@ auto r2 = printf("S[f=%f, m=0x%llx]\n", s.f,
reinterpret_cast
https://github.com/llvm/llvm-project/pull/84758
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Set up executor implicitly to account for init PTUs (PR #84758)
https://github.com/weliveindetail edited https://github.com/llvm/llvm-project/pull/84758 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Expose CreateExecutor() and ResetExecutor() in extended Interpreter interface (PR #84460)
weliveindetail wrote: > > Do you mind incorporating this patch so that we avoid churn? > > Sure. Essentially, this drops lazy creation of the executor and makes it > dependent on the frontend action explicitly. Fine for me. We can still expose > explicit setup/tear-down. @vgvassilev I moved it into the separate review https://github.com/llvm/llvm-project/pull/84758, because it has unexpected side-effects. https://github.com/llvm/llvm-project/pull/84460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [analyzer] Mention possibility of underflow in array overflow errors (PR #84201)
=?utf-8?q?Donát?= Nagy ,NagyDonat
,Balazs Benics ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
https://github.com/NagyDonat updated
https://github.com/llvm/llvm-project/pull/84201
>From 74c7c04cb456c0a058ca36a2ca4ffa4d70d576e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?=
Date: Wed, 6 Mar 2024 18:09:02 +0100
Subject: [PATCH 1/5] [analyzer] Mention possibility of underflow in array
overflow errors
The checker alpha.security.ArrayBoundV2 performs bounds checking in two
steps: first it checks for underflow, and if it isn't guaranteed then it
assumes that there is no underflow. After this, it checks for overflow,
and if it's guaranteed or the index is tainted then it reports it.
This meant that in situations where overflow and underflow are both
possible (but the index is either tainted or guaranteed to be invalid),
the checker was reporting an overflow error.
This commit modifies the messages printed in these cases to mention the
possibility of an underflow.
---
.../Checkers/ArrayBoundCheckerV2.cpp | 37 +++-
.../test/Analysis/out-of-bounds-diagnostics.c | 56 ++-
.../test/Analysis/taint-diagnostic-visitor.c | 2 +-
3 files changed, 80 insertions(+), 15 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
index 29eb932584027d..664d26b00a3cab 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -83,6 +83,8 @@ class StateUpdateReporter {
AssumedUpperBound = UpperBoundVal;
}
+ bool assumedNonNegative() { return AssumedNonNegative; }
+
const NoteTag *createNoteTag(CheckerContext &C) const;
private:
@@ -402,7 +404,8 @@ static bool tryDividePair(std::optional &Val1,
}
static Messages getExceedsMsgs(ASTContext &ACtx, const SubRegion *Region,
- NonLoc Offset, NonLoc Extent, SVal Location) {
+ NonLoc Offset, NonLoc Extent, SVal Location,
+ bool AssumedNonNegative) {
std::string RegName = getRegionName(Region);
const auto *EReg = Location.getAsRegion()->getAs();
assert(EReg && "this checker only handles element access");
@@ -414,6 +417,7 @@ static Messages getExceedsMsgs(ASTContext &ACtx, const
SubRegion *Region,
int64_t ElemSize = ACtx.getTypeSizeInChars(ElemType).getQuantity();
bool UseByteOffsets = !tryDividePair(OffsetN, ExtentN, ElemSize);
+ const char *OffsetOrIndex = UseByteOffsets ? "byte offset" : "index";
SmallString<256> Buf;
llvm::raw_svector_ostream Out(Buf);
@@ -421,10 +425,12 @@ static Messages getExceedsMsgs(ASTContext &ACtx, const
SubRegion *Region,
if (!ExtentN && !UseByteOffsets)
Out << "'" << ElemType.getAsString() << "' element in ";
Out << RegName << " at ";
- if (OffsetN) {
-Out << (UseByteOffsets ? "byte offset " : "index ") << *OffsetN;
+ if (AssumedNonNegative) {
+Out << "a negative or overflowing " << OffsetOrIndex;
+ } else if (OffsetN) {
+Out << OffsetOrIndex << " " << *OffsetN;
} else {
-Out << "an overflowing " << (UseByteOffsets ? "byte offset" : "index");
+Out << "an overflowing " << OffsetOrIndex;
}
if (ExtentN) {
Out << ", while it holds only ";
@@ -441,17 +447,19 @@ static Messages getExceedsMsgs(ASTContext &ACtx, const
SubRegion *Region,
Out << "s";
}
- return {
- formatv("Out of bound access to memory after the end of {0}", RegName),
- std::string(Buf)};
+ return {formatv("Out of bound access to memory {0} {1}",
+ AssumedNonNegative ? "around" : "after the end of", RegName),
+ std::string(Buf)};
}
-static Messages getTaintMsgs(const SubRegion *Region, const char *OffsetName) {
+static Messages getTaintMsgs(const SubRegion *Region, const char *OffsetName,
+ bool AssumedNonNegative) {
std::string RegName = getRegionName(Region);
return {formatv("Potential out of bound access to {0} with tainted {1}",
RegName, OffsetName),
- formatv("Access of {0} with a tainted {1} that may be too large",
- RegName, OffsetName)};
+ formatv("Access of {0} with a tainted {1} that may be {2}too large",
+ RegName, OffsetName,
+ AssumedNonNegative ? "negative or " : "")};
}
const NoteTag *StateUpdateReporter::createNoteTag(CheckerContext &C) const {
@@ -603,6 +611,8 @@ void ArrayBoundCheckerV2::performCheck(const Expr *E,
CheckerContext &C) const {
auto [WithinUpperBound, ExceedsUpperBound] =
compareValueToThreshold(State, ByteOffset, *KnownSize, SVB);
+bool AssumedNonNegative = SUR.assumedNonNegative();
+
if (ExceedsUpperBound) {
// The offset may be invalid (>= Size)...
if (!WithinUpperBound) {
@@ -615,8 +625,9 @@ void ArrayBoundCheckerV2::performCheck(const Expr
[clang] [llvm] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,Balazs Benics
Message-ID:
In-Reply-To:
https://github.com/NagyDonat updated
https://github.com/llvm/llvm-project/pull/84469
>From c357aa998204e6693430c801f5b7d3a9e5e09e37 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?=
Date: Fri, 8 Mar 2024 13:06:01 +0100
Subject: [PATCH 1/4] [analyzer] Accept C library functions from the `std`
namespace
Previously, the function `isCLibraryFunction()` and logic relying on it
only accepted functions that are declared directly within a TU (i.e. not
in a namespace or a class). However C++ headers like declare
many C standard library functions within the namespace `std`, so this
commit ensures that functions within the namespace `std` are also
accepted.
After this commit it will be possible to match functions like `malloc` or
`free` with `CallDescription::Mode::CLibrary`.
---
.../StaticAnalyzer/Core/PathSensitive/CallDescription.h | 8 ++--
clang/lib/StaticAnalyzer/Core/CheckerContext.cpp | 9 ++---
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
index 3432d2648633c2..7da65734a44cf3 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
@@ -41,12 +41,8 @@ class CallDescription {
/// - We also accept calls where the number of arguments or parameters is
///greater than the specified value.
/// For the exact heuristics, see CheckerContext::isCLibraryFunction().
-/// Note that functions whose declaration context is not a TU (e.g.
-/// methods, functions in namespaces) are not accepted as C library
-/// functions.
-/// FIXME: If I understand it correctly, this discards calls where C++ code
-/// refers a C library function through the namespace `std::` via headers
-/// like .
+/// (This mode only matches functions that are declared either directly
+/// within a TU or in the `std::` namespace.)
CLibrary,
/// Matches "simple" functions that are not methods. (Static methods are
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
index d6d4cec9dd3d4d..c1ae9b441d98d1 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -12,6 +12,7 @@
//===--===//
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Basic/Builtins.h"
#include "clang/Lex/Lexer.h"
#include "llvm/ADT/StringExtras.h"
@@ -87,9 +88,11 @@ bool CheckerContext::isCLibraryFunction(const FunctionDecl
*FD,
if (!II)
return false;
- // Look through 'extern "C"' and anything similar invented in the future.
- // If this function is not in TU directly, it is not a C library function.
- if (!FD->getDeclContext()->getRedeclContext()->isTranslationUnit())
+ // C library functions are either declared directly within a TU (the common
+ // case) or they are accessed through the namespace `std::` (when they are
+ // used in C++ via headers like ).
+ if (!FD->getDeclContext()->getRedeclContext()->isTranslationUnit() &&
+ !AnalysisDeclContext::isInStdNamespace(FD))
return false;
// If this function is not externally visible, it is not a C library
function.
>From 6afadd86a6ee790f58c7339dc019d8c8eac8a6b9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?=
Date: Fri, 8 Mar 2024 13:22:27 +0100
Subject: [PATCH 2/4] Don't match methods from the namespace `std`
Only accept those functions whose declaration is _directly_ within the
namespace `std` (that is, not within a class or a sub-namespace).
Transparent declaration contexts (e.g. `extern "C"`) are still allowed,
but this prevents matching methods.
---
clang/lib/StaticAnalyzer/Core/CheckerContext.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
index c1ae9b441d98d1..5e706d17eeae14 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -12,7 +12,6 @@
//===--===//
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
-#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Basic/Builtins.h"
#include "clang/Lex/Lexer.h"
#include "llvm/ADT/StringExtras.h"
@@ -91,8 +90,8 @@ bool CheckerContext::isCLibraryFunction(const FunctionDecl
*FD,
// C library functions are either declared directly within a TU (the common
// case) or they are accessed through the namespace `s
[clang] [clang][nullability] allow _Nonnull etc on nullable class types (PR #82705)
@@ -2156,9 +2156,10 @@ def TypeNonNull : TypeAttr {
let Documentation = [TypeNonNullDocs];
}
-def TypeNullable : TypeAttr {
+def TypeNullable : DeclOrTypeAttr {
let Spellings = [CustomKeyword<"_Nullable">];
let Documentation = [TypeNullableDocs];
+// let Subjects = SubjectList<[CXXRecord], ErrorDiag>;
sam-mccall wrote:
With this enabled, the validation will *only* accept `_Nullable` on class
declarations, not on types. As far as I can tell, SubjectList can only describe
declarations.
Other DeclOrTypeAttrs use a commented-out `Subjects` like this, I assume as
documentation, so I did it here for consistency. I can drop it if you want...
https://github.com/llvm/llvm-project/pull/82705
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
=?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy ,Balazs Benics Message-ID: In-Reply-To: NagyDonat wrote: @steakhal Thanks for contributing the unit tests, I applied your commit. https://github.com/llvm/llvm-project/pull/84469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
=?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy ,Balazs Benics Message-ID: In-Reply-To: https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/84469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
=?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy ,Balazs Benics Message-ID: In-Reply-To: https://github.com/steakhal approved this pull request. I'm good now. https://github.com/llvm/llvm-project/pull/84469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,Balazs Benics
Message-ID:
In-Reply-To:
@@ -0,0 +1,89 @@
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/AnalysisDeclContext.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+#include
+
+using namespace clang;
+using namespace ento;
+using namespace ast_matchers;
+
+testing::AssertionResult extractFunctionDecl(StringRef Code,
+ const FunctionDecl *&Result) {
+ auto ASTUnit = tooling::buildASTFromCode(Code);
+ if (!ASTUnit)
+return testing::AssertionFailure() << "AST construction failed";
+
+ ASTContext &Context = ASTUnit->getASTContext();
+ if (Context.getDiagnostics().hasErrorOccurred())
+return testing::AssertionFailure() << "Compilation error";
+
+ auto Matches = ast_matchers::match(functionDecl().bind("fn"), Context);
+ if (Matches.empty())
+return testing::AssertionFailure() << "No function declaration found";
+
+ if (Matches.size() > 1)
+return testing::AssertionFailure()
+ << "Multiple function declarations found";
+
+ Result = Matches[0].getNodeAs("fn");
+ return testing::AssertionSuccess();
+}
+
+TEST(IsCLibraryFunctionTest, AcceptsGlobal) {
+ const FunctionDecl *Result;
+ ASSERT_TRUE(extractFunctionDecl(R"cpp(void fun();)cpp", Result));
+ EXPECT_TRUE(CheckerContext::isCLibraryFunction(Result));
+}
+
+TEST(IsCLibraryFunctionTest, AcceptsExternCGlobal) {
+ const FunctionDecl *Result;
+ ASSERT_TRUE(
+ extractFunctionDecl(R"cpp(extern "C" { void fun(); })cpp", Result));
+ EXPECT_TRUE(CheckerContext::isCLibraryFunction(Result));
+}
+
+TEST(IsCLibraryFunctionTest, AcceptsStaticGlobal) {
+ const FunctionDecl *Result;
+ ASSERT_TRUE(extractFunctionDecl(R"cpp(static void fun();)cpp", Result));
+ EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result));
+ // FIXME: Shouldn't this be TRUE?
+}
steakhal wrote:
What do you all think about this case?
I thought `static` means the same for `C` in this context, and as such it
should be accepted by `isCLibraryFunction` - at least that's what I'd expect.
This behavior was likely broken before, thus I don't expect direct actions on
this.
https://github.com/llvm/llvm-project/pull/84469
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][nullability] allow _Nonnull etc on nullable class types (PR #82705)
@@ -4096,6 +4096,11 @@ non-underscored keywords. For example: @property (assign, nullable) NSView *superview; @property (readonly, nonnull) NSArray *subviews; @end + +As well as built-in pointer types, ithe nullability attributes can be attached +to nullable types from the C++ standard library such as ``std::unique_ptr`` and sam-mccall wrote: Done. This makes it easier to get out of sync, but I agree comprehensive docs are important. (Inside Google we often just point people at the code and are able to deploy fixes quickly, so docs for these details are less critical and skew is more of a concern. So I often have blind spots for this stuff...) https://github.com/llvm/llvm-project/pull/82705 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Set up executor implicitly to account for init PTUs (PR #84758)
@@ -14,7 +14,7 @@ struct A { int a; A(int a) : a(a) {} virtual ~A(); };
// PartialTranslationUnit.
inline A::~A() { printf("~A(%d)\n", a); }
-// Create one instance with new and delete it.
+// Create one instance with new and delete it. We crash here now:
A *a1 = new A(1);
weliveindetail wrote:
With `-O2` this test fails unexpectedly now. Minimal repro:
```
clang-repl> struct A { int a; A(int a) : a(a) {} virtual ~A() {} };
clang-repl> A *a1 = new A(1);
clang-repl: llvm/include/llvm/ADT/SmallVector.h:308: const_reference
llvm::SmallVectorTemplateCommon::operator[](size_type)
const [T = llvm::PointerAlignElem]: Assertion `idx < size()' failed.
```
The following still works and thus I assume it's related to
https://github.com/llvm/llvm-project/commit/c861d32d7c2791bdc058d9d9fbaecc1c2f07b8c7:
```
clang-repl> struct A { int a; A(int a) : a(a) {} virtual ~A() {} }; A *a1 = new
A(1);
```
@hahnjo Maybe we now process init code that clashes with your above fix. Do you
think that's possible? Any idea how to handle it? If possible, I'd like to
submit the patch as-is with the test `XFAIL`ed for later investigation.
https://github.com/llvm/llvm-project/pull/84758
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][C++23] Implement P2448R2: Relaxing some constexpr restrictions (PR #77753)
stbergmann wrote: I think the discourse thread [Clang is overly aggressive when evaluating constexpr functions](https://discourse.llvm.org/t/clang-is-overly-aggressive-when-evaluating-constexpr-functions/67006) is relevant here. https://github.com/llvm/llvm-project/pull/77753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][nullability] allow _Nonnull etc on nullable class types (PR #82705)
@@ -1494,6 +1494,15 @@ void
Parser::ParseMicrosoftInheritanceClassAttributes(ParsedAttributes &attrs) {
}
}
+void Parser::ParseNullabilityClassAttributes(ParsedAttributes &attrs) {
+ while (Tok.is(tok::kw__Nullable)) {
sam-mccall wrote:
It's not important that we accept it, but I think better than changing this to
`if`:
- consistent with other attributes: we accept `struct [[nodiscard]]
[[nodiscard]] S{};`
- if we forbid this here by not parsing the second `_Nullable` as an attribute,
we'll parse it as a class name instead which I think is unlikely to be the
intent. The follow-on diagnostics are pretty bad.
- on the other hand, if we forbid this elsewhere by checking the attribute
list, this costs extra ad-hoc code that (I think) we've decided isn't worth it
for other attributes
https://github.com/llvm/llvm-project/pull/82705
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][nullability] allow _Nonnull etc on nullable class types (PR #82705)
https://github.com/sam-mccall edited https://github.com/llvm/llvm-project/pull/82705 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Driver] Default -msmall-data-limit= to 0 and clean up code (PR #83093)
kito-cheng wrote: There is some discussion in last (2024/2/29) LLVM sync up meeting: We all agree that might not useful in linux target and those platforms disable GP relaxation, like Android and fuchsia; However it's still useful for embedded toolchain, so this change may surprise those embedded toolchain user - cause some code size regression due to this change; although lld NOT default enable GP relaxation and also not implement all GP relaxation (`R_RISCV_PCREL_HI20` + `R_RISCV_PCREL_LO12_I` or `R_RISCV_PCREL_LO12_S` is missing), but user may use GNU linker or proprietary linker, so people would like to keep some case using same default in some case. --- Back to the implementation - yes, GCC is not support `-G` in this case, but why clang support `-G`? it's kinda inherit from the MIPS and Hexagon, they defined `-msmall-data-threshold` and then alias to `-G`, so RISC-V also follow the same convention: `-msmall-data-limit=` is alias to `-G` (that's my understanding after archaeology, although I work with Shiva at that timing but I don't have too much memory about that). > -mcmodel=large doesn't work for RISC-V yet, so the special case is strange. My best guess is that is because @ShivaChen was working for Andes at that timing, and the patch is extracted from downstream source tree and it already has that logic, and no body notice that during review process...:P [1] https://reviews.llvm.org/D57497 https://github.com/llvm/llvm-project/pull/83093 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add bit-precise overloads for builtin operators (PR #84755)
@@ -8955,6 +8962,22 @@ class BuiltinOperatorOverloadBuilder {
(S.Context.getAuxTargetInfo() &&
S.Context.getAuxTargetInfo()->hasInt128Type()))
ArithmeticTypes.push_back(S.Context.UnsignedInt128Ty);
+
+/// _BitInt overloads are a bit special. We don't want to add candidates
+/// for the entire family of _BitInt types, so instead we only add
+/// candidates for the unique, unqualified _BitInt types present in the
+/// candidate type set. The candidate set already handled ensuring the type
cor3ntin wrote:
This comment is a bit weird imo. "We don't want to add candidates for the
entire family of _BitInt types" seems a bit obvious (and if we wanted to do
that, wouldn't we need a lot more special handling any way?
https://github.com/llvm/llvm-project/pull/84755
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add bit-precise overloads for builtin operators (PR #84755)
@@ -8955,6 +8962,22 @@ class BuiltinOperatorOverloadBuilder {
(S.Context.getAuxTargetInfo() &&
S.Context.getAuxTargetInfo()->hasInt128Type()))
ArithmeticTypes.push_back(S.Context.UnsignedInt128Ty);
+
+/// _BitInt overloads are a bit special. We don't want to add candidates
+/// for the entire family of _BitInt types, so instead we only add
+/// candidates for the unique, unqualified _BitInt types present in the
+/// candidate type set. The candidate set already handled ensuring the type
+/// is unqualified and canonical, but because we're adding from N different
+/// sets, we need to do some extra work to unique things. Copy the
+/// candidates into a unique set, then copy from that set into the list of
+/// arithmetic types.
+llvm::SmallSetVector BitIntCandidates;
+llvm::for_each(CandidateTypes, [&BitIntCandidates](
+ BuiltinCandidateTypeSet &Candidate) {
+ for (QualType BitTy : Candidate.bitint_types())
+BitIntCandidates.insert(CanQualType::CreateUnsafe(BitTy));
+});
+llvm::copy(BitIntCandidates, std::back_inserter(ArithmeticTypes));
cor3ntin wrote:
```suggestion
llvm::move(BitIntCandidates, std::back_inserter(ArithmeticTypes));
```
https://github.com/llvm/llvm-project/pull/84755
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Add bit-precise overloads for builtin operators (PR #84755)
@@ -8955,6 +8962,22 @@ class BuiltinOperatorOverloadBuilder {
(S.Context.getAuxTargetInfo() &&
S.Context.getAuxTargetInfo()->hasInt128Type()))
ArithmeticTypes.push_back(S.Context.UnsignedInt128Ty);
+
+/// _BitInt overloads are a bit special. We don't want to add candidates
+/// for the entire family of _BitInt types, so instead we only add
+/// candidates for the unique, unqualified _BitInt types present in the
+/// candidate type set. The candidate set already handled ensuring the type
+/// is unqualified and canonical, but because we're adding from N different
+/// sets, we need to do some extra work to unique things. Copy the
+/// candidates into a unique set, then copy from that set into the list of
+/// arithmetic types.
+llvm::SmallSetVector BitIntCandidates;
+llvm::for_each(CandidateTypes, [&BitIntCandidates](
+ BuiltinCandidateTypeSet &Candidate) {
+ for (QualType BitTy : Candidate.bitint_types())
+BitIntCandidates.insert(CanQualType::CreateUnsafe(BitTy));
+});
+llvm::copy(BitIntCandidates, std::back_inserter(ArithmeticTypes));
erichkeane wrote:
I'm concerned about the assert on `9001`. It seems to me that this changes us
from being a reasonably 'fixed' list to having a perhaps-unlimited list.
I think that the intent of that assert is nice (ensuring we can
small-vector-opt this list), but this section here makes that no longer
guaranteed, right?
The `ArithmeticTypesCap` is 24, and we do a `push_back` of 23 other types (by
my count!). So adding TWO bit-int types + having all the rest of the types
enabled causes that assert to fire.
https://github.com/llvm/llvm-project/pull/84755
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HIP] Make the new driver bundle outputs for device-only (PR #84534)
https://github.com/jhuber6 updated
https://github.com/llvm/llvm-project/pull/84534
>From 1d22692bf3133bbab33485b5e8d65239a7386200 Mon Sep 17 00:00:00 2001
From: Joseph Huber
Date: Fri, 8 Mar 2024 12:49:38 -0600
Subject: [PATCH] [HIP] Make the new driver bundle outputs for device-only
Summary:
The current behavior of HIP is that when --offload-device-only is set it
still bundles the outputs into a fat binary. Even though this is
different from how all the other targets handle this, it seems to be
dependned on by some tooling so just make it backwards compatible for
the `-fno-gpu-rdc` case.
---
clang/lib/Driver/Driver.cpp | 9 -
clang/test/Driver/hip-binding.hip | 10 +++---
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index fce43430a91374..21d116656ed13e 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -4638,7 +4638,10 @@ Action *Driver::BuildOffloadingActions(Compilation &C,
}
}
- if (offloadDeviceOnly())
+ // All kinds exit now in device-only mode except for non-RDC mode HIP.
+ if (offloadDeviceOnly() &&
+ (!C.isOffloadingHostKind(Action::OFK_HIP) ||
+ Args.hasFlag(options::OPT_fgpu_rdc, options::OPT_fno_gpu_rdc, false)))
return C.MakeAction(DDeps, types::TY_Nothing);
if (OffloadActions.empty())
@@ -4671,6 +4674,10 @@ Action *Driver::BuildOffloadingActions(Compilation &C,
nullptr, C.getActiveOffloadKinds());
}
+ // HIP wants '--offload-device-only' to create a fatbinary by default.
+ if (offloadDeviceOnly())
+return C.MakeAction(DDep, types::TY_Nothing);
+
// If we are unable to embed a single device output into the host, we need to
// add each device output as a host dependency to ensure they are still
built.
bool SingleDeviceOutput = !llvm::any_of(OffloadActions, [](Action *A) {
diff --git a/clang/test/Driver/hip-binding.hip
b/clang/test/Driver/hip-binding.hip
index 79ec2039edb74c..cb17112c28d90a 100644
--- a/clang/test/Driver/hip-binding.hip
+++ b/clang/test/Driver/hip-binding.hip
@@ -64,10 +64,14 @@
// MULTI-D-ONLY-NEXT: # "amdgcn-amd-amdhsa" - "clang", inputs: ["[[INPUT]]"],
output: "[[GFX90a:.+]]"
// MULTI-D-ONLY-NEXT: # "amdgcn-amd-amdhsa" - "AMDGCN::Linker", inputs:
["[[GFX90a]]"], output: "[[GFX90a_OUT:.+]]"
//
-// RUN: not %clang -### --target=x86_64-linux-gnu --offload-new-driver
-ccc-print-bindings -nogpulib -nogpuinc \
-// RUN:--offload-arch=gfx90a --offload-arch=gfx908
--offload-device-only -c -o %t %s 2>&1 \
+// RUN: %clang -### --target=x86_64-linux-gnu --offload-new-driver
-ccc-print-bindings -nogpulib -nogpuinc \
+// RUN:--offload-arch=gfx90a --offload-arch=gfx908
--offload-device-only -c -o a.out %s 2>&1 \
// RUN: | FileCheck -check-prefix=MULTI-D-ONLY-O %s
-// MULTI-D-ONLY-O: error: cannot specify -o when generating multiple output
files
+// MULTI-D-ONLY-O: "amdgcn-amd-amdhsa" - "clang", inputs:
["[[INPUT:.+]]"], output: "[[GFX908_OBJ:.+]]"
+// MULTI-D-ONLY-O-NEXT: "amdgcn-amd-amdhsa" - "AMDGCN::Linker", inputs:
["[[GFX908_OBJ]]"], output: "[[GFX908:.+]]"
+// MULTI-D-ONLY-O-NEXT: "amdgcn-amd-amdhsa" - "clang", inputs: ["[[INPUT]]"],
output: "[[GFX90A_OBJ:.+]]"
+// MULTI-D-ONLY-O-NEXT: "amdgcn-amd-amdhsa" - "AMDGCN::Linker", inputs:
["[[GFX90A_OBJ]]"], output: "[[GFX90A:.+]]"
+// MULTI-D-ONLY-O-NEXT: "amdgcn-amd-amdhsa" - "AMDGCN::Linker", inputs:
["[[GFX908]]", "[[GFX90A]]"], output: "a.out"
//
// Check to ensure that we can use '-fsyntax-only' for HIP output with the new
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema]: Allow flexible arrays in unions and alone in structs (PR #84428)
erichkeane wrote: > > Left my comment on the main list, but I don't see this as a well motivated > > change, and even if GCC supported it, it would still be a very difficult to > > motivate extension without massive historical workloads already using it. > > This is needed by the Linux kernel, and is in active use. Directly converting > from [0] to [] isn't possible due to all the places flex arrays are used in > unions. Linux is working around this by tricking the syntax checker > currently, which needlessly complicates things. There are currently over 200 > separate unions using the work-around. > > So, this fixes the accidental lack of the existing [0] extensions not being > directly applied to [], and doesn't cause problems for any other users. What > solution should Linux use if not fixing this directly nor providing something > like -fflex-array-extensions? I don't see this as being 'accidental', see the GCC bug, Martin makes good points: with the `0`, this is a valid extension point, without, it is not. It seems that Martin @ GCC isn't willing to make this change, or at least isn't compelled either, so I'd prefer whatever we do to be in lockstep with them, if we choose to do anything. https://github.com/llvm/llvm-project/pull/84428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][nullability] allow _Nonnull etc on nullable class types (PR #82705)
@@ -215,6 +215,18 @@ void Sema::inferGslOwnerPointerAttribute(CXXRecordDecl
*Record) {
inferGslPointerAttribute(Record, Record);
}
+void Sema::inferNullableClassAttribute(CXXRecordDecl *CRD) {
+ static llvm::StringSet<> Nullable{
+ "auto_ptr", "shared_ptr", "unique_ptr", "exception_ptr",
sam-mccall wrote:
I don't think our nullability concept is a good match for weak pointers - they
don't behave like other pointers.
If a managed object is shared across threads, then the nullable/non-null
distinction isn't useful. Allowing it to be used in contracts is probably
harmful (attractive nuisance & breaks analyses that treat nullable objects
generically). Since pointers may become null at any point without running any
code, implementations should always treat them as nullable, generic static
analysis gives incorrect results, and dynamic checks are racy. Threads are
involved reasonably often and it's not locally possible to tell.
In the absence of threads, `weak_ptr`s that must not be null should almost
always be `shared_ptr _Nonnull` instead, and probably are. (This isn't a
strong objection: `T* _Nonnull` is useful despite `T&` existing - but raw
pointers are *way* more common than weak ones).
https://github.com/llvm/llvm-project/pull/82705
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][nullability] allow _Nonnull etc on nullable class types (PR #82705)
@@ -5955,6 +5955,20 @@ static void handleBuiltinAliasAttr(Sema &S, Decl *D,
D->addAttr(::new (S.Context) BuiltinAliasAttr(S.Context, AL, Ident));
}
+static void handleNullableTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+ if (AL.isUsedAsTypeAttr())
+return;
+
+ if (auto *CRD = dyn_cast(D);
+ !D || !(CRD->isClass() || CRD->isStruct())) {
sam-mccall wrote:
Oops, this was supposed to be `!CRD`! Fixed & will add a testcase.
> Struct as the Subject in Attr.td
Yeah, unfortunately that doesn't seem to work with attributes that can also
appear on types :-(
https://github.com/llvm/llvm-project/pull/82705
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][nullability] allow _Nonnull etc on nullable class types (PR #82705)
https://github.com/sam-mccall updated
https://github.com/llvm/llvm-project/pull/82705
>From eccc46840e343e7ba15200cd4b81316a51c46943 Mon Sep 17 00:00:00 2001
From: Sam McCall
Date: Thu, 22 Feb 2024 16:00:44 +0100
Subject: [PATCH 1/2] [clang][nullability] allow _Nonnull etc on nullable class
types
This enables clang and external nullability checkers to make use of
these annotations on nullable C++ class types like unique_ptr.
These types are recognized by the presence of the _Nullable attribute.
Nullable standard library types implicitly receive this attribute.
Existing static warnings for raw pointers are extended to smart pointers:
- nullptr used as return value or argument for non-null functions
(`-Wnonnull`)
- assigning or initializing nonnull variables with nullable values
(`-Wnullable-to-nonnull-conversion`)
It doesn't implicitly add these attributes based on the assume_nonnull
pragma, nor warn on missing attributes where the pragma would apply them.
I'm not confident that the pragma's current behavior will work well for
C++ (where type-based metaprogramming is much more common than C/ObjC).
We'd like to revisit this once we have more implementation experience.
Support can be detected as `__has_feature(nullability_on_classes)`.
This is needed for back-compatibility, as previously clang would issue a
hard error when _Nullable appears on a smart pointer.
UBSan's `-fsanitize=nullability` will not check smart-pointer types.
It can be made to do so by synthesizing calls to `operator bool`, but
that's left for future work.
---
clang/include/clang/Basic/Attr.td | 3 +-
clang/include/clang/Basic/AttrDocs.td | 16 +++
clang/include/clang/Basic/Features.def | 1 +
clang/include/clang/Parse/Parser.h | 1 +
clang/include/clang/Sema/Sema.h| 3 ++
clang/lib/AST/Type.cpp | 29 +++-
clang/lib/CodeGen/CGCall.cpp | 3 +-
clang/lib/CodeGen/CodeGenFunction.cpp | 3 +-
clang/lib/Parse/ParseDeclCXX.cpp | 33 ++
clang/lib/Sema/SemaAttr.cpp| 12 +
clang/lib/Sema/SemaChecking.cpp| 9
clang/lib/Sema/SemaDecl.cpp| 4 +-
clang/lib/Sema/SemaDeclAttr.cpp| 18
clang/lib/Sema/SemaInit.cpp| 5 +++
clang/lib/Sema/SemaOverload.cpp| 7 +++
clang/lib/Sema/SemaTemplate.cpp| 1 +
clang/lib/Sema/SemaType.cpp| 18 ++--
clang/test/SemaCXX/nullability.cpp | 62 +-
18 files changed, 199 insertions(+), 29 deletions(-)
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index fa191c7378dba4..f75aa126cbb939 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -2156,9 +2156,10 @@ def TypeNonNull : TypeAttr {
let Documentation = [TypeNonNullDocs];
}
-def TypeNullable : TypeAttr {
+def TypeNullable : DeclOrTypeAttr {
let Spellings = [CustomKeyword<"_Nullable">];
let Documentation = [TypeNullableDocs];
+// let Subjects = SubjectList<[CXXRecord], ErrorDiag>;
}
def TypeNullableResult : TypeAttr {
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index b96fbddd51154c..8c248fb052f19e 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -4096,6 +4096,11 @@ non-underscored keywords. For example:
@property (assign, nullable) NSView *superview;
@property (readonly, nonnull) NSArray *subviews;
@end
+
+As well as built-in pointer types, ithe nullability attributes can be attached
+to nullable types from the C++ standard library such as ``std::unique_ptr`` and
+``std::function``, as well as C++ classes marked with the ``_Nullable``
+attribute.
}];
}
@@ -4130,6 +4135,17 @@ The ``_Nullable`` nullability qualifier indicates that a
value of the
int fetch_or_zero(int * _Nullable ptr);
a caller of ``fetch_or_zero`` can provide null.
+
+The ``_Nullable`` attribute on classes indicates that the given class can
+represent null values, and so the ``_Nullable``, ``_Nonnull`` etc qualifiers
+make sense for this type. For example:
+
+ .. code-block:: c
+
+class _Nullable ArenaPointer { ... };
+
+ArenaPointer _Nonnull x = ...;
+ArenaPointer _Nullable y = nullptr;
}];
}
diff --git a/clang/include/clang/Basic/Features.def
b/clang/include/clang/Basic/Features.def
index 5fad5fc3623cb6..7c0755b7318306 100644
--- a/clang/include/clang/Basic/Features.def
+++ b/clang/include/clang/Basic/Features.def
@@ -94,6 +94,7 @@ EXTENSION(define_target_os_macros,
FEATURE(enumerator_attributes, true)
FEATURE(nullability, true)
FEATURE(nullability_on_arrays, true)
+FEATURE(nullability_on_classes, true)
FEATURE(nullability_nullable_result, true)
FEATURE(memory_sanitizer,
LangOpts.Sanitize.hasOneOf(SanitizerKind::Memory |
diff --git a/clang/include/clang/Parse/Parser.h
b/clang/include/clang/Parse/Parser.h
index 071520f535bc95..3d858
[clang] [llvm] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,Balazs Benics
Message-ID:
In-Reply-To:
@@ -0,0 +1,89 @@
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/AnalysisDeclContext.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+#include
+
+using namespace clang;
+using namespace ento;
+using namespace ast_matchers;
+
+testing::AssertionResult extractFunctionDecl(StringRef Code,
+ const FunctionDecl *&Result) {
+ auto ASTUnit = tooling::buildASTFromCode(Code);
+ if (!ASTUnit)
+return testing::AssertionFailure() << "AST construction failed";
+
+ ASTContext &Context = ASTUnit->getASTContext();
+ if (Context.getDiagnostics().hasErrorOccurred())
+return testing::AssertionFailure() << "Compilation error";
+
+ auto Matches = ast_matchers::match(functionDecl().bind("fn"), Context);
+ if (Matches.empty())
+return testing::AssertionFailure() << "No function declaration found";
+
+ if (Matches.size() > 1)
+return testing::AssertionFailure()
+ << "Multiple function declarations found";
+
+ Result = Matches[0].getNodeAs("fn");
+ return testing::AssertionSuccess();
+}
+
+TEST(IsCLibraryFunctionTest, AcceptsGlobal) {
+ const FunctionDecl *Result;
+ ASSERT_TRUE(extractFunctionDecl(R"cpp(void fun();)cpp", Result));
+ EXPECT_TRUE(CheckerContext::isCLibraryFunction(Result));
+}
+
+TEST(IsCLibraryFunctionTest, AcceptsExternCGlobal) {
+ const FunctionDecl *Result;
+ ASSERT_TRUE(
+ extractFunctionDecl(R"cpp(extern "C" { void fun(); })cpp", Result));
+ EXPECT_TRUE(CheckerContext::isCLibraryFunction(Result));
+}
+
+TEST(IsCLibraryFunctionTest, AcceptsStaticGlobal) {
+ const FunctionDecl *Result;
+ ASSERT_TRUE(extractFunctionDecl(R"cpp(static void fun();)cpp", Result));
+ EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result));
+ // FIXME: Shouldn't this be TRUE?
+}
NagyDonat wrote:
The method `isCLibraryFunction()` applies a heuristic that C library functions
must be externally visible (unless they're inlined):
```cpp
// If this function is not externally visible, it is not a C library function.
// Note that we make an exception for inline functions, which may be
// declared in header files without external linkage.
if (!FD->isInlined() && !FD->isExternallyVisible())
return false;
```
I think this is a reasonable heuristic that should not cause false negatives (I
don't think that we can have a library function that isn't inlined and isn't
externally visible) and might prevent a few false positives.
https://github.com/llvm/llvm-project/pull/84469
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Fix crash when using name of UnresolvedUsingValueDecl with template arguments (PR #83842)
sdkrystian wrote: So, it seems that this crash occurs because we filter out all non-template functions, which will trigger ADL if the only class member we found was a non-template function. https://github.com/llvm/llvm-project/pull/83842 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy ,Balazs Benics
Message-ID:
In-Reply-To:
@@ -0,0 +1,89 @@
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/AnalysisDeclContext.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+#include
+
+using namespace clang;
+using namespace ento;
+using namespace ast_matchers;
+
+testing::AssertionResult extractFunctionDecl(StringRef Code,
+ const FunctionDecl *&Result) {
+ auto ASTUnit = tooling::buildASTFromCode(Code);
+ if (!ASTUnit)
+return testing::AssertionFailure() << "AST construction failed";
+
+ ASTContext &Context = ASTUnit->getASTContext();
+ if (Context.getDiagnostics().hasErrorOccurred())
+return testing::AssertionFailure() << "Compilation error";
+
+ auto Matches = ast_matchers::match(functionDecl().bind("fn"), Context);
+ if (Matches.empty())
+return testing::AssertionFailure() << "No function declaration found";
+
+ if (Matches.size() > 1)
+return testing::AssertionFailure()
+ << "Multiple function declarations found";
+
+ Result = Matches[0].getNodeAs("fn");
+ return testing::AssertionSuccess();
+}
+
+TEST(IsCLibraryFunctionTest, AcceptsGlobal) {
+ const FunctionDecl *Result;
+ ASSERT_TRUE(extractFunctionDecl(R"cpp(void fun();)cpp", Result));
+ EXPECT_TRUE(CheckerContext::isCLibraryFunction(Result));
+}
+
+TEST(IsCLibraryFunctionTest, AcceptsExternCGlobal) {
+ const FunctionDecl *Result;
+ ASSERT_TRUE(
+ extractFunctionDecl(R"cpp(extern "C" { void fun(); })cpp", Result));
+ EXPECT_TRUE(CheckerContext::isCLibraryFunction(Result));
+}
+
+TEST(IsCLibraryFunctionTest, AcceptsStaticGlobal) {
+ const FunctionDecl *Result;
+ ASSERT_TRUE(extractFunctionDecl(R"cpp(static void fun();)cpp", Result));
+ EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result));
+ // FIXME: Shouldn't this be TRUE?
+}
NagyDonat wrote:
```suggestion
TEST(IsCLibraryFunctionTest, RejectsNoInlineNoExternalLinkage) {
// Functions that are neither inlined nor externally visible cannot be C
library functions.
const FunctionDecl *Result;
ASSERT_TRUE(extractFunctionDecl(R"cpp(static void fun();)cpp", Result));
EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result));
}
```
https://github.com/llvm/llvm-project/pull/84469
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
=?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy ,Balazs Benics Message-ID: In-Reply-To: https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/84469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce a tool 'clang-named-modules-querier' and two plugins 'ClangGetUsedFilesFromModulesPlugin' and 'ClangGetDeclsInModulesPlugin' (PR #72956)
mathstuf wrote: > ClangGetUsedFilesFromModulesPlugin This has a hole where if a currently-unused file is not listed, but it is changed in such a way that it now matters (e.g., it changes include order, adds/removes includes, etc.), we need to recompile consumers. > what happens if someone adds an overload, or other interesting name > resolution to the module? We would need to do (at least) one of: - track considered-but-discarded decls (e.g., if something SFINAE'd away now matters because of a new decl); - track "new" decls since the last compile (not sure how the state tracking works here though) and recompile if any show up > "col": 12, > "kind": "Function", > "line": 3, So we change the decl hash if a comment adds a line? That seems like low-hanging fruit to me. Can we enumerate decls and use an index instead? That depends on preprocessor state though, so may be hard to externally verify… https://github.com/llvm/llvm-project/pull/72956 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
