[clang] 1d2f727 - [clang][Sema] Improve `collectViableConversionCandidates` (#97908)
Author: MagentaTreehouse Date: 2024-09-14T09:00:48+02:00 New Revision: 1d2f7277e695d046efaf2818dd1a4b251ce745fd URL: https://github.com/llvm/llvm-project/commit/1d2f7277e695d046efaf2818dd1a4b251ce745fd DIFF: https://github.com/llvm/llvm-project/commit/1d2f7277e695d046efaf2818dd1a4b251ce745fd.diff LOG: [clang][Sema] Improve `collectViableConversionCandidates` (#97908) * Use range-based for * The value of `Conv` is not used when `ConvTemplate` is not null, so we do not need to compute it on that path Added: Modified: clang/lib/Sema/SemaOverload.cpp Removed: diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index a155bb2fd3ba2a..0c1e054f7c30a4 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -6567,29 +6567,22 @@ static void collectViableConversionCandidates(Sema &SemaRef, Expr *From, QualType ToType, UnresolvedSetImpl &ViableConversions, OverloadCandidateSet &CandidateSet) { - for (unsigned I = 0, N = ViableConversions.size(); I != N; ++I) { -DeclAccessPair FoundDecl = ViableConversions[I]; + for (const DeclAccessPair &FoundDecl : ViableConversions.pairs()) { NamedDecl *D = FoundDecl.getDecl(); CXXRecordDecl *ActingContext = cast(D->getDeclContext()); if (isa(D)) D = cast(D)->getTargetDecl(); -CXXConversionDecl *Conv; -FunctionTemplateDecl *ConvTemplate; -if ((ConvTemplate = dyn_cast(D))) - Conv = cast(ConvTemplate->getTemplatedDecl()); -else - Conv = cast(D); - -if (ConvTemplate) +if (auto *ConvTemplate = dyn_cast(D)) { SemaRef.AddTemplateConversionCandidate( ConvTemplate, FoundDecl, ActingContext, From, ToType, CandidateSet, - /*AllowObjCConversionOnExplicit=*/false, /*AllowExplicit*/ true); -else - SemaRef.AddConversionCandidate(Conv, FoundDecl, ActingContext, From, - ToType, CandidateSet, - /*AllowObjCConversionOnExplicit=*/false, - /*AllowExplicit*/ true); + /*AllowObjCConversionOnExplicit=*/false, /*AllowExplicit=*/true); + continue; +} +CXXConversionDecl *Conv = cast(D); +SemaRef.AddConversionCandidate( +Conv, FoundDecl, ActingContext, From, ToType, CandidateSet, +/*AllowObjCConversionOnExplicit=*/false, /*AllowExplicit=*/true); } } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Improve `collectViableConversionCandidates` (PR #97908)
https://github.com/cor3ntin closed https://github.com/llvm/llvm-project/pull/97908 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [TableGen] Avoid repeated map lookups (NFC) (PR #108675)
https://github.com/nikic approved this pull request. https://github.com/llvm/llvm-project/pull/108675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] issue-63565: community requested small QoL fix for more configurabili… (PR #108005)
MK-Alias wrote: Thanks for the input, a followup question is: Which one takes precedence? Does the `--function-arg-placeholders` argument take precedence over `.clangd` config!? Or the other way around!? https://github.com/llvm/llvm-project/pull/108005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] issue-63565: community requested small QoL fix for more configurabili… (PR #108005)
HighCommander4 wrote: > Thanks for the input, a followup question is: Which one takes precedence? > Does the `--function-arg-placeholders` argument take precedence over > `.clangd` config!? Or the other way around!? `.clangd` should take precedence because it's the a more specific setting (command line arguments apply to the whole clangd process, `.clangd` files are scoped to a directory). If you are handling the flag in `FlagsConfigProvider` as suggested, this will be the resulting behaviour will no further logic needed. https://github.com/llvm/llvm-project/pull/108005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [PowerPC] Fix incorrect store alignment for __builtin_vsx_build_pair() (PR #108606)
@@ -18197,7 +18197,7 @@ Value *CodeGenFunction::EmitPPCBuiltinExpr(unsigned BuiltinID, CallOps.push_back(Ops[i]); llvm::Function *F = CGM.getIntrinsic(ID); Value *Call = Builder.CreateCall(F, CallOps); -return Builder.CreateAlignedStore(Call, Ops[0], MaybeAlign(64)); +return Builder.CreateAlignedStore(Call, Ops[0], MaybeAlign()); nikic wrote: ```suggestion return Builder.CreateStore(Call, Ops[0]); ``` Should work, I think? No need to use CreateAlignedStore if you want a naturally aligned store. https://github.com/llvm/llvm-project/pull/108606 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] scan-build-py: respect LLVM_LIBDIR_SUFFIX like other tools do (PR #108549)
nikic wrote: It looks like you're trying to undo https://github.com/llvm/llvm-project/pull/106612? cc @mgorny https://github.com/llvm/llvm-project/pull/108549 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] scan-build-py: respect LLVM_LIBDIR_SUFFIX like other tools do (PR #108549)
shr-project wrote: > It looks like you're trying to undo #106612? cc @mgorny Ah, not intentionally, I should have checked the git history more carefully before creating this PR (which is still only a draft). I don't use libscanbuild at all, but in OpenEmbedded builds the python modules are installed in the configured libdir which could be /usr/lib, /usr/lib32, /usr/lib64 and then they are packaged correctly, while libscanbuild didn't follow this and caused installed-vs-shipped QA issue (which basically means that something was installed in locations which aren't packaged by configured rules). I believe the python code should be fixed to look in the right install locations, but I'll leave that to someone actually using libscanbuild. https://github.com/llvm/llvm-project/pull/108549 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ast-matcher] Fixed a crash when traverse lambda expr with invalid captures (PR #108689)
https://github.com/HerrCai0907 created https://github.com/llvm/llvm-project/pull/108689 Fixes: #106444 >From af8720ade42fef1571b59e2ca21943abc6b998d1 Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Sat, 14 Sep 2024 15:38:11 +0800 Subject: [PATCH] [ast-matcher] Fixed a crash when traverse lambda expr with invalid captures Fixes: #106444 --- clang/docs/ReleaseNotes.rst | 2 ++ clang/lib/ASTMatchers/ASTMatchFinder.cpp| 5 +++-- .../ASTMatchers/ASTMatchersTraversalTest.cpp| 13 + 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 79b154ef1aef5e..a6f4b4e602d571 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -504,6 +504,8 @@ AST Matchers - Fixed an ordering issue with the `hasOperands` matcher occuring when setting a binding in the first matcher and using it in the second matcher. +- Fixed a crash when traverse lambda expr with invalid captures. + clang-format diff --git a/clang/lib/ASTMatchers/ASTMatchFinder.cpp b/clang/lib/ASTMatchers/ASTMatchFinder.cpp index 0bac2ed63a927e..3d01a70395a9bb 100644 --- a/clang/lib/ASTMatchers/ASTMatchFinder.cpp +++ b/clang/lib/ASTMatchers/ASTMatchFinder.cpp @@ -285,12 +285,13 @@ class MatchChildASTVisitor ScopedIncrement ScopedDepth(&CurrentDepth); for (unsigned I = 0, N = Node->capture_size(); I != N; ++I) { - const auto *C = Node->capture_begin() + I; + const LambdaCapture *C = Node->capture_begin() + I; if (!C->isExplicit()) continue; if (Node->isInitCapture(C) && !match(*C->getCapturedVar())) return false; - if (!match(*Node->capture_init_begin()[I])) + const Expr *CIE = Node->capture_init_begin()[I]; + if (CIE != nullptr && !match(*CIE)) return false; } diff --git a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp index 028392f499da3b..ec0be27774d8b2 100644 --- a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp +++ b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp @@ -5052,6 +5052,19 @@ TEST(ForEachConstructorInitializer, MatchesInitializers) { cxxConstructorDecl(forEachConstructorInitializer(cxxCtorInitializer(); } +TEST(LambdaCapture, InvalidLambdaCapture) { + // not crash + EXPECT_FALSE(matches( + R"(int main() { +struct A { A()=default; A(A const&)=delete; }; +A a; [a]() -> void {}(); +return 0; + })", + traverse(TK_IgnoreUnlessSpelledInSource, + lambdaExpr(has(lambdaCapture(, + langCxx11OrLater())); +} + TEST(ForEachLambdaCapture, MatchesCaptures) { EXPECT_TRUE(matches( "int main() { int x, y; auto f = [x, y]() { return x + y; }; }", ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ast-matcher] Fixed a crash when traverse lambda expr with invalid captures (PR #108689)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Congcong Cai (HerrCai0907) Changes Fixes: #106444 --- Full diff: https://github.com/llvm/llvm-project/pull/108689.diff 3 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+2) - (modified) clang/lib/ASTMatchers/ASTMatchFinder.cpp (+3-2) - (modified) clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp (+13) ``diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 79b154ef1aef5e..a6f4b4e602d571 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -504,6 +504,8 @@ AST Matchers - Fixed an ordering issue with the `hasOperands` matcher occuring when setting a binding in the first matcher and using it in the second matcher. +- Fixed a crash when traverse lambda expr with invalid captures. + clang-format diff --git a/clang/lib/ASTMatchers/ASTMatchFinder.cpp b/clang/lib/ASTMatchers/ASTMatchFinder.cpp index 0bac2ed63a927e..3d01a70395a9bb 100644 --- a/clang/lib/ASTMatchers/ASTMatchFinder.cpp +++ b/clang/lib/ASTMatchers/ASTMatchFinder.cpp @@ -285,12 +285,13 @@ class MatchChildASTVisitor ScopedIncrement ScopedDepth(&CurrentDepth); for (unsigned I = 0, N = Node->capture_size(); I != N; ++I) { - const auto *C = Node->capture_begin() + I; + const LambdaCapture *C = Node->capture_begin() + I; if (!C->isExplicit()) continue; if (Node->isInitCapture(C) && !match(*C->getCapturedVar())) return false; - if (!match(*Node->capture_init_begin()[I])) + const Expr *CIE = Node->capture_init_begin()[I]; + if (CIE != nullptr && !match(*CIE)) return false; } diff --git a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp index 028392f499da3b..ec0be27774d8b2 100644 --- a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp +++ b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp @@ -5052,6 +5052,19 @@ TEST(ForEachConstructorInitializer, MatchesInitializers) { cxxConstructorDecl(forEachConstructorInitializer(cxxCtorInitializer(); } +TEST(LambdaCapture, InvalidLambdaCapture) { + // not crash + EXPECT_FALSE(matches( + R"(int main() { +struct A { A()=default; A(A const&)=delete; }; +A a; [a]() -> void {}(); +return 0; + })", + traverse(TK_IgnoreUnlessSpelledInSource, + lambdaExpr(has(lambdaCapture(, + langCxx11OrLater())); +} + TEST(ForEachLambdaCapture, MatchesCaptures) { EXPECT_TRUE(matches( "int main() { int x, y; auto f = [x, y]() { return x + y; }; }", `` https://github.com/llvm/llvm-project/pull/108689 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 918972b - [clang] Strip unneeded calls to raw_string_ostream::str() (NFC)
Author: JOE1994 Date: 2024-09-14T04:38:50-04:00 New Revision: 918972bded27de6a2bfacc15b4ad3edebd81f405 URL: https://github.com/llvm/llvm-project/commit/918972bded27de6a2bfacc15b4ad3edebd81f405 DIFF: https://github.com/llvm/llvm-project/commit/918972bded27de6a2bfacc15b4ad3edebd81f405.diff LOG: [clang] Strip unneeded calls to raw_string_ostream::str() (NFC) Avoid extra layer of indirection. p.s. Also, remove calls to raw_string_ostream::flush(), which are no-ops. Added: Modified: clang/tools/clang-refactor/ClangRefactor.cpp clang/tools/driver/cc1gen_reproducer_main.cpp clang/unittests/AST/SourceLocationTest.cpp clang/unittests/AST/TemplateNameTest.cpp clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp clang/unittests/Frontend/OutputStreamTest.cpp clang/unittests/StaticAnalyzer/RangeSetTest.cpp clang/unittests/Tooling/ASTSelectionTest.cpp clang/unittests/Tooling/DiagnosticsYamlTest.cpp clang/unittests/Tooling/RecursiveASTVisitorTestDeclVisitor.cpp clang/unittests/Tooling/RecursiveASTVisitorTests/TemplateArgumentLocTraverser.cpp clang/unittests/Tooling/RefactoringTest.cpp clang/unittests/Tooling/RewriterTestContext.h clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp clang/utils/TableGen/MveEmitter.cpp Removed: diff --git a/clang/tools/clang-refactor/ClangRefactor.cpp b/clang/tools/clang-refactor/ClangRefactor.cpp index 175a2b8234e9ac..9310263c446aea 100644 --- a/clang/tools/clang-refactor/ClangRefactor.cpp +++ b/clang/tools/clang-refactor/ClangRefactor.cpp @@ -560,7 +560,6 @@ class ClangRefactorTool { << "' can't be invoked with the given arguments:\n"; for (const auto &Opt : MissingOptions) OS << " missing '-" << Opt.getKey() << "' option\n"; - OS.flush(); return llvm::make_error( Error, llvm::inconvertibleErrorCode()); } @@ -591,7 +590,6 @@ class ClangRefactorTool { OS << "note: the following actions are supported:\n"; for (const auto &Subcommand : SubCommands) OS.indent(2) << Subcommand->getName() << "\n"; - OS.flush(); return llvm::make_error( Error, llvm::inconvertibleErrorCode()); } diff --git a/clang/tools/driver/cc1gen_reproducer_main.cpp b/clang/tools/driver/cc1gen_reproducer_main.cpp index e97fa3d2775667..be081cac8c03b1 100644 --- a/clang/tools/driver/cc1gen_reproducer_main.cpp +++ b/clang/tools/driver/cc1gen_reproducer_main.cpp @@ -105,8 +105,8 @@ static std::string generateReproducerMetaInfo(const ClangInvocationInfo &Info) { OS << '}'; // FIXME: Compare unsaved file hashes and report mismatch in the reproducer. if (Info.Dump) -llvm::outs() << "REPRODUCER METAINFO: " << OS.str() << "\n"; - return std::move(OS.str()); +llvm::outs() << "REPRODUCER METAINFO: " << Result << "\n"; + return Result; } /// Generates a reproducer for a set of arguments from a specific invocation. diff --git a/clang/unittests/AST/SourceLocationTest.cpp b/clang/unittests/AST/SourceLocationTest.cpp index 66daa56aace28a..daea2d62fe4968 100644 --- a/clang/unittests/AST/SourceLocationTest.cpp +++ b/clang/unittests/AST/SourceLocationTest.cpp @@ -105,7 +105,7 @@ class WhileParenLocationVerifier : public MatchVerifier { RParenLoc.print(Msg, *Result.SourceManager); Msg << ">"; - this->setFailure(Msg.str()); + this->setFailure(MsgStr); } } }; diff --git a/clang/unittests/AST/TemplateNameTest.cpp b/clang/unittests/AST/TemplateNameTest.cpp index 444ccfb5c9c811..2eac5c508d0595 100644 --- a/clang/unittests/AST/TemplateNameTest.cpp +++ b/clang/unittests/AST/TemplateNameTest.cpp @@ -21,7 +21,7 @@ std::string printTemplateName(TemplateName TN, const PrintingPolicy &Policy, std::string Result; llvm::raw_string_ostream Out(Result); TN.print(Out, Policy, Qual); - return Out.str(); + return Result; } TEST(TemplateName, PrintTemplate) { diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp index 9c4ec07e139a12..137baab53301ae 100644 --- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp +++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp @@ -78,7 +78,7 @@ mutatedBy(const SmallVectorImpl &Results, ASTUnit *AST) { std::string Buffer; llvm::raw_string_ostream Stream(Buffer); By->printPretty(Stream, nullptr, AST->getASTContext().getPrintingPolicy()); -Chain.emplace_back(StringRef(Stream.str()).trim().str()); +Chain.emplace_back(StringRef(Buffer).trim().str()); E = dyn_cast(By); } return Chain; diff --git a/clang/unittests/Frontend/OutputStreamTest.cpp b/clang/unittests/Frontend/OutputStreamTest.cpp index 7d360f661daa30..2618558c7e11ee 100644 --- a/clang/unittests/Frontend/OutputStreamTest.cpp +++ b/clang/unittests/Frontend/OutputStreamTest.cpp @@ -67,7 +67,7 @@ TEST(Fronte
[clang] scan-build-py: respect LLVM_LIBDIR_SUFFIX like other tools do (PR #108549)
mgorny wrote: Sounds like a bug in OpenEmbedded then. CPython installs Python modules in `/usr/lib`, unconditionally. https://github.com/llvm/llvm-project/pull/108549 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Fix a bug in annotating StartOfName (PR #99791)
alex1701c wrote: That is very unfortunate. It means one will either get different results for the clang-format versions, one disables the formatting, or works around it by sth. Like Q_EMIT(something()->mySignal()). https://github.com/llvm/llvm-project/pull/99791 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Start moving X86Builtins.def to X86Builtins.td (PR #106005)
philnik777 wrote: gentle ping~ https://github.com/llvm/llvm-project/pull/106005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy]suggest use `std::span` as replacement of c array in C++20 for modernize-avoid-c-arrays (PR #108555)
@@ -72,11 +81,24 @@ void AvoidCArraysCheck::registerMatchers(MatchFinder *Finder) { void AvoidCArraysCheck::check(const MatchFinder::MatchResult &Result) { const auto *ArrayType = Result.Nodes.getNodeAs("typeloc"); - + const bool IsInParam = + Result.Nodes.getNodeAs("param_decl") != nullptr; + const bool IsVLA = ArrayType->getTypePtr()->isVariableArrayType(); + enum class RecommendType { Array, Vector, Span }; + RecommendType RecommendType = RecommendType::Array; + if (IsVLA) { +RecommendType = RecommendType::Vector; + } else if (ArrayType->getTypePtr()->isIncompleteArrayType() && IsInParam) { +// in function parameter, we also don't know the size of +// IncompleteArrayType. +RecommendType = Result.Context->getLangOpts().CPlusPlus20 +? RecommendType::Span +: RecommendType::Vector; + } HerrCai0907 wrote: If I remember correctly, LLVM only has this requirement for simple if statement, but not for if - else if statement for readability. https://github.com/llvm/llvm-project/pull/108555 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libcxx] [Clang] Add __builtin_common_type (PR #99473)
philnik777 wrote: gentle ping~ https://github.com/llvm/llvm-project/pull/99473 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [FMV][AArch64] Unify ls64, ls64_v and ls64_accdata. (PR #108024)
@@ -73,8 +73,6 @@ enum CPUFeatures { FEAT_SSBS, FEAT_SSBS2, FEAT_BTI, - FEAT_LS64, - FEAT_LS64_V, Wilco1 wrote: It became a defacto ABI when the first implementation became available. We could change the names of the global/init function to explicitly break the ABI with old objects at linktime to avoid silent ABI breaks that lead to incorrect execution at runtime. https://github.com/llvm/llvm-project/pull/108024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy]suggest use `std::span` as replacement of c array in C++20 for modernize-avoid-c-arrays (PR #108555)
https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/108555 >From 35742b5f2f085b8f6626456cd0d21ebecbe8f831 Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Thu, 12 Sep 2024 23:11:51 +0800 Subject: [PATCH 1/3] [clang-tidy]suggest use `std::span` as replacement of c array in C++20 for modernize-avoid-c-arrays --- .../modernize/AvoidCArraysCheck.cpp | 26 --- clang-tools-extra/docs/ReleaseNotes.rst | 4 +++ .../modernize/avoid-c-arrays-c++20.cpp| 7 + .../modernize/avoid-c-arrays-ignores-main.cpp | 6 ++--- .../avoid-c-arrays-ignores-strings.cpp| 4 +-- .../avoid-c-arrays-ignores-three-arg-main.cpp | 10 +++ .../checkers/modernize/avoid-c-arrays.cpp | 4 +-- 7 files changed, 46 insertions(+), 15 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-c++20.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp b/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp index 89790ea70cf229..9fd0259ed1aacf 100644 --- a/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp @@ -9,6 +9,7 @@ #include "AvoidCArraysCheck.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" using namespace clang::ast_matchers; @@ -38,6 +39,13 @@ AST_MATCHER(clang::ParmVarDecl, isArgvOfMain) { return FD ? FD->isMain() : false; } +AST_MATCHER_P(clang::TypeLoc, alwaysTrue, + clang::ast_matchers::internal::Matcher, + InnerMatcher) { + InnerMatcher.matches(Node, Finder, Builder); + return true; +} + } // namespace AvoidCArraysCheck::AvoidCArraysCheck(StringRef Name, ClangTidyContext *Context) @@ -60,6 +68,7 @@ void AvoidCArraysCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( typeLoc(hasValidBeginLoc(), hasType(arrayType()), + alwaysTrue(hasParent(parmVarDecl().bind("param_decl"))), unless(anyOf(hasParent(parmVarDecl(isArgvOfMain())), hasParent(varDecl(isExternC())), hasParent(fieldDecl( @@ -72,11 +81,22 @@ void AvoidCArraysCheck::registerMatchers(MatchFinder *Finder) { void AvoidCArraysCheck::check(const MatchFinder::MatchResult &Result) { const auto *ArrayType = Result.Nodes.getNodeAs("typeloc"); - + bool const IsInParam = + Result.Nodes.getNodeAs("param_decl") != nullptr; + const bool IsVLA = ArrayType->getTypePtr()->isVariableArrayType(); + // in function parameter, we also don't know the size of IncompleteArrayType. + const bool IsUnknownSize = + IsVLA || (ArrayType->getTypePtr()->isIncompleteArrayType() && IsInParam); + enum class RecommendType { Array, Vector, Span }; + const RecommendType RecommendType = + (IsInParam && Result.Context->getLangOpts().CPlusPlus20) + ? RecommendType::Span + : IsUnknownSize ? RecommendType::Vector + : RecommendType::Array; diag(ArrayType->getBeginLoc(), "do not declare %select{C-style|C VLA}0 arrays, use " - "%select{std::array<>|std::vector<>}0 instead") - << ArrayType->getTypePtr()->isVariableArrayType(); + "%select{std::array<>|std::vector<>|std::span<>}1 instead") + << IsVLA << static_cast(RecommendType); } } // namespace clang::tidy::modernize diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 1ad8cedc902cb5..c5a2b3c36d130f 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -111,6 +111,10 @@ Changes in existing checks ` check to suggest replacing the offending code with ``reinterpret_cast``, to more clearly express intent. +- Improved :doc:`modernize-avoid-c-arrays + ` check to suggest use std::span + as replacement of c array in C++20. + - Improved :doc:`modernize-use-std-format ` check to support replacing member function calls too. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-c++20.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-c++20.cpp new file mode 100644 index 00..a9298245feaaeb --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-c++20.cpp @@ -0,0 +1,7 @@ +// RUN: %check_clang_tidy -std=c++20 %s modernize-avoid-c-arrays %t + +int foo(int data[], int size) { + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: do not declare C-style arrays, use std::span<> instead + int f4[] = {1, 2}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-ignores-main.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-ignores-main.cpp index 6549422f393aaa
[clang] [analyzer] Refactor MallocChecker to use `BindExpr` in `evalCall` (PR #106081)
pskrgag wrote: > LGTM, feel free to merge this patch. Thank you for review! I have no rights to push to llvm-project. So you can merge if you think it's good time to do so =) https://github.com/llvm/llvm-project/pull/106081 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 223e2ef - [clang] Nits on uses of raw_string_ostream (NFC)
Author: JOE1994 Date: 2024-09-14T05:29:40-04:00 New Revision: 223e2efa5e886502a9467b7ef700ebce9b7886e8 URL: https://github.com/llvm/llvm-project/commit/223e2efa5e886502a9467b7ef700ebce9b7886e8 DIFF: https://github.com/llvm/llvm-project/commit/223e2efa5e886502a9467b7ef700ebce9b7886e8.diff LOG: [clang] Nits on uses of raw_string_ostream (NFC) * Don't call raw_string_ostream::flush(), which is essentially a no-op. * Strip unneeded calls to raw_string_ostream::str(), to avoid extra indirection. Added: Modified: clang/lib/AST/APValue.cpp clang/lib/AST/DeclPrinter.cpp clang/lib/AST/Expr.cpp clang/lib/AST/Mangle.cpp clang/lib/AST/MicrosoftMangle.cpp clang/lib/AST/StmtViz.cpp clang/lib/ASTMatchers/Dynamic/Registry.cpp clang/lib/Analysis/CFG.cpp Removed: diff --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp index d8e33ff421c06c..4f5d14cbd59bbf 100644 --- a/clang/lib/AST/APValue.cpp +++ b/clang/lib/AST/APValue.cpp @@ -947,7 +947,6 @@ std::string APValue::getAsString(const ASTContext &Ctx, QualType Ty) const { std::string Result; llvm::raw_string_ostream Out(Result); printPretty(Out, Ctx, Ty); - Out.flush(); return Result; } diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp index 07be813abd8adc..0d51fdbc7e1262 100644 --- a/clang/lib/AST/DeclPrinter.cpp +++ b/clang/lib/AST/DeclPrinter.cpp @@ -629,7 +629,6 @@ static void printExplicitSpecifier(ExplicitSpecifier ES, llvm::raw_ostream &Out, EOut << ")"; } EOut << " "; - EOut.flush(); Out << Proto; } @@ -790,7 +789,6 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) { llvm::raw_string_ostream EOut(Proto); FT->getNoexceptExpr()->printPretty(EOut, nullptr, SubPolicy, Indentation, "\n", &Context); -EOut.flush(); Proto += ")"; } } diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index e10142eff8ec47..2e463fc00c6b68 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -612,7 +612,7 @@ std::string SYCLUniqueStableNameExpr::ComputeName(ASTContext &Context, llvm::raw_string_ostream Out(Buffer); Ctx->mangleCanonicalTypeName(Ty, Out); - return Out.str(); + return Buffer; } PredefinedExpr::PredefinedExpr(SourceLocation L, QualType FNTy, @@ -798,7 +798,6 @@ std::string PredefinedExpr::ComputeName(PredefinedIdentKind IK, FD->printQualifiedName(POut, Policy); if (IK == PredefinedIdentKind::Function) { - POut.flush(); Out << Proto; return std::string(Name); } @@ -880,15 +879,12 @@ std::string PredefinedExpr::ComputeName(PredefinedIdentKind IK, } } -TOut.flush(); if (!TemplateParams.empty()) { // remove the trailing comma and space TemplateParams.resize(TemplateParams.size() - 2); POut << " [" << TemplateParams << "]"; } -POut.flush(); - // Print "auto" for all deduced return types. This includes C++1y return // type deduction and lambdas. For trailing return types resolve the // decltype expression. Otherwise print the real type when this is diff --git a/clang/lib/AST/Mangle.cpp b/clang/lib/AST/Mangle.cpp index 75f6e2161a6338..4875e8537b3c11 100644 --- a/clang/lib/AST/Mangle.cpp +++ b/clang/lib/AST/Mangle.cpp @@ -574,9 +574,9 @@ class ASTNameGenerator::Implementation { std::string BackendBuf; llvm::raw_string_ostream BOS(BackendBuf); -llvm::Mangler::getNameWithPrefix(BOS, FOS.str(), DL); +llvm::Mangler::getNameWithPrefix(BOS, FrontendBuf, DL); -return BOS.str(); +return BackendBuf; } std::string getMangledThunk(const CXXMethodDecl *MD, const ThunkInfo &T, @@ -589,9 +589,9 @@ class ASTNameGenerator::Implementation { std::string BackendBuf; llvm::raw_string_ostream BOS(BackendBuf); -llvm::Mangler::getNameWithPrefix(BOS, FOS.str(), DL); +llvm::Mangler::getNameWithPrefix(BOS, FrontendBuf, DL); -return BOS.str(); +return BackendBuf; } }; diff --git a/clang/lib/AST/MicrosoftMangle.cpp b/clang/lib/AST/MicrosoftMangle.cpp index 018ab617a0ecee..7b069c66aed598 100644 --- a/clang/lib/AST/MicrosoftMangle.cpp +++ b/clang/lib/AST/MicrosoftMangle.cpp @@ -1396,7 +1396,7 @@ void MicrosoftCXXNameMangler::mangleNestedName(GlobalDecl GD) { Stream << '_' << Discriminator; if (ParameterDiscriminator) Stream << '_' << ParameterDiscriminator; -return Stream.str(); +return Buffer; }; unsigned Discriminator = BD->getBlockManglingNumber(); diff --git a/clang/lib/AST/StmtViz.cpp b/clang/lib/AST/StmtViz.cpp index 4eb0da8a0e105f..c86363031eb53d 100644 --- a/clang/lib/AST/StmtViz.cpp +++ b/clang/lib/AST/StmtViz.cpp @@ -34,15 +34,14 @@ struct DOTGraphTraits : public DefaultDOTGraphTraits { static std::string getNodeLa
[clang] [Clang] Avoid transforming lambdas when rebuilding immediate expressions (PR #108693)
https://github.com/zyn0217 created https://github.com/llvm/llvm-project/pull/108693 When rebuilding immediate invocations inside `RemoveNestedImmediateInvocation()`, we employed a `TreeTransform` to exercise the traversal. The transformation has a side effect that, for template specialization types, their default template arguments are substituted separately, and if any lambdas are present, they will be transformed into distinct types than those used to instantiate the templates right before the `consteval` handling. This resulted in `B::func()` getting redundantly instantiated for the case in question. Since we're also in an immediate evaluation context, the body of `foo()` would also get instantiated, so we end up with a spurious friend redefinition error. Like what we have done in `ComplexRemove`, this patch also avoids the lambda's transformation in TemplateInstantiator if we know we're rebuilding immediate calls. In addition, this patch also consolidates the default argument substitution logic in `CheckTemplateArgumentList()`. Fixes #107175 >From 4e05a1972e539d08589b44677031f506b0c91203 Mon Sep 17 00:00:00 2001 From: Younan Zhang Date: Sat, 14 Sep 2024 18:31:42 +0800 Subject: [PATCH] [Clang] Avoid transforming lambdas when rebuilding immediate expressions --- clang/docs/ReleaseNotes.rst | 4 +- clang/lib/Sema/SemaExpr.cpp | 4 +- clang/lib/Sema/SemaTemplate.cpp | 55 ++- clang/lib/Sema/SemaTemplateInstantiate.cpp| 4 ++ .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 10 +++- clang/test/SemaCXX/cxx2a-consteval.cpp| 24 6 files changed, 57 insertions(+), 44 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 79b154ef1aef5e..022c10e11c545a 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -387,8 +387,8 @@ Bug Fixes to C++ Support - A follow-up fix was added for (#GH61460), as the previous fix was not entirely correct. (#GH86361) - Fixed a crash in the typo correction of an invalid CTAD guide. (#GH107887) - Fixed a crash when clang tries to subtitute parameter pack while retaining the parameter - pack. #GH63819, #GH107560 - + pack. (#GH63819), (#GH107560) +- Avoided a redundant friend declaration instantiation under the certain ``consteval`` contexts. (#GH107175) Bug Fixes to AST Handling ^ diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 8f3e15cc9a9bb7..2a22b05afe1c34 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -17521,7 +17521,7 @@ static void RemoveNestedImmediateInvocation( else break; } - /// ConstantExpr are the first layer of implicit node to be removed so if + /// ConstantExprs are the first layer of implicit node to be removed so if /// Init isn't a ConstantExpr, no ConstantExpr will be skipped. if (auto *CE = dyn_cast(Init); CE && CE->isImmediateInvocation()) @@ -17534,7 +17534,7 @@ static void RemoveNestedImmediateInvocation( } ExprResult TransformLambdaExpr(LambdaExpr *E) { // Do not rebuild lambdas to avoid creating a new type. - // Lambdas have already been processed inside their eval context. + // Lambdas have already been processed inside their eval contexts. return E; } bool AlwaysRebuild() { return false; } diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index e5ea02a919f4eb..b052afede2cd67 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -5508,50 +5508,31 @@ bool Sema::CheckTemplateArgumentList( } // Check whether we have a default argument. -TemplateArgumentLoc Arg; +bool HasDefaultArg; // Retrieve the default template argument from the template // parameter. For each kind of template parameter, we substitute the // template arguments provided thus far and any "outer" template arguments // (when the template parameter was part of a nested template) into // the default argument. -if (TemplateTypeParmDecl *TTP = dyn_cast(*Param)) { - if (!hasReachableDefaultArgument(TTP)) -return diagnoseMissingArgument(*this, TemplateLoc, Template, TTP, +TemplateArgumentLoc Arg = SubstDefaultTemplateArgumentIfAvailable( +Template, TemplateLoc, RAngleLoc, *Param, SugaredConverted, +CanonicalConverted, HasDefaultArg); + +if (Arg.getArgument().isNull()) { + if (!HasDefaultArg) { +if (TemplateTypeParmDecl *TTP = dyn_cast(*Param)) + return diagnoseMissingArgument(*this, TemplateLoc, Template, TTP, + NewArgs); +if (NonTypeTemplateParmDecl *NTTP = +dyn_cast(*Param)) + return diagnoseMissingArgument(*this, TemplateLoc, Template, NTTP, + NewArgs);
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -249,7 +249,10 @@ Attribute Changes in Clang (#GH106864) - Introduced a new attribute ``[[clang::coro_await_elidable]]`` on coroutine return types - to express elideability at call sites where the coroutine is co_awaited as a prvalue. + to express elideability at call sites where the coroutine is invoked under a safe elide context. + +- Introduced a new attribute ``[[clang::coro_must_await]]`` on function parameters to vogelsgesang wrote: What do you think about `[[clang::coro_await_elideable_argument]]`? I would slightly prefer this over `[[clang::coro_elideable_await_argument]]` because it is more similar to its sibling argument `[[clang::coro_await_elideable]]`, and the duality between the two arguments becomes more obvious already from their names. I think we should particularly try to avoid any name with `must` in its name. To me, `must` expresses a guarantee given by the compiler to the programmer (such as for the `[[musttail]]` attribute which will fail compilation in case the call cannot be tail-called), enforced by the compiler. However, this attribute behaves the other way around: It expresses a guarantee given by the programmer to the compiler, the compiler does nothing to check or enforce the correctness of it. https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -8281,8 +8288,62 @@ Example: co_await t; } -The behavior is undefined if the caller coroutine is destroyed earlier than the -callee coroutine. +Such elision replaces the heap allocated activation frame of the callee coroutine +with a local variable within the enclosing braces in the caller's stack frame. +The local variable, like other variables in coroutines, may be collected into the +coroutine frame, which may be allocated on the heap. The behavior is undefined +if the caller coroutine is destroyed earlier than the callee coroutine. + +}]; +} + +def CoroMustAwaitDoc : Documentation { + let Category = DocCatDecl; + let Content = [{ + +The ``[[clang::coro_must_await]]`` is a function parameter attribute. It works +in conjunction with ``[[clang::coro_await_elidable]]`` to propagate a safe elide +context to a parameter or parameter pack if the function is called under a safe +elide context. + +This is sometimes necessary on utility functions whose role is to compose or +modify the behavior of a callee coroutine. vogelsgesang wrote: ```suggestion This is sometimes necessary on utility functions used to compose or modify the behavior of a callee coroutine. ``` https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -8261,12 +8261,19 @@ def CoroAwaitElidableDoc : Documentation { The ``[[clang::coro_await_elidable]]`` is a class attribute which can be applied to a coroutine return type. -When a coroutine function that returns such a type calls another coroutine function, -the compiler performs heap allocation elision when the call to the coroutine function -is immediately co_awaited as a prvalue. In this case, the coroutine frame for the -callee will be a local variable within the enclosing braces in the caller's stack -frame. And the local variable, like other variables in coroutines, may be collected -into the coroutine frame, which may be allocated on the heap. +When a coroutine function returns such a type, a direct call expression therein +that returns a prvalue of a type attributed ``[[clang::coro_await_elidable]]`` +is said to be under a safe elide context if one of the following is true: vogelsgesang wrote: > When a coroutine function returns such a type, a direct call expression > therein that returns a prvalue of a type attributed > ``[[clang::coro_await_elidable]]`` [...] If I am reading the code correctly, it is not actually a requirement that the containing / callee coroutine itself is also annotated as `coro_await_elidable`. Only the called coroutine needs to be annotated as `coro_await_elidable`. I.e., we would also apply HALO for something like ``` class [[clang::coro_await_elidable]] ElidableTask { ... }; class NonElidableTask { ... }; ElidableTask foo(); NonElidableTask bar() { co_await foo(); // foo()'s coroutine frame on this line is elidable } ``` Or did I misread the code? Assuming I understood code correctly, I would propose something like ``` A call to a coroutine function returning such a type is said to be safe-to-elide if one of the following is true: - it is the immediate right-hand side operand to a co_await expression. - it is an argument to a [[clang::coro_must_await]] parameter or parameter pack of another safe-to-elide function call. Do note that the safe-to-elide context applies only to the call expression itself, and the context does not transitively include any of its subexpressions unless ``[[clang::coro_must_await]]`` is used to opt-in to transivitely applying safe-to-elide. The compiler performs heap allocation elision on call expressions on safe-to-elide calls, if the callee is a coroutine. ``` https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -8281,8 +8288,62 @@ Example: co_await t; } -The behavior is undefined if the caller coroutine is destroyed earlier than the -callee coroutine. +Such elision replaces the heap allocated activation frame of the callee coroutine +with a local variable within the enclosing braces in the caller's stack frame. +The local variable, like other variables in coroutines, may be collected into the +coroutine frame, which may be allocated on the heap. The behavior is undefined +if the caller coroutine is destroyed earlier than the callee coroutine. + +}]; +} + +def CoroMustAwaitDoc : Documentation { + let Category = DocCatDecl; + let Content = [{ + +The ``[[clang::coro_must_await]]`` is a function parameter attribute. It works +in conjunction with ``[[clang::coro_await_elidable]]`` to propagate a safe elide +context to a parameter or parameter pack if the function is called under a safe +elide context. + +This is sometimes necessary on utility functions whose role is to compose or +modify the behavior of a callee coroutine. + +Example: + +.. code-block:: c++ + + template + class [[clang::coro_await_elidable]] Task { ... }; + + template + class [[clang::coro_await_elidable]] WhenAll { ... }; + + // `when_all` is a utility function that compose coroutines. It does not need vogelsgesang wrote: ```suggestion // `when_all` is a utility function that composes coroutines. It does not need ``` https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -8281,8 +8288,62 @@ Example: co_await t; } -The behavior is undefined if the caller coroutine is destroyed earlier than the -callee coroutine. +Such elision replaces the heap allocated activation frame of the callee coroutine +with a local variable within the enclosing braces in the caller's stack frame. +The local variable, like other variables in coroutines, may be collected into the +coroutine frame, which may be allocated on the heap. The behavior is undefined +if the caller coroutine is destroyed earlier than the callee coroutine. + +}]; +} + +def CoroMustAwaitDoc : Documentation { + let Category = DocCatDecl; + let Content = [{ + +The ``[[clang::coro_must_await]]`` is a function parameter attribute. It works +in conjunction with ``[[clang::coro_await_elidable]]`` to propagate a safe elide +context to a parameter or parameter pack if the function is called under a safe +elide context. + +This is sometimes necessary on utility functions whose role is to compose or +modify the behavior of a callee coroutine. + +Example: + +.. code-block:: c++ + + template + class [[clang::coro_await_elidable]] Task { ... }; + + template + class [[clang::coro_await_elidable]] WhenAll { ... }; + + // `when_all` is a utility function that compose coroutines. It does not need + // to be a coroutine to propagate. + template + WhenAll when_all([[clang::coro_must_await]] Task tasks...); + + Task foo(); + Task bar(); + Task example1() { +// `when_all``, `foo``, and `bar` are all elide safe because `when_all` is +// under a safe elide context and propagated such contexts to foo and bar. vogelsgesang wrote: ```suggestion // under a safe elide context and, thanks to the [[coro_must_await]] attribute, // this context is propagated to foo and bar. ``` https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -8261,12 +8261,19 @@ def CoroAwaitElidableDoc : Documentation { The ``[[clang::coro_await_elidable]]`` is a class attribute which can be applied to a coroutine return type. vogelsgesang wrote: ```suggestion to a coroutine return type. It provides a hint to the compiler to apply Heap Allocation Elision more aggressively. ``` I like it if the first paragraph of any documentation immediately gives me a high-level picture, so I know if I should keep reading the rest of its documentation or skip it. By immediately mentioning Heap Allocation Elision in the first paragraph, the reader also gets more context when read the rest of the paragraph, so he will hopefully be able to understand the following paragraphs more easily https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -84,4 +84,35 @@ Task nonelidable() { co_return 1; } +// CHECK-LABEL: define{{.*}} @_Z8addTasksO4TaskIiES1_{{.*}} { +Task addTasks([[clang::coro_must_await]] Task &&t1, Task &&t2) { + int i1 = co_await t1; + int i2 = co_await t2; + co_return i1 + i2; +} + +// CHECK-LABEL: define{{.*}} @_Z10returnSamei{{.*}} { +Task returnSame(int i) { + co_return i; +} + +// CHECK-LABEL: define{{.*}} @_Z21elidableWithMustAwaitv{{.*}} { +Task elidableWithMustAwait() { + // CHECK: call void @_Z10returnSamei(ptr {{.*}}, i32 noundef 2) #[[ELIDE_SAFE]] + // CHECK-NOT: call void @_Z10returnSamei(ptr {{.*}}, i32 noundef 3) #[[ELIDE_SAFE]] + co_return co_await addTasks(returnSame(2), returnSame(3)); +} + +template +Task sumAll([[clang::coro_must_await]] Args && ... tasks); + +// CHECK-LABEL: define{{.*}} @_Z16elidableWithPackv{{.*}} { +Task elidableWithPack() { vogelsgesang wrote: maybe also add a test case for recursive elision? I.e. for something like ``` co_await sumAll(addTasks(returnSame(1), returnSame(2)), returnSame(3)); ``` https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -84,4 +84,35 @@ Task nonelidable() { co_return 1; } +// CHECK-LABEL: define{{.*}} @_Z8addTasksO4TaskIiES1_{{.*}} { +Task addTasks([[clang::coro_must_await]] Task &&t1, Task &&t2) { + int i1 = co_await t1; + int i2 = co_await t2; + co_return i1 + i2; +} + +// CHECK-LABEL: define{{.*}} @_Z10returnSamei{{.*}} { +Task returnSame(int i) { + co_return i; +} + +// CHECK-LABEL: define{{.*}} @_Z21elidableWithMustAwaitv{{.*}} { +Task elidableWithMustAwait() { + // CHECK: call void @_Z10returnSamei(ptr {{.*}}, i32 noundef 2) #[[ELIDE_SAFE]] + // CHECK-NOT: call void @_Z10returnSamei(ptr {{.*}}, i32 noundef 3) #[[ELIDE_SAFE]] vogelsgesang wrote: Can we somehow turn this into a positive test? Negative tests can easily regress, e.g. if something else in the line would change. Can we use a `$` (or some other mechanism) to check for the end of the line, and there by explicitly check that a call to `_Z10returnSamei(ptr {{.*}}, i32 noundef 3)` does exist, but does not have the ELIDE_SAFE annotation? ```suggestion // CHECK: call void @_Z10returnSamei(ptr {{.*}}, i32 noundef 3)$ ``` https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -880,14 +898,12 @@ ExprResult Sema::BuildUnresolvedCoawaitExpr(SourceLocation Loc, Expr *Operand, } auto *RD = Promise->getType()->getAsCXXRecordDecl(); - bool AwaitElidable = - isCoroAwaitElidableCall(Operand) && - isAttributedCoroAwaitElidable( - getCurFunctionDecl(/*AllowLambda=*/true)->getReturnType()); - - if (AwaitElidable) -if (auto *Call = dyn_cast(Operand->IgnoreImplicit())) - Call->setCoroElideSafe(); + + bool CurFnAwaitElidable = isAttributedCoroAwaitElidable( + getCurFunctionDecl(/*AllowLambda=*/true)->getReturnType()); vogelsgesang wrote: shouldn't we actually check if the return type of àwait_transform` is marked as elideable, instead of the immediate argument to `co_await`? Or maybe we should check that both the immediate argument and the transformed awaitable are annotated? (Not really related to this commit. I just noticed this as I was reviewing this code change here. Fell free to ship this PR without addressing this comment. In case this is actually something we want to change, we should probably do so in a separate PR, anyway...) https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
@@ -8261,12 +8261,19 @@ def CoroAwaitElidableDoc : Documentation { The ``[[clang::coro_await_elidable]]`` is a class attribute which can be applied to a coroutine return type. -When a coroutine function that returns such a type calls another coroutine function, -the compiler performs heap allocation elision when the call to the coroutine function -is immediately co_awaited as a prvalue. In this case, the coroutine frame for the -callee will be a local variable within the enclosing braces in the caller's stack -frame. And the local variable, like other variables in coroutines, may be collected -into the coroutine frame, which may be allocated on the heap. +When a coroutine function returns such a type, a direct call expression therein +that returns a prvalue of a type attributed ``[[clang::coro_await_elidable]]`` +is said to be under a safe elide context if one of the following is true: +- it is the immediate right-hand side operand to a co_await expression. +- it is an argument to a [[clang::coro_must_await]] parameter or parameter pack vogelsgesang wrote: ```suggestion - it is an argument to a ``[[clang::coro_must_await]]`` parameter or parameter pack ``` https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
https://github.com/vogelsgesang commented: Thanks for implementing this! Looking forward to using it 🙂 https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Propagate elide safe context through [[clang::coro_must_await]] (PR #108474)
https://github.com/vogelsgesang edited https://github.com/llvm/llvm-project/pull/108474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] issue-63565: community requested small QoL fix for more configurabili… (PR #108005)
MK-Alias wrote: Because the `.clangd` config is made out of blocks, and no block was specified for our new `ArgumentLists` key, I've put it under `Completion`. So now it is like ```yaml Completion: AllScopes: None ArgumentLists: None ``` It seems like the most logical choice, because it's an auto-completion feature! Is this okay!? Or must it go in an other block!? https://github.com/llvm/llvm-project/pull/108005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 11f9008 - [ast matcher][NFC] fix typo in release note
Author: Congcong Cai Date: 2024-09-14T21:27:35+08:00 New Revision: 11f9008daec2b5593dfcc781ee879f93de121da2 URL: https://github.com/llvm/llvm-project/commit/11f9008daec2b5593dfcc781ee879f93de121da2 DIFF: https://github.com/llvm/llvm-project/commit/11f9008daec2b5593dfcc781ee879f93de121da2.diff LOG: [ast matcher][NFC] fix typo in release note Added: Modified: clang/docs/ReleaseNotes.rst Removed: diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 79b154ef1aef5e..485b75049927fe 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -501,7 +501,7 @@ AST Matchers - Fixed an issue with the `hasName` and `hasAnyName` matcher when matching inline namespaces with an enclosing namespace of the same name. -- Fixed an ordering issue with the `hasOperands` matcher occuring when setting a +- Fixed an ordering issue with the `hasOperands` matcher occurring when setting a binding in the first matcher and using it in the second matcher. clang-format ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
https://github.com/a-tarasyuk created https://github.com/llvm/llvm-project/pull/108701 Fixes #108687 >From 3e7e69e36e93f12c33ccdda9fea78e22cc9bdd84 Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Sat, 14 Sep 2024 17:02:01 +0300 Subject: [PATCH] [Clang] handle invalid close location in static assert declaration --- clang/docs/ReleaseNotes.rst| 2 +- clang/lib/Parse/ParseDeclCXX.cpp | 2 ++ clang/test/SemaCXX/static-assert-cxx26.cpp | 3 +++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 79b154ef1aef5e..27dde935ad19b9 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -388,7 +388,7 @@ Bug Fixes to C++ Support - Fixed a crash in the typo correction of an invalid CTAD guide. (#GH107887) - Fixed a crash when clang tries to subtitute parameter pack while retaining the parameter pack. #GH63819, #GH107560 - +- Fix a crash when a static assert declaration has an invalid close location. (#GH108687) Bug Fixes to AST Handling ^ diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index 6370da1fab0042..d02601172fe239 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -1110,6 +1110,8 @@ Decl *Parser::ParseStaticAssertDeclaration(SourceLocation &DeclEnd) { } T.consumeClose(); + if (T.getCloseLocation().isInvalid()) +return nullptr; DeclEnd = Tok.getLocation(); ExpectAndConsumeSemi(diag::err_expected_semi_after_static_assert, TokName); diff --git a/clang/test/SemaCXX/static-assert-cxx26.cpp b/clang/test/SemaCXX/static-assert-cxx26.cpp index 7d896d8b365b74..95ffa8cd68cf7c 100644 --- a/clang/test/SemaCXX/static-assert-cxx26.cpp +++ b/clang/test/SemaCXX/static-assert-cxx26.cpp @@ -415,3 +415,6 @@ static_assert( // expected-note@-1 {{read of dereferenced one-past-the-end pointer is not allowed in a constant expression}} ); } + +static_assert(true, "" // expected-note {{to match this '('}} + // expected-error {{expected ')'}} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Oleksandr T. (a-tarasyuk) Changes Fixes #108687 --- Full diff: https://github.com/llvm/llvm-project/pull/108701.diff 3 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+1-1) - (modified) clang/lib/Parse/ParseDeclCXX.cpp (+2) - (modified) clang/test/SemaCXX/static-assert-cxx26.cpp (+3) ``diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 79b154ef1aef5e..27dde935ad19b9 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -388,7 +388,7 @@ Bug Fixes to C++ Support - Fixed a crash in the typo correction of an invalid CTAD guide. (#GH107887) - Fixed a crash when clang tries to subtitute parameter pack while retaining the parameter pack. #GH63819, #GH107560 - +- Fix a crash when a static assert declaration has an invalid close location. (#GH108687) Bug Fixes to AST Handling ^ diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index 6370da1fab0042..d02601172fe239 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -1110,6 +1110,8 @@ Decl *Parser::ParseStaticAssertDeclaration(SourceLocation &DeclEnd) { } T.consumeClose(); + if (T.getCloseLocation().isInvalid()) +return nullptr; DeclEnd = Tok.getLocation(); ExpectAndConsumeSemi(diag::err_expected_semi_after_static_assert, TokName); diff --git a/clang/test/SemaCXX/static-assert-cxx26.cpp b/clang/test/SemaCXX/static-assert-cxx26.cpp index 7d896d8b365b74..95ffa8cd68cf7c 100644 --- a/clang/test/SemaCXX/static-assert-cxx26.cpp +++ b/clang/test/SemaCXX/static-assert-cxx26.cpp @@ -415,3 +415,6 @@ static_assert( // expected-note@-1 {{read of dereferenced one-past-the-end pointer is not allowed in a constant expression}} ); } + +static_assert(true, "" // expected-note {{to match this '('}} + // expected-error {{expected ')'}} `` https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] bae275f - [TableGen] Avoid repeated map lookups (NFC) (#108675)
Author: Kazu Hirata Date: 2024-09-14T07:39:00-07:00 New Revision: bae275f65ecd27a16743f7b4695940dcfa70b218 URL: https://github.com/llvm/llvm-project/commit/bae275f65ecd27a16743f7b4695940dcfa70b218 DIFF: https://github.com/llvm/llvm-project/commit/bae275f65ecd27a16743f7b4695940dcfa70b218.diff LOG: [TableGen] Avoid repeated map lookups (NFC) (#108675) Added: Modified: clang/utils/TableGen/NeonEmitter.cpp Removed: diff --git a/clang/utils/TableGen/NeonEmitter.cpp b/clang/utils/TableGen/NeonEmitter.cpp index 9e5480be20adac..82148f98d04fcc 100644 --- a/clang/utils/TableGen/NeonEmitter.cpp +++ b/clang/utils/TableGen/NeonEmitter.cpp @@ -2155,9 +2155,7 @@ void NeonEmitter::genOverloadTypeCheckCode(raw_ostream &OS, } if (Mask) { - std::string Name = Def->getMangledName(); - OverloadMap.insert(std::make_pair(Name, OverloadInfo())); - OverloadInfo &OI = OverloadMap[Name]; + OverloadInfo &OI = OverloadMap[Def->getMangledName()]; OI.Mask |= Mask; OI.PtrArgNum |= PtrArgNum; OI.HasConstPtr = HasConstPtr; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [TableGen] Avoid repeated map lookups (NFC) (PR #108675)
https://github.com/kazutakahirata closed https://github.com/llvm/llvm-project/pull/108675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [FMV][AArch64] Unify ls64, ls64_v and ls64_accdata. (PR #108024)
@@ -73,8 +73,6 @@ enum CPUFeatures { FEAT_SSBS, FEAT_SSBS2, FEAT_BTI, - FEAT_LS64, - FEAT_LS64_V, jroelofs wrote: We could do that automatically every release by putting the version number in its name. https://github.com/llvm/llvm-project/pull/108024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
@@ -1110,6 +1110,8 @@ Decl *Parser::ParseStaticAssertDeclaration(SourceLocation &DeclEnd) { } T.consumeClose(); + if (T.getCloseLocation().isInvalid()) +return nullptr; cor3ntin wrote: `consumeClose` returms true on failure so this can just be `if(T.consumeClose()) return nullptr` https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
https://github.com/cor3ntin edited https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] fix false positive that floating point variable only used in increment expr in cert-flp30-c (PR #108706)
https://github.com/HerrCai0907 created https://github.com/llvm/llvm-project/pull/108706 Fixes: #108049 cert-flp30-c only provides non-compliant example with normal loop. Essentially it wants to avoid that floating point variables are used as loop counters which are checked in condition expr and modified in increment expr. This patch wants to give more precise matcheres to identify this cases. >From 9fbbc1821c41a75a3a606f4a8bb0a7c43fe9dee3 Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Sat, 14 Sep 2024 23:25:36 +0800 Subject: [PATCH] [clang-tidy] fix false positive that floating point variable only used in increment expr in cert-flp30-c Fixes: #108049 cert-flp30-c only provides non-compliant example with normal loop. Essentially it wants to avoid that floating point variables are used as loop counters which are checked in condition expr and modified in increment expr. This patch wants to give more precise matcheres to identify this cases. --- .../clang-tidy/cert/FloatLoopCounter.cpp | 21 + clang-tools-extra/docs/ReleaseNotes.rst | 3 +++ .../test/clang-tidy/checkers/cert/flp30-c.c | 23 +-- 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp b/clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp index a9363837597867..ecaa078cc44fa5 100644 --- a/clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp +++ b/clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp @@ -9,22 +9,33 @@ #include "FloatLoopCounter.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" using namespace clang::ast_matchers; namespace clang::tidy::cert { void FloatLoopCounter::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher( - forStmt(hasIncrement(expr(hasType(realFloatingPointType().bind("for"), - this); + Finder->addMatcher(forStmt(hasIncrement(forEachDescendant( + declRefExpr(hasType(realFloatingPointType()), + to(varDecl().bind("var"), + hasCondition(forEachDescendant(declRefExpr( + hasType(realFloatingPointType()), + to(varDecl(equalsBoundNode("var"))) + .bind("for"), + this); } void FloatLoopCounter::check(const MatchFinder::MatchResult &Result) { const auto *FS = Result.Nodes.getNodeAs("for"); - diag(FS->getInc()->getExprLoc(), "loop induction expression should not have " - "floating-point type"); + diag(FS->getInc()->getBeginLoc(), "loop induction expression should not have " +"floating-point type"); + + if (!FS->getInc()->getType()->isRealFloatingType()) +if (const auto *V = Result.Nodes.getNodeAs("var")) + diag(V->getBeginLoc(), "floating-point type loop induction variable", + DiagnosticIDs::Note); } } // namespace clang::tidy::cert diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 88b92830fda6b4..12638ec66c913a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -111,6 +111,9 @@ Changes in existing checks ` check to suggest replacing the offending code with ``reinterpret_cast``, to more clearly express intent. +- Improved :doc:`cert-flp30-c` check to + fix false positive that floating point variable only used in increment expr. + - Improved :doc:`modernize-use-std-format ` check to support replacing member function calls too. diff --git a/clang-tools-extra/test/clang-tidy/checkers/cert/flp30-c.c b/clang-tools-extra/test/clang-tidy/checkers/cert/flp30-c.c index eee16bee3d82f4..b9985887a81c53 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cert/flp30-c.c +++ b/clang-tools-extra/test/clang-tidy/checkers/cert/flp30-c.c @@ -1,19 +1,28 @@ // RUN: %check_clang_tidy %s cert-flp30-c %t float g(void); +int c(float); +float f = 1.0f; + +void match(void) { -void func(void) { for (float x = 0.1f; x <= 1.0f; x += 0.1f) {} - // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: loop induction expression should not have floating-point type [cert-flp30-c] + // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: loop induction expression should not have floating-point type [cert-flp30-c] - float f = 1.0f; for (; f > 0; --f) {} - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: loop induction expression + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: loop induction expression should not have floating-point type [cert-flp30-c] - for (;;g()) {} - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: loop induction expression + for (float x = 0.0f; c(x); x = g()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: loop induction expression should not have floating-point
[clang-tools-extra] [clang-tidy] fix false positive that floating point variable only used in increment expr in cert-flp30-c (PR #108706)
llvmbot wrote: @llvm/pr-subscribers-clang-tools-extra Author: Congcong Cai (HerrCai0907) Changes Fixes: #108049 cert-flp30-c only provides non-compliant example with normal loop. Essentially it wants to avoid that floating point variables are used as loop counters which are checked in condition expr and modified in increment expr. This patch wants to give more precise matcheres to identify this cases. --- Full diff: https://github.com/llvm/llvm-project/pull/108706.diff 3 Files Affected: - (modified) clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp (+16-5) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+3) - (modified) clang-tools-extra/test/clang-tidy/checkers/cert/flp30-c.c (+16-7) ``diff diff --git a/clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp b/clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp index a9363837597867..ecaa078cc44fa5 100644 --- a/clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp +++ b/clang-tools-extra/clang-tidy/cert/FloatLoopCounter.cpp @@ -9,22 +9,33 @@ #include "FloatLoopCounter.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" using namespace clang::ast_matchers; namespace clang::tidy::cert { void FloatLoopCounter::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher( - forStmt(hasIncrement(expr(hasType(realFloatingPointType().bind("for"), - this); + Finder->addMatcher(forStmt(hasIncrement(forEachDescendant( + declRefExpr(hasType(realFloatingPointType()), + to(varDecl().bind("var"), + hasCondition(forEachDescendant(declRefExpr( + hasType(realFloatingPointType()), + to(varDecl(equalsBoundNode("var"))) + .bind("for"), + this); } void FloatLoopCounter::check(const MatchFinder::MatchResult &Result) { const auto *FS = Result.Nodes.getNodeAs("for"); - diag(FS->getInc()->getExprLoc(), "loop induction expression should not have " - "floating-point type"); + diag(FS->getInc()->getBeginLoc(), "loop induction expression should not have " +"floating-point type"); + + if (!FS->getInc()->getType()->isRealFloatingType()) +if (const auto *V = Result.Nodes.getNodeAs("var")) + diag(V->getBeginLoc(), "floating-point type loop induction variable", + DiagnosticIDs::Note); } } // namespace clang::tidy::cert diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 88b92830fda6b4..12638ec66c913a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -111,6 +111,9 @@ Changes in existing checks ` check to suggest replacing the offending code with ``reinterpret_cast``, to more clearly express intent. +- Improved :doc:`cert-flp30-c` check to + fix false positive that floating point variable only used in increment expr. + - Improved :doc:`modernize-use-std-format ` check to support replacing member function calls too. diff --git a/clang-tools-extra/test/clang-tidy/checkers/cert/flp30-c.c b/clang-tools-extra/test/clang-tidy/checkers/cert/flp30-c.c index eee16bee3d82f4..b9985887a81c53 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cert/flp30-c.c +++ b/clang-tools-extra/test/clang-tidy/checkers/cert/flp30-c.c @@ -1,19 +1,28 @@ // RUN: %check_clang_tidy %s cert-flp30-c %t float g(void); +int c(float); +float f = 1.0f; + +void match(void) { -void func(void) { for (float x = 0.1f; x <= 1.0f; x += 0.1f) {} - // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: loop induction expression should not have floating-point type [cert-flp30-c] + // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: loop induction expression should not have floating-point type [cert-flp30-c] - float f = 1.0f; for (; f > 0; --f) {} - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: loop induction expression + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: loop induction expression should not have floating-point type [cert-flp30-c] - for (;;g()) {} - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: loop induction expression + for (float x = 0.0f; c(x); x = g()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: loop induction expression should not have floating-point type [cert-flp30-c] - for (int i = 0; i < 10; i += 1.0f) {} + for (int i=0; i < 10 && f < 2.0f; f++, i++) {} + // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: loop induction expression should not have floating-point type [cert-flp30-c] + // CHECK-MESSAGES: :5:1: note: floating-point type loop induction variable +} +void not_match(void) { + for (int i = 0; i < 10; i += 1.0f) {} for (int i = 0; i < 10; ++i) {} + for (int i = 0; i < 10; ++i, f++) {} + for (int i = 0
[clang] Implementing `asfloat` using `bit_cast` (PR #108686)
@@ -361,6 +361,23 @@ bool any(double3); _HLSL_BUILTIN_ALIAS(__builtin_hlsl_any) bool any(double4); +//===--===// +// asfloat builtins +//===--===// + +/// \fn uint asfloat(T Val) +/// \brief Interprets the bit pattern of x as an unsigned integer. llvm-beanz wrote: This looks copy-pasted and not updated. https://github.com/llvm/llvm-project/pull/108686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
@@ -945,50 +959,11 @@ class DiagnosticsEngine : public RefCountedBase { void Report(const StoredDiagnostic &storedDiag); - /// Determine whethere there is already a diagnostic in flight. - bool isDiagnosticInFlight() const { -return CurDiagID != std::numeric_limits::max(); - } - - /// Set the "delayed" diagnostic that will be emitted once - /// the current diagnostic completes. - /// - /// If a diagnostic is already in-flight but the front end must - /// report a problem (e.g., with an inconsistent file system - /// state), this routine sets a "delayed" diagnostic that will be - /// emitted after the current diagnostic completes. This should - /// only be used for fatal errors detected at inconvenient - /// times. If emitting a delayed diagnostic causes a second delayed - /// diagnostic to be introduced, that second delayed diagnostic - /// will be ignored. - /// - /// \param DiagID The ID of the diagnostic being delayed. - /// - /// \param Arg1 A string argument that will be provided to the - /// diagnostic. A copy of this string will be stored in the - /// DiagnosticsEngine object itself. - /// - /// \param Arg2 A string argument that will be provided to the - /// diagnostic. A copy of this string will be stored in the - /// DiagnosticsEngine object itself. - /// - /// \param Arg3 A string argument that will be provided to the - /// diagnostic. A copy of this string will be stored in the - /// DiagnosticsEngine object itself. - void SetDelayedDiagnostic(unsigned DiagID, StringRef Arg1 = "", -StringRef Arg2 = "", StringRef Arg3 = ""); - - /// Clear out the current diagnostic. - void Clear() { CurDiagID = std::numeric_limits::max(); } - - /// Return the value associated with this diagnostic flag. - StringRef getFlagValue() const { return FlagValue; } - private: // This is private state used by DiagnosticBuilder. We put it here instead of // in DiagnosticBuilder in order to keep DiagnosticBuilder a small lightweight - // object. This implementation choice means that we can only have one - // diagnostic "in flight" at a time, but this seems to be a reasonable + // object. This implementation choice means that we can only have a few + // diagnostics "in flight" at a time, but this seems to be a reasonable // tradeoff to keep these objects small. Assertions verify that only one // diagnostic is in flight at a time. vvd170501 wrote: "Assertions verify that only one diagnostic is in flight at a time" is now outdated and can be removed. https://github.com/llvm/llvm-project/pull/108187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implementing `asfloat` using `bit_cast` (PR #108686)
https://github.com/joaosaffran updated https://github.com/llvm/llvm-project/pull/108686 >From 32e71ea8fe790ea81163fac76bd2029991d780bd Mon Sep 17 00:00:00 2001 From: Joao Saffran Date: Sat, 14 Sep 2024 06:23:08 + Subject: [PATCH] implementing asfloat using bit_cast --- clang/lib/Headers/hlsl/hlsl_intrinsics.h | 17 clang/test/CodeGenHLSL/builtins/asfloat.hlsl | 40 +++ .../SemaHLSL/BuiltIns/asfloat-errors.hlsl | 35 3 files changed, 92 insertions(+) create mode 100644 clang/test/CodeGenHLSL/builtins/asfloat.hlsl create mode 100644 clang/test/SemaHLSL/BuiltIns/asfloat-errors.hlsl diff --git a/clang/lib/Headers/hlsl/hlsl_intrinsics.h b/clang/lib/Headers/hlsl/hlsl_intrinsics.h index b5c22f7c91b2d6..6a50d50ebd3479 100644 --- a/clang/lib/Headers/hlsl/hlsl_intrinsics.h +++ b/clang/lib/Headers/hlsl/hlsl_intrinsics.h @@ -361,6 +361,23 @@ bool any(double3); _HLSL_BUILTIN_ALIAS(__builtin_hlsl_any) bool any(double4); +//===--===// +// asfloat builtins +//===--===// + +/// \fn float asfloat(T Val) +/// \brief Interprets the bit pattern of x as float point number. +/// \param Val The input value. + +template +_HLSL_INLINE vector asfloat(vector V) { + return __detail::bit_cast(V); +} + +template _HLSL_INLINE float asfloat(T F) { + return __detail::bit_cast(F); +} + //===--===// // asin builtins //===--===// diff --git a/clang/test/CodeGenHLSL/builtins/asfloat.hlsl b/clang/test/CodeGenHLSL/builtins/asfloat.hlsl new file mode 100644 index 00..59fc15fa60b1e5 --- /dev/null +++ b/clang/test/CodeGenHLSL/builtins/asfloat.hlsl @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple dxil-pc-shadermodel6.3-library %s -fnative-half-type -emit-llvm -O1 -o - | FileCheck %s + +// CHECK: define {{.*}}test_uint{{.*}}(i32 {{.*}} [[VAL:%.*]]){{.*}} +// CHECK: bitcast i32 [[VAL]] to float +float test_uint(uint p0) { + return asfloat(p0); +} + +// CHECK: define {{.*}}test_int{{.*}}(i32 {{.*}} [[VAL:%.*]]){{.*}} +// CHECK: bitcast i32 [[VAL]] to float +float test_int(int p0) { + return asfloat(p0); +} + +// CHECK: define {{.*}}test_float{{.*}}(float {{.*}} [[VAL:%.*]]){{.*}} +// CHECK-NOT: bitcast +// CHECK: ret float [[VAL]] +float test_float(float p0) { + return asfloat(p0); +} + +// CHECK: define {{.*}}test_vector_uint{{.*}}(<4 x i32> {{.*}} [[VAL:%.*]]){{.*}} +// CHECK: bitcast <4 x i32> [[VAL]] to <4 x float> + +float4 test_vector_uint(uint4 p0) { + return asfloat(p0); +} + +// CHECK: define {{.*}}test_vector_int{{.*}}(<4 x i32> {{.*}} [[VAL:%.*]]){{.*}} +// CHECK: bitcast <4 x i32> [[VAL]] to <4 x float> +float4 test_vector_int(int4 p0) { + return asfloat(p0); +} + +// CHECK: define {{.*}}test_vector_float{{.*}}(<4 x float> {{.*}} [[VAL:%.*]]){{.*}} +// CHECK-NOT: bitcast +// CHECK: ret <4 x float> [[VAL]] +float4 test_vector_float(float4 p0) { + return asfloat(p0); +} diff --git a/clang/test/SemaHLSL/BuiltIns/asfloat-errors.hlsl b/clang/test/SemaHLSL/BuiltIns/asfloat-errors.hlsl new file mode 100644 index 00..16273a513c41e6 --- /dev/null +++ b/clang/test/SemaHLSL/BuiltIns/asfloat-errors.hlsl @@ -0,0 +1,35 @@ +// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -verify + + +float4 test_float_too_many_arg(float p0, float p1) { + return asfloat(p0, p1); + // expected-error@-1 {{no matching function for call to 'asfloat'}} + // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires single argument 'V', but 2 arguments were provided}} + // expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function template not viable: requires single argument 'F', but 2 arguments were provided}} +} + + +float test_float_double(double p1) { +return asfloat(p1); +// expected-error@hlsl/hlsl_intrinsics.h:* {{no matching function for call to 'bit_cast'}} +// expected-note@-2 {{in instantiation of function template specialization 'hlsl::asfloat'}} +// expected-note@hlsl/hlsl_detail.h:* {{candidate template ignored: could not match 'vector' against 'double'}} +// expected-note@hlsl/hlsl_detail.h:* {{candidate template ignored: substitution failure [with U = float, T = double]: no type named 'Type'}} +} + +float test_float_half(half p1) { +return asfloat(p1); +// expected-error@hlsl/hlsl_intrinsics.h:* {{no matching function for call to 'bit_cast'}} +// expected-note@-2 {{in instantiation of function template specialization 'hlsl::asfloat'}} +// expected-note@hlsl/hlsl_detail.h:* {{candidate template ignored: could not match 'vector' against 'half'}} +// expected-note@hlsl/hlsl_detail.h:* {{candidate template igno
[clang] Implementing `asfloat` using `bit_cast` (PR #108686)
@@ -361,6 +361,23 @@ bool any(double3); _HLSL_BUILTIN_ALIAS(__builtin_hlsl_any) bool any(double4); +//===--===// +// asfloat builtins +//===--===// + +/// \fn uint asfloat(T Val) +/// \brief Interprets the bit pattern of x as an unsigned integer. joaosaffran wrote: Missed that, thanks for pointing out https://github.com/llvm/llvm-project/pull/108686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
https://github.com/a-tarasyuk updated https://github.com/llvm/llvm-project/pull/108701 >From cbe06cf33277f904fe002db90d428a55b777ff5d Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Sat, 14 Sep 2024 17:02:01 +0300 Subject: [PATCH] [Clang] handle invalid close location in static assert declaration --- clang/docs/ReleaseNotes.rst| 2 +- clang/lib/Parse/ParseDeclCXX.cpp | 3 ++- clang/test/SemaCXX/static-assert-cxx26.cpp | 3 +++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 79b154ef1aef5e..27dde935ad19b9 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -388,7 +388,7 @@ Bug Fixes to C++ Support - Fixed a crash in the typo correction of an invalid CTAD guide. (#GH107887) - Fixed a crash when clang tries to subtitute parameter pack while retaining the parameter pack. #GH63819, #GH107560 - +- Fix a crash when a static assert declaration has an invalid close location. (#GH108687) Bug Fixes to AST Handling ^ diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index 6370da1fab0042..6f0f5a0311bc18 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -1109,7 +1109,8 @@ Decl *Parser::ParseStaticAssertDeclaration(SourceLocation &DeclEnd) { } } - T.consumeClose(); + if (T.consumeClose()) +return nullptr; DeclEnd = Tok.getLocation(); ExpectAndConsumeSemi(diag::err_expected_semi_after_static_assert, TokName); diff --git a/clang/test/SemaCXX/static-assert-cxx26.cpp b/clang/test/SemaCXX/static-assert-cxx26.cpp index 7d896d8b365b74..95ffa8cd68cf7c 100644 --- a/clang/test/SemaCXX/static-assert-cxx26.cpp +++ b/clang/test/SemaCXX/static-assert-cxx26.cpp @@ -415,3 +415,6 @@ static_assert( // expected-note@-1 {{read of dereferenced one-past-the-end pointer is not allowed in a constant expression}} ); } + +static_assert(true, "" // expected-note {{to match this '('}} + // expected-error {{expected ')'}} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
@@ -1110,6 +1110,8 @@ Decl *Parser::ParseStaticAssertDeclaration(SourceLocation &DeclEnd) { } T.consumeClose(); + if (T.getCloseLocation().isInvalid()) +return nullptr; a-tarasyuk wrote: @cor3ntin Thanks for the review. I've added changes to omit redundant checks. https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] fix(clang/**.py): fix comparison to None (PR #94014)
https://github.com/e-kwsm updated https://github.com/llvm/llvm-project/pull/94014 >From 3fcf81d949e461fb91b4e1713365da29ae1078cd Mon Sep 17 00:00:00 2001 From: Eisuke Kawashima Date: Sat, 11 May 2024 23:57:11 +0900 Subject: [PATCH] fix(clang/**.py): fix comparison to None from PEP8 (https://peps.python.org/pep-0008/#programming-recommendations): > Comparisons to singletons like None should always be done with is or > is not, never the equality operators. --- clang/docs/DebuggingCoroutines.rst | 8 clang/tools/include-mapping/gen_std.py | 2 +- clang/utils/check_cfc/obj_diff.py | 2 +- clang/utils/module-deps-to-rsp.py | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/clang/docs/DebuggingCoroutines.rst b/clang/docs/DebuggingCoroutines.rst index 53bdd08fdbc02f..7f464c1f4f28ca 100644 --- a/clang/docs/DebuggingCoroutines.rst +++ b/clang/docs/DebuggingCoroutines.rst @@ -513,7 +513,7 @@ So we can use the ``continuation`` field to construct the asynchronous stack: self.coro_frame = coro_frame self.resume_func = dereference(self.coro_frame.resume_addr) self.resume_func_block = gdb.block_for_pc(self.resume_func) - if self.resume_func_block == None: + if self.resume_func_block is None: raise Exception('Not stackless coroutine.') self.line_info = gdb.find_pc_line(self.resume_func) @@ -543,8 +543,8 @@ So we can use the ``continuation`` field to construct the asynchronous stack: self.function_name = f def __str__(self, shift = 2): - addr = "" if self.address() == None else '%#x' % self.address() + " in " - location = "" if self.filename() == None else " at " + self.filename() + ":" + str(self.line()) + addr = "" if self.address() is None else '%#x' % self.address() + " in " + location = "" if self.filename() is None else " at " + self.filename() + ":" + str(self.line()) return addr + self.function() + " " + str([str(args) for args in self.frame_args()]) + location class CoroutineFilter: @@ -598,7 +598,7 @@ So we can use the ``continuation`` field to construct the asynchronous stack: addr = int(argv[0], 16) block = gdb.block_for_pc(long(cast_addr2long_pointer(addr).dereference())) - if block == None: + if block is None: print "block " + str(addr) + " is none." return diff --git a/clang/tools/include-mapping/gen_std.py b/clang/tools/include-mapping/gen_std.py index fcd3bd0d843ea1..f362227bc6aab9 100755 --- a/clang/tools/include-mapping/gen_std.py +++ b/clang/tools/include-mapping/gen_std.py @@ -215,7 +215,7 @@ def GetCCompatibilitySymbols(symbol): # Introduce two more entries, both in the global namespace, one using the # C++-compat header and another using the C header. results = [] -if symbol.namespace != None: +if symbol.namespace is not None: # avoid printing duplicated entries, for C macros! results.append(cppreference_parser.Symbol(symbol.name, None, [header])) c_header = "<" + header[2:-1] + ".h>" # => diff --git a/clang/utils/check_cfc/obj_diff.py b/clang/utils/check_cfc/obj_diff.py index 99ed19e522be20..9d602593a4e1a9 100755 --- a/clang/utils/check_cfc/obj_diff.py +++ b/clang/utils/check_cfc/obj_diff.py @@ -57,7 +57,7 @@ def first_diff(a, b, fromfile, tofile): first_diff_idx = idx break -if first_diff_idx == None: +if first_diff_idx is None: # No difference return None diff --git a/clang/utils/module-deps-to-rsp.py b/clang/utils/module-deps-to-rsp.py index 6c9f263a786efc..b57a44e5c87809 100755 --- a/clang/utils/module-deps-to-rsp.py +++ b/clang/utils/module-deps-to-rsp.py @@ -74,7 +74,7 @@ def main(): if args.module_name: cmd = findModule(args.module_name, full_deps)["command-line"] -elif args.tu_index != None: +elif args.tu_index is not None: tu = full_deps.translation_units[args.tu_index] cmd = tu["commands"][args.tu_cmd_index]["command-line"] ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] issue-63565: community requested small QoL fix for more configurabili… (PR #108005)
HighCommander4 wrote: > Is this okay!? Yep. By the way, this was specified in the [original proposal](https://github.com/llvm/llvm-project/issues/63565#issuecomment-1975065771): > Here's a concrete proposal for a config file syntax that would address this > request, https://github.com/clangd/clangd/issues/1252, and supersede > `--function-arg-placeholders`: add a new key **under `Completion`** called > `ArgumentLists`, with the following supported values: https://github.com/llvm/llvm-project/pull/108005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] issue-63565: community requested small QoL fix for more configurabili… (PR #108005)
https://github.com/MK-Alias updated https://github.com/llvm/llvm-project/pull/108005 >From 4e9cbd615c56604fbc3c05f677b9da11f30977a2 Mon Sep 17 00:00:00 2001 From: MK-Alias Date: Tue, 10 Sep 2024 13:24:46 +0200 Subject: [PATCH 1/2] issue-63565: community requested small QoL fix for more configurability on --function-arg-placeholders --- clang-tools-extra/clangd/CodeComplete.cpp| 32 ++ clang-tools-extra/clangd/CodeComplete.h | 18 +-- clang-tools-extra/clangd/tool/ClangdMain.cpp | 34 +++- 3 files changed, 60 insertions(+), 24 deletions(-) diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 89eee392837af4..9fb264ef9160b3 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -350,8 +350,7 @@ struct CodeCompletionBuilder { CodeCompletionContext::Kind ContextKind, const CodeCompleteOptions &Opts, bool IsUsingDeclaration, tok::TokenKind NextTokenKind) - : ASTCtx(ASTCtx), -EnableFunctionArgSnippets(Opts.EnableFunctionArgSnippets), + : ASTCtx(ASTCtx), PlaceholderType(Opts.PlaceholderType), IsUsingDeclaration(IsUsingDeclaration), NextTokenKind(NextTokenKind) { Completion.Deprecated = true; // cleared by any non-deprecated overload. add(C, SemaCCS, ContextKind); @@ -561,6 +560,14 @@ struct CodeCompletionBuilder { } std::string summarizeSnippet() const { +/// localize PlaceholderType for better readability +const bool None = PlaceholderType == CodeCompleteOptions::None; +const bool Open = PlaceholderType == CodeCompleteOptions::OpenDelimiter; +const bool Delim = PlaceholderType == CodeCompleteOptions::Delimiters; +const bool Full = +PlaceholderType == CodeCompleteOptions::FullPlaceholders || +(!None && !Open && !Delim); // <-- failsafe + if (IsUsingDeclaration) return ""; auto *Snippet = onlyValue<&BundledEntry::SnippetSuffix>(); @@ -568,7 +575,7 @@ struct CodeCompletionBuilder { // All bundles are function calls. // FIXME(ibiryukov): sometimes add template arguments to a snippet, e.g. // we need to complete 'forward<$1>($0)'. - return "($0)"; + return None ? "" : (Open ? "(" : "($0)"); if (Snippet->empty()) return ""; @@ -607,7 +614,7 @@ struct CodeCompletionBuilder { return ""; } } -if (EnableFunctionArgSnippets) +if (Full) return *Snippet; // Replace argument snippets with a simplified pattern. @@ -622,9 +629,9 @@ struct CodeCompletionBuilder { bool EmptyArgs = llvm::StringRef(*Snippet).ends_with("()"); if (Snippet->front() == '<') -return EmptyArgs ? "<$1>()$0" : "<$1>($0)"; +return None ? "" : (Open ? "<" : (EmptyArgs ? "<$1>()$0" : "<$1>($0)")); if (Snippet->front() == '(') -return EmptyArgs ? "()" : "($0)"; +return None ? "" : (Open ? "(" : (EmptyArgs ? "()$0" : "($0)")); return *Snippet; // Not an arg snippet? } // 'CompletionItemKind::Interface' matches template type aliases. @@ -638,7 +645,7 @@ struct CodeCompletionBuilder { // e.g. Foo<${1:class}>. if (llvm::StringRef(*Snippet).ends_with("<>")) return "<>"; // can happen with defaulted template arguments. - return "<$0>"; + return None ? "" : (Open ? "<" : "<$0>"); } return *Snippet; } @@ -654,7 +661,7 @@ struct CodeCompletionBuilder { ASTContext *ASTCtx; CodeCompletion Completion; llvm::SmallVector Bundled; - bool EnableFunctionArgSnippets; + CodeCompleteOptions::PlaceholderOption PlaceholderType; // No snippets will be generated for using declarations and when the function // arguments are already present. bool IsUsingDeclaration; @@ -797,8 +804,8 @@ SpecifiedScope getQueryScopes(CodeCompletionContext &CCContext, llvm::StringRef SpelledSpecifier = Lexer::getSourceText( CharSourceRange::getCharRange(SemaSpecifier->getRange()), CCSema.SourceMgr, clang::LangOptions()); - if (SpelledSpecifier.consume_front("::")) - Scopes.QueryScopes = {""}; + if (SpelledSpecifier.consume_front("::")) +Scopes.QueryScopes = {""}; Scopes.UnresolvedQualifier = std::string(SpelledSpecifier); // Sema excludes the trailing "::". if (!Scopes.UnresolvedQualifier->empty()) @@ -1591,7 +1598,7 @@ class CodeCompleteFlow { CompletionPrefix HeuristicPrefix; std::optional Filter; // Initialized once Sema runs. Range ReplacedRange; - std::vector QueryScopes; // Initialized once Sema runs. + std::vector QueryScopes; // Initialized once Sema runs. std::vector AccessibleScopes; // Initialized once Sema runs. // Initialized once QueryScopes is initialized, if there are scopes. std::optional ScopeProximity; @@ -2387,8 +2394,7 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS
[clang-tools-extra] issue-63565: community requested small QoL fix for more configurabili… (PR #108005)
MK-Alias wrote: Updated the source. It should now confirm to everything. To recap: `.clangd` now has a added key: `ArgumentLists` in the `Completion` block. It supports these values: `None` | `OpenDelimiter` | `Delimiters` | `FullPlaceholders`. The `.clangd` config will overwite the prevously used: `--function-arg-placeholders`, which means that the only effect still done by `--function-arg-placeholders=0`, is when `.clangd` has no `ArgumentLists` value. In which case it will use the `Delimiters`. UnitTest have also been updated. Other then the tests already in place, it will also check the `None` option and the `OpenDelimiter` option. The `FullPlaceholders` option is not checked. It was also never checked prior to this patch. As mentions in [this post](https://github.com/llvm/llvm-project/issues/63565#issuecomment-2337396222): these to issues remain within vsocde/vscode-clangd: - None: after completion an error squiggly line will occur on the identifier. Would be slightly nicer if it wouldn't. - OpenDelimiter: the signature help hover doesn't spawn. It seems that [Sam's Patch from #398](https://github.com/clangd/vscode-clangd/pull/398/files) needs to be reconfigured for this. The OpenDelimiter issue seems like a small fix and would generally improve the UX on this. Ill be awaiting your response. https://github.com/llvm/llvm-project/pull/108005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] issue-63565: community requested small QoL fix for more configurabili… (PR #108005)
https://github.com/MK-Alias updated https://github.com/llvm/llvm-project/pull/108005 >From 4e9cbd615c56604fbc3c05f677b9da11f30977a2 Mon Sep 17 00:00:00 2001 From: MK-Alias Date: Tue, 10 Sep 2024 13:24:46 +0200 Subject: [PATCH 1/3] issue-63565: community requested small QoL fix for more configurability on --function-arg-placeholders --- clang-tools-extra/clangd/CodeComplete.cpp| 32 ++ clang-tools-extra/clangd/CodeComplete.h | 18 +-- clang-tools-extra/clangd/tool/ClangdMain.cpp | 34 +++- 3 files changed, 60 insertions(+), 24 deletions(-) diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 89eee392837af4..9fb264ef9160b3 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -350,8 +350,7 @@ struct CodeCompletionBuilder { CodeCompletionContext::Kind ContextKind, const CodeCompleteOptions &Opts, bool IsUsingDeclaration, tok::TokenKind NextTokenKind) - : ASTCtx(ASTCtx), -EnableFunctionArgSnippets(Opts.EnableFunctionArgSnippets), + : ASTCtx(ASTCtx), PlaceholderType(Opts.PlaceholderType), IsUsingDeclaration(IsUsingDeclaration), NextTokenKind(NextTokenKind) { Completion.Deprecated = true; // cleared by any non-deprecated overload. add(C, SemaCCS, ContextKind); @@ -561,6 +560,14 @@ struct CodeCompletionBuilder { } std::string summarizeSnippet() const { +/// localize PlaceholderType for better readability +const bool None = PlaceholderType == CodeCompleteOptions::None; +const bool Open = PlaceholderType == CodeCompleteOptions::OpenDelimiter; +const bool Delim = PlaceholderType == CodeCompleteOptions::Delimiters; +const bool Full = +PlaceholderType == CodeCompleteOptions::FullPlaceholders || +(!None && !Open && !Delim); // <-- failsafe + if (IsUsingDeclaration) return ""; auto *Snippet = onlyValue<&BundledEntry::SnippetSuffix>(); @@ -568,7 +575,7 @@ struct CodeCompletionBuilder { // All bundles are function calls. // FIXME(ibiryukov): sometimes add template arguments to a snippet, e.g. // we need to complete 'forward<$1>($0)'. - return "($0)"; + return None ? "" : (Open ? "(" : "($0)"); if (Snippet->empty()) return ""; @@ -607,7 +614,7 @@ struct CodeCompletionBuilder { return ""; } } -if (EnableFunctionArgSnippets) +if (Full) return *Snippet; // Replace argument snippets with a simplified pattern. @@ -622,9 +629,9 @@ struct CodeCompletionBuilder { bool EmptyArgs = llvm::StringRef(*Snippet).ends_with("()"); if (Snippet->front() == '<') -return EmptyArgs ? "<$1>()$0" : "<$1>($0)"; +return None ? "" : (Open ? "<" : (EmptyArgs ? "<$1>()$0" : "<$1>($0)")); if (Snippet->front() == '(') -return EmptyArgs ? "()" : "($0)"; +return None ? "" : (Open ? "(" : (EmptyArgs ? "()$0" : "($0)")); return *Snippet; // Not an arg snippet? } // 'CompletionItemKind::Interface' matches template type aliases. @@ -638,7 +645,7 @@ struct CodeCompletionBuilder { // e.g. Foo<${1:class}>. if (llvm::StringRef(*Snippet).ends_with("<>")) return "<>"; // can happen with defaulted template arguments. - return "<$0>"; + return None ? "" : (Open ? "<" : "<$0>"); } return *Snippet; } @@ -654,7 +661,7 @@ struct CodeCompletionBuilder { ASTContext *ASTCtx; CodeCompletion Completion; llvm::SmallVector Bundled; - bool EnableFunctionArgSnippets; + CodeCompleteOptions::PlaceholderOption PlaceholderType; // No snippets will be generated for using declarations and when the function // arguments are already present. bool IsUsingDeclaration; @@ -797,8 +804,8 @@ SpecifiedScope getQueryScopes(CodeCompletionContext &CCContext, llvm::StringRef SpelledSpecifier = Lexer::getSourceText( CharSourceRange::getCharRange(SemaSpecifier->getRange()), CCSema.SourceMgr, clang::LangOptions()); - if (SpelledSpecifier.consume_front("::")) - Scopes.QueryScopes = {""}; + if (SpelledSpecifier.consume_front("::")) +Scopes.QueryScopes = {""}; Scopes.UnresolvedQualifier = std::string(SpelledSpecifier); // Sema excludes the trailing "::". if (!Scopes.UnresolvedQualifier->empty()) @@ -1591,7 +1598,7 @@ class CodeCompleteFlow { CompletionPrefix HeuristicPrefix; std::optional Filter; // Initialized once Sema runs. Range ReplacedRange; - std::vector QueryScopes; // Initialized once Sema runs. + std::vector QueryScopes; // Initialized once Sema runs. std::vector AccessibleScopes; // Initialized once Sema runs. // Initialized once QueryScopes is initialized, if there are scopes. std::optional ScopeProximity; @@ -2387,8 +2394,7 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS
[clang-tools-extra] [clang-tidy] fix nullptr dereference in bugprone-forwarding-reference (PR #106856)
https://github.com/nicovank approved this pull request. LGTM. https://github.com/llvm/llvm-project/pull/106856 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] issue-63565: community requested small QoL fix for more configurabili… (PR #108005)
https://github.com/MK-Alias updated https://github.com/llvm/llvm-project/pull/108005 >From 4e9cbd615c56604fbc3c05f677b9da11f30977a2 Mon Sep 17 00:00:00 2001 From: MK-Alias Date: Tue, 10 Sep 2024 13:24:46 +0200 Subject: [PATCH 1/4] issue-63565: community requested small QoL fix for more configurability on --function-arg-placeholders --- clang-tools-extra/clangd/CodeComplete.cpp| 32 ++ clang-tools-extra/clangd/CodeComplete.h | 18 +-- clang-tools-extra/clangd/tool/ClangdMain.cpp | 34 +++- 3 files changed, 60 insertions(+), 24 deletions(-) diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 89eee392837af4..9fb264ef9160b3 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -350,8 +350,7 @@ struct CodeCompletionBuilder { CodeCompletionContext::Kind ContextKind, const CodeCompleteOptions &Opts, bool IsUsingDeclaration, tok::TokenKind NextTokenKind) - : ASTCtx(ASTCtx), -EnableFunctionArgSnippets(Opts.EnableFunctionArgSnippets), + : ASTCtx(ASTCtx), PlaceholderType(Opts.PlaceholderType), IsUsingDeclaration(IsUsingDeclaration), NextTokenKind(NextTokenKind) { Completion.Deprecated = true; // cleared by any non-deprecated overload. add(C, SemaCCS, ContextKind); @@ -561,6 +560,14 @@ struct CodeCompletionBuilder { } std::string summarizeSnippet() const { +/// localize PlaceholderType for better readability +const bool None = PlaceholderType == CodeCompleteOptions::None; +const bool Open = PlaceholderType == CodeCompleteOptions::OpenDelimiter; +const bool Delim = PlaceholderType == CodeCompleteOptions::Delimiters; +const bool Full = +PlaceholderType == CodeCompleteOptions::FullPlaceholders || +(!None && !Open && !Delim); // <-- failsafe + if (IsUsingDeclaration) return ""; auto *Snippet = onlyValue<&BundledEntry::SnippetSuffix>(); @@ -568,7 +575,7 @@ struct CodeCompletionBuilder { // All bundles are function calls. // FIXME(ibiryukov): sometimes add template arguments to a snippet, e.g. // we need to complete 'forward<$1>($0)'. - return "($0)"; + return None ? "" : (Open ? "(" : "($0)"); if (Snippet->empty()) return ""; @@ -607,7 +614,7 @@ struct CodeCompletionBuilder { return ""; } } -if (EnableFunctionArgSnippets) +if (Full) return *Snippet; // Replace argument snippets with a simplified pattern. @@ -622,9 +629,9 @@ struct CodeCompletionBuilder { bool EmptyArgs = llvm::StringRef(*Snippet).ends_with("()"); if (Snippet->front() == '<') -return EmptyArgs ? "<$1>()$0" : "<$1>($0)"; +return None ? "" : (Open ? "<" : (EmptyArgs ? "<$1>()$0" : "<$1>($0)")); if (Snippet->front() == '(') -return EmptyArgs ? "()" : "($0)"; +return None ? "" : (Open ? "(" : (EmptyArgs ? "()$0" : "($0)")); return *Snippet; // Not an arg snippet? } // 'CompletionItemKind::Interface' matches template type aliases. @@ -638,7 +645,7 @@ struct CodeCompletionBuilder { // e.g. Foo<${1:class}>. if (llvm::StringRef(*Snippet).ends_with("<>")) return "<>"; // can happen with defaulted template arguments. - return "<$0>"; + return None ? "" : (Open ? "<" : "<$0>"); } return *Snippet; } @@ -654,7 +661,7 @@ struct CodeCompletionBuilder { ASTContext *ASTCtx; CodeCompletion Completion; llvm::SmallVector Bundled; - bool EnableFunctionArgSnippets; + CodeCompleteOptions::PlaceholderOption PlaceholderType; // No snippets will be generated for using declarations and when the function // arguments are already present. bool IsUsingDeclaration; @@ -797,8 +804,8 @@ SpecifiedScope getQueryScopes(CodeCompletionContext &CCContext, llvm::StringRef SpelledSpecifier = Lexer::getSourceText( CharSourceRange::getCharRange(SemaSpecifier->getRange()), CCSema.SourceMgr, clang::LangOptions()); - if (SpelledSpecifier.consume_front("::")) - Scopes.QueryScopes = {""}; + if (SpelledSpecifier.consume_front("::")) +Scopes.QueryScopes = {""}; Scopes.UnresolvedQualifier = std::string(SpelledSpecifier); // Sema excludes the trailing "::". if (!Scopes.UnresolvedQualifier->empty()) @@ -1591,7 +1598,7 @@ class CodeCompleteFlow { CompletionPrefix HeuristicPrefix; std::optional Filter; // Initialized once Sema runs. Range ReplacedRange; - std::vector QueryScopes; // Initialized once Sema runs. + std::vector QueryScopes; // Initialized once Sema runs. std::vector AccessibleScopes; // Initialized once Sema runs. // Initialized once QueryScopes is initialized, if there are scopes. std::optional ScopeProximity; @@ -2387,8 +2394,7 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS
[clang] [clang-format] Fix regression with BlockIndent of Braced Initializers (PR #108717)
https://github.com/gedare created https://github.com/llvm/llvm-project/pull/108717 Fixes #73584. >From a95b990e48df19b8b674fe9df6bea803415129bf Mon Sep 17 00:00:00 2001 From: Gedare Bloom Date: Sat, 14 Sep 2024 13:13:26 -0600 Subject: [PATCH] [clang-format] Fix regression with BlockIndent of Braced Initializers Fixes #73584. --- clang/lib/Format/ContinuationIndenter.cpp | 7 +++ clang/unittests/Format/FormatTest.cpp | 11 +++ 2 files changed, 18 insertions(+) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index f29f8796ea9290..5c77af2da5add5 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -348,6 +348,13 @@ bool ContinuationIndenter::canBreak(const LineState &State) { } } + // Don't allow breaking before a closing right brace of a block-indented + // braced list initializer if there was not already a break. + if (Current.is(tok::r_brace) && Current.MatchingParen && + Current.isBlockIndentedInitRBrace(Style)) { +return CurrentState.BreakBeforeClosingBrace; + } + // If binary operators are moved to the next line (including commas for some // styles of constructor initializers), that's always ok. if (!Current.isOneOf(TT_BinaryOperator, tok::comma) && diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 5ebf0d7068dd6c..d1cb2b053e33d6 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -9336,6 +9336,9 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) { "ccc(a, //\n" "b));", Style); + verifyFormat("aaa const aa{\n" + "a(aaa, )};", + Style); Style.ColumnLimit = 30; verifyFormat("for (int foo = 0; foo < FOO;\n" @@ -9395,6 +9398,9 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) { "fooo(new FOO::BA(\n" "XXXZ()));", Style); + verifyFormat("aaa const aa{\n" + "a(aaa, )};", + Style); Style.AlignAfterOpenBracket = FormatStyle::BAS_BlockIndent; Style.BinPackArguments = false; @@ -9441,6 +9447,11 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) { "\n" ");", Style); + verifyFormat("aaa const aa{\n" + "a(aaa, )\n" + "};", + Style); + verifyFormat("bool aaa(\n" "const bool &a, const void *aa\n" ") const {\n" ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Fix regression with BlockIndent of Braced Initializers (PR #108717)
llvmbot wrote: @llvm/pr-subscribers-clang-format Author: Gedare Bloom (gedare) Changes Fixes #73584. --- Full diff: https://github.com/llvm/llvm-project/pull/108717.diff 2 Files Affected: - (modified) clang/lib/Format/ContinuationIndenter.cpp (+7) - (modified) clang/unittests/Format/FormatTest.cpp (+11) ``diff diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index f29f8796ea9290..5c77af2da5add5 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -348,6 +348,13 @@ bool ContinuationIndenter::canBreak(const LineState &State) { } } + // Don't allow breaking before a closing right brace of a block-indented + // braced list initializer if there was not already a break. + if (Current.is(tok::r_brace) && Current.MatchingParen && + Current.isBlockIndentedInitRBrace(Style)) { +return CurrentState.BreakBeforeClosingBrace; + } + // If binary operators are moved to the next line (including commas for some // styles of constructor initializers), that's always ok. if (!Current.isOneOf(TT_BinaryOperator, tok::comma) && diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 5ebf0d7068dd6c..d1cb2b053e33d6 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -9336,6 +9336,9 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) { "ccc(a, //\n" "b));", Style); + verifyFormat("aaa const aa{\n" + "a(aaa, )};", + Style); Style.ColumnLimit = 30; verifyFormat("for (int foo = 0; foo < FOO;\n" @@ -9395,6 +9398,9 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) { "fooo(new FOO::BA(\n" "XXXZ()));", Style); + verifyFormat("aaa const aa{\n" + "a(aaa, )};", + Style); Style.AlignAfterOpenBracket = FormatStyle::BAS_BlockIndent; Style.BinPackArguments = false; @@ -9441,6 +9447,11 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) { "\n" ");", Style); + verifyFormat("aaa const aa{\n" + "a(aaa, )\n" + "};", + Style); + verifyFormat("bool aaa(\n" "const bool &a, const void *aa\n" ") const {\n" `` https://github.com/llvm/llvm-project/pull/108717 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Fix regression in AlwaysBreak for-await (PR #108634)
https://github.com/gedare updated https://github.com/llvm/llvm-project/pull/108634 >From cc3f586d2fee5b89eece3bf341d1ffe1d169e3f7 Mon Sep 17 00:00:00 2001 From: Gedare Bloom Date: Fri, 13 Sep 2024 13:41:49 -0600 Subject: [PATCH 1/2] [clang-format] Fix regression in AlwaysBreak for-await Fixes #108589. --- clang/lib/Format/ContinuationIndenter.cpp | 2 +- clang/unittests/Format/FormatTestJS.cpp | 5 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index f29f8796ea9290..6ed33475882a8e 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -809,7 +809,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, if (Tok.Previous->isIf()) return Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak; return !Tok.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while, - tok::kw_switch); + tok::kw_switch, Keywords.kw_await); }; auto IsFunctionCallParen = [](const FormatToken &Tok) { return Tok.is(tok::l_paren) && Tok.ParameterCount > 0 && Tok.Previous && diff --git a/clang/unittests/Format/FormatTestJS.cpp b/clang/unittests/Format/FormatTestJS.cpp index c25228a69a748f..b129dcd14d9e14 100644 --- a/clang/unittests/Format/FormatTestJS.cpp +++ b/clang/unittests/Format/FormatTestJS.cpp @@ -2865,6 +2865,11 @@ TEST_F(FormatTestJS, BreakAfterOpenBracket) { verifyFormat("failedUserIds.push(await subscriptionSubset.map(\n" "subscription => subscription.getUserId()));", Style); + verifyFormat("for await (const packageId of ops.api.iterateEmbeddedFiles(\n" + "this.getFileId().getDriveFile(),\n" + ")) {\n" + "}", + Style); } } // namespace format >From ff86ae59a764ce30f8c0f4b42d37c30971e7b8d1 Mon Sep 17 00:00:00 2001 From: Gedare Bloom Date: Sat, 14 Sep 2024 13:20:15 -0600 Subject: [PATCH 2/2] Update clang/lib/Format/ContinuationIndenter.cpp Co-authored-by: Owen Pan --- clang/lib/Format/ContinuationIndenter.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index 6ed33475882a8e..4e9ae41b566f49 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -809,7 +809,8 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, if (Tok.Previous->isIf()) return Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak; return !Tok.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while, - tok::kw_switch, Keywords.kw_await); + tok::kw_switch) && + !(Style.isJavaScript() && Tok.Previous->is(Keywords.kw_await)); }; auto IsFunctionCallParen = [](const FormatToken &Tok) { return Tok.is(tok::l_paren) && Tok.ParameterCount > 0 && Tok.Previous && ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Utils] Add new --update-tests flag to llvm-lit (PR #108425)
@@ -2,3 +2,15 @@ config.substitutions = list(config.substitutions) config.substitutions.insert( 0, (r"%clang\b", """*** Do not use the driver in Sema tests. ***""") ) + +if lit_config.update_tests: +import sys +import os + +curdir = os.path.dirname(os.path.realpath(__file__)) +testdir = os.path.dirname(curdir) +clangdir = os.path.dirname(testdir) +utilspath = os.path.join(clangdir, "utils") nhaehnle wrote: I believe this can be simplified to `os.path.join(clangdir, "..", "..", "utils")`? https://github.com/llvm/llvm-project/pull/108425 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Utils] Add new --update-tests flag to llvm-lit (PR #108425)
@@ -1,3 +1,5 @@ +// RUN: %clang_cc1 -verify %s +// RUN: diff %s %s.expected nhaehnle wrote: I still think those new invocations of `diff` should be in `lit-plugin.test`. Those `diff` lines attempt to check that this file (or rather its temporarily copied version) was overwritten correctly. It isn't sound to have the code to check whether a file was correctly overwritten in the file that is overwritten. https://github.com/llvm/llvm-project/pull/108425 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Utils] Add new --update-tests flag to llvm-lit (PR #108425)
https://github.com/nhaehnle commented: Thanks for splitting this up. It does look mostly good to me, but I do have two comments. https://github.com/llvm/llvm-project/pull/108425 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Utils] Add new --update-tests flag to llvm-lit (PR #108425)
https://github.com/nhaehnle edited https://github.com/llvm/llvm-project/pull/108425 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
https://github.com/igelbox updated https://github.com/llvm/llvm-project/pull/108187 >From aee4cf70dedaa3c8b7b6508238e01f92d60c631c Mon Sep 17 00:00:00 2001 From: Sergei Date: Tue, 10 Sep 2024 16:19:05 + Subject: [PATCH 1/4] fix quick OOM in FormatDiagnostic --- .../ClangTidyDiagnosticConsumer.cpp | 2 - clang/include/clang/Basic/Diagnostic.h| 269 ++ clang/include/clang/Basic/DiagnosticIDs.h | 7 +- clang/include/clang/Basic/PartialDiagnostic.h | 5 +- clang/include/clang/Sema/Sema.h | 6 +- clang/lib/Basic/Diagnostic.cpp| 86 +++--- clang/lib/Basic/DiagnosticIDs.cpp | 22 +- clang/lib/Basic/SourceManager.cpp | 23 +- clang/lib/Frontend/Rewrite/FixItRewriter.cpp | 4 +- clang/lib/Frontend/TextDiagnosticPrinter.cpp | 2 +- clang/lib/Sema/Sema.cpp | 19 +- clang/lib/Sema/SemaBase.cpp | 2 +- clang/lib/Serialization/ASTReader.cpp | 15 +- clang/unittests/Basic/DiagnosticTest.cpp | 4 - clang/unittests/Driver/DXCModeTest.cpp| 5 - 15 files changed, 174 insertions(+), 297 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp index 200bb87a5ac3cb..4c75b422701148 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -380,7 +380,6 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic( ++Context.Stats.ErrorsIgnoredNOLINT; // Ignored a warning, should ignore related notes as well LastErrorWasIgnored = true; -Context.DiagEngine->Clear(); for (const auto &Error : SuppressionErrors) Context.diag(Error); return; @@ -457,7 +456,6 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic( if (Info.hasSourceManager()) checkFilters(Info.getLocation(), Info.getSourceManager()); - Context.DiagEngine->Clear(); for (const auto &Error : SuppressionErrors) Context.diag(Error); } diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h index 0c7836c2ea569c..1eecab4f6e49a2 100644 --- a/clang/include/clang/Basic/Diagnostic.h +++ b/clang/include/clang/Basic/Diagnostic.h @@ -183,6 +183,41 @@ struct DiagnosticStorage { DiagnosticStorage() = default; }; +/// An allocator for DiagnosticStorage objects, which uses a small cache to +/// objects, used to reduce malloc()/free() traffic for partial diagnostics. +class DiagStorageAllocator { + static const unsigned NumCached = 16; + DiagnosticStorage Cached[NumCached]; + DiagnosticStorage *FreeList[NumCached]; + unsigned NumFreeListEntries; + +public: + DiagStorageAllocator(); + ~DiagStorageAllocator(); + + /// Allocate new storage. + DiagnosticStorage *Allocate() { +if (NumFreeListEntries == 0) + return new DiagnosticStorage; + +DiagnosticStorage *Result = FreeList[--NumFreeListEntries]; +Result->NumDiagArgs = 0; +Result->DiagRanges.clear(); +Result->FixItHints.clear(); +return Result; + } + + /// Free the given storage object. + void Deallocate(DiagnosticStorage *S) { +if (S >= Cached && S <= Cached + NumCached) { + FreeList[NumFreeListEntries++] = S; + return; +} + +delete S; + } +}; + /// Concrete class used by the front-end to report problems and issues. /// /// This massages the diagnostics (e.g. handling things like "report warnings @@ -520,27 +555,6 @@ class DiagnosticsEngine : public RefCountedBase { void *ArgToStringCookie = nullptr; ArgToStringFnTy ArgToStringFn; - /// ID of the "delayed" diagnostic, which is a (typically - /// fatal) diagnostic that had to be delayed because it was found - /// while emitting another diagnostic. - unsigned DelayedDiagID; - - /// First string argument for the delayed diagnostic. - std::string DelayedDiagArg1; - - /// Second string argument for the delayed diagnostic. - std::string DelayedDiagArg2; - - /// Third string argument for the delayed diagnostic. - std::string DelayedDiagArg3; - - /// Optional flag value. - /// - /// Some flags accept values, for instance: -Wframe-larger-than= and - /// -Rpass=. The content of this string is emitted after the flag name - /// and '='. - std::string FlagValue; - public: explicit DiagnosticsEngine(IntrusiveRefCntPtr Diags, IntrusiveRefCntPtr DiagOpts, @@ -945,50 +959,11 @@ class DiagnosticsEngine : public RefCountedBase { void Report(const StoredDiagnostic &storedDiag); - /// Determine whethere there is already a diagnostic in flight. - bool isDiagnosticInFlight() const { -return CurDiagID != std::numeric_limits::max(); - } - - /// Set the "delayed" diagnostic that will be emitted once - /// the current diagnostic completes. - /// - /// If a diagnostic is already in-flight but the front end must - /// repo
[clang] [clang-tools-extra] Fix OOM in FormatDiagnostic (PR #108187)
@@ -945,50 +959,11 @@ class DiagnosticsEngine : public RefCountedBase { void Report(const StoredDiagnostic &storedDiag); - /// Determine whethere there is already a diagnostic in flight. - bool isDiagnosticInFlight() const { -return CurDiagID != std::numeric_limits::max(); - } - - /// Set the "delayed" diagnostic that will be emitted once - /// the current diagnostic completes. - /// - /// If a diagnostic is already in-flight but the front end must - /// report a problem (e.g., with an inconsistent file system - /// state), this routine sets a "delayed" diagnostic that will be - /// emitted after the current diagnostic completes. This should - /// only be used for fatal errors detected at inconvenient - /// times. If emitting a delayed diagnostic causes a second delayed - /// diagnostic to be introduced, that second delayed diagnostic - /// will be ignored. - /// - /// \param DiagID The ID of the diagnostic being delayed. - /// - /// \param Arg1 A string argument that will be provided to the - /// diagnostic. A copy of this string will be stored in the - /// DiagnosticsEngine object itself. - /// - /// \param Arg2 A string argument that will be provided to the - /// diagnostic. A copy of this string will be stored in the - /// DiagnosticsEngine object itself. - /// - /// \param Arg3 A string argument that will be provided to the - /// diagnostic. A copy of this string will be stored in the - /// DiagnosticsEngine object itself. - void SetDelayedDiagnostic(unsigned DiagID, StringRef Arg1 = "", -StringRef Arg2 = "", StringRef Arg3 = ""); - - /// Clear out the current diagnostic. - void Clear() { CurDiagID = std::numeric_limits::max(); } - - /// Return the value associated with this diagnostic flag. - StringRef getFlagValue() const { return FlagValue; } - private: // This is private state used by DiagnosticBuilder. We put it here instead of // in DiagnosticBuilder in order to keep DiagnosticBuilder a small lightweight - // object. This implementation choice means that we can only have one - // diagnostic "in flight" at a time, but this seems to be a reasonable + // object. This implementation choice means that we can only have a few + // diagnostics "in flight" at a time, but this seems to be a reasonable // tradeoff to keep these objects small. Assertions verify that only one // diagnostic is in flight at a time. igelbox wrote: Okay. Removed misleading `Cur`-prefix (coz it isn't current it's the one per builder/diagnostic obj) and seemingly meaningless comments as well. https://github.com/llvm/llvm-project/pull/108187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] e392056 - Reapply "Reapply "[clang] Extend diagnose_if to accept more detailed warning information (#70976)" (#108453)"
Author: Nikolas Klauser Date: 2024-09-14T22:25:08+02:00 New Revision: e39205654dc11c50bd117e8ccac243a641ebd71f URL: https://github.com/llvm/llvm-project/commit/e39205654dc11c50bd117e8ccac243a641ebd71f DIFF: https://github.com/llvm/llvm-project/commit/e39205654dc11c50bd117e8ccac243a641ebd71f.diff LOG: Reapply "Reapply "[clang] Extend diagnose_if to accept more detailed warning information (#70976)" (#108453)" This reverts commit e1bd9740faa62c11cc785a7b70ec1ad17e286bd1. Fixes incorrect use of the `DiagnosticsEngine` in the clangd tests. Added: clang/test/SemaCXX/diagnose_if-warning-group.cpp Modified: clang-tools-extra/clangd/Diagnostics.cpp clang-tools-extra/clangd/Diagnostics.h clang-tools-extra/clangd/ParsedAST.cpp clang-tools-extra/clangd/Preamble.cpp clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp clang/include/clang/Basic/Attr.td clang/include/clang/Basic/Diagnostic.h clang/include/clang/Basic/DiagnosticCategories.h clang/include/clang/Basic/DiagnosticIDs.h clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Basic/Diagnostic.cpp clang/lib/Basic/DiagnosticIDs.cpp clang/lib/Frontend/LogDiagnosticPrinter.cpp clang/lib/Frontend/SerializedDiagnosticPrinter.cpp clang/lib/Frontend/TextDiagnosticPrinter.cpp clang/lib/Sema/Sema.cpp clang/lib/Sema/SemaCUDA.cpp clang/lib/Sema/SemaDeclAttr.cpp clang/lib/Sema/SemaOverload.cpp clang/lib/Sema/SemaTemplateInstantiateDecl.cpp clang/lib/Serialization/ASTReader.cpp clang/lib/Serialization/ASTWriter.cpp clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp clang/test/Sema/diagnose_if.c clang/tools/diagtool/ListWarnings.cpp clang/tools/diagtool/ShowEnabledWarnings.cpp clang/tools/libclang/CXStoredDiagnostic.cpp flang/lib/Frontend/TextDiagnosticPrinter.cpp Removed: diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index d5eca083eb6512..552dd36b6900bf 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -579,7 +579,17 @@ std::vector StoreDiags::take(const clang::tidy::ClangTidyContext *Tidy) { for (auto &Diag : Output) { if (const char *ClangDiag = getDiagnosticCode(Diag.ID)) { // Warnings controlled by -Wfoo are better recognized by that name. - StringRef Warning = DiagnosticIDs::getWarningOptionForDiag(Diag.ID); + const StringRef Warning = [&] { +if (OrigSrcMgr) { + return OrigSrcMgr->getDiagnostics() + .getDiagnosticIDs() + ->getWarningOptionForDiag(Diag.ID); +} +if (!DiagnosticIDs::IsCustomDiag(Diag.ID)) + return DiagnosticIDs{}.getWarningOptionForDiag(Diag.ID); +return StringRef{}; + }(); + if (!Warning.empty()) { Diag.Name = ("-W" + Warning).str(); } else { @@ -896,20 +906,23 @@ void StoreDiags::flushLastDiag() { Output.push_back(std::move(*LastDiag)); } -bool isBuiltinDiagnosticSuppressed(unsigned ID, - const llvm::StringSet<> &Suppress, - const LangOptions &LangOpts) { +bool isDiagnosticSuppressed(const clang::Diagnostic &Diag, +const llvm::StringSet<> &Suppress, +const LangOptions &LangOpts) { // Don't complain about header-only stuff in mainfiles if it's a header. // FIXME: would be cleaner to suppress in clang, once we decide whether the //behavior should be to silently-ignore or respect the pragma. - if (ID == diag::pp_pragma_sysheader_in_main_file && LangOpts.IsHeaderFile) + if (Diag.getID() == diag::pp_pragma_sysheader_in_main_file && + LangOpts.IsHeaderFile) return true; - if (const char *CodePtr = getDiagnosticCode(ID)) { + if (const char *CodePtr = getDiagnosticCode(Diag.getID())) { if (Suppress.contains(normalizeSuppressedCode(CodePtr))) return true; } - StringRef Warning = DiagnosticIDs::getWarningOptionForDiag(ID); + StringRef Warning = + Diag.getDiags()->getDiagnosticIDs()->getWarningOptionForDiag( + Diag.getID()); if (!Warning.empty() && Suppress.contains(Warning)) return true; return false; diff --git a/clang-tools-extra/clangd/Diagnostics.h b/clang-tools-extra/clangd/Diagnostics.h index d4c0478c63a5cf..c45d8dc3aa6ce6 100644 --- a/clang-tools-extra/clangd/Diagnostics.h +++ b/clang-tools-extra/clangd/Diagnostics.h @@ -181,11 +181,11 @@ class StoreDiags : public DiagnosticConsumer { }; /// Determine whether a (non-clang-tidy) diagnostic is suppressed by config. -bool isBuiltinDiagnosticSuppressed(unsigned ID, - const llvm::StringSet<> &Suppressed, - const LangOptions &); +bool isDiagnosticSuppressed(const clang::Diagnostic &Di
[clang-tools-extra] e392056 - Reapply "Reapply "[clang] Extend diagnose_if to accept more detailed warning information (#70976)" (#108453)"
Author: Nikolas Klauser Date: 2024-09-14T22:25:08+02:00 New Revision: e39205654dc11c50bd117e8ccac243a641ebd71f URL: https://github.com/llvm/llvm-project/commit/e39205654dc11c50bd117e8ccac243a641ebd71f DIFF: https://github.com/llvm/llvm-project/commit/e39205654dc11c50bd117e8ccac243a641ebd71f.diff LOG: Reapply "Reapply "[clang] Extend diagnose_if to accept more detailed warning information (#70976)" (#108453)" This reverts commit e1bd9740faa62c11cc785a7b70ec1ad17e286bd1. Fixes incorrect use of the `DiagnosticsEngine` in the clangd tests. Added: clang/test/SemaCXX/diagnose_if-warning-group.cpp Modified: clang-tools-extra/clangd/Diagnostics.cpp clang-tools-extra/clangd/Diagnostics.h clang-tools-extra/clangd/ParsedAST.cpp clang-tools-extra/clangd/Preamble.cpp clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp clang/include/clang/Basic/Attr.td clang/include/clang/Basic/Diagnostic.h clang/include/clang/Basic/DiagnosticCategories.h clang/include/clang/Basic/DiagnosticIDs.h clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Basic/Diagnostic.cpp clang/lib/Basic/DiagnosticIDs.cpp clang/lib/Frontend/LogDiagnosticPrinter.cpp clang/lib/Frontend/SerializedDiagnosticPrinter.cpp clang/lib/Frontend/TextDiagnosticPrinter.cpp clang/lib/Sema/Sema.cpp clang/lib/Sema/SemaCUDA.cpp clang/lib/Sema/SemaDeclAttr.cpp clang/lib/Sema/SemaOverload.cpp clang/lib/Sema/SemaTemplateInstantiateDecl.cpp clang/lib/Serialization/ASTReader.cpp clang/lib/Serialization/ASTWriter.cpp clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp clang/test/Sema/diagnose_if.c clang/tools/diagtool/ListWarnings.cpp clang/tools/diagtool/ShowEnabledWarnings.cpp clang/tools/libclang/CXStoredDiagnostic.cpp flang/lib/Frontend/TextDiagnosticPrinter.cpp Removed: diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index d5eca083eb6512..552dd36b6900bf 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -579,7 +579,17 @@ std::vector StoreDiags::take(const clang::tidy::ClangTidyContext *Tidy) { for (auto &Diag : Output) { if (const char *ClangDiag = getDiagnosticCode(Diag.ID)) { // Warnings controlled by -Wfoo are better recognized by that name. - StringRef Warning = DiagnosticIDs::getWarningOptionForDiag(Diag.ID); + const StringRef Warning = [&] { +if (OrigSrcMgr) { + return OrigSrcMgr->getDiagnostics() + .getDiagnosticIDs() + ->getWarningOptionForDiag(Diag.ID); +} +if (!DiagnosticIDs::IsCustomDiag(Diag.ID)) + return DiagnosticIDs{}.getWarningOptionForDiag(Diag.ID); +return StringRef{}; + }(); + if (!Warning.empty()) { Diag.Name = ("-W" + Warning).str(); } else { @@ -896,20 +906,23 @@ void StoreDiags::flushLastDiag() { Output.push_back(std::move(*LastDiag)); } -bool isBuiltinDiagnosticSuppressed(unsigned ID, - const llvm::StringSet<> &Suppress, - const LangOptions &LangOpts) { +bool isDiagnosticSuppressed(const clang::Diagnostic &Diag, +const llvm::StringSet<> &Suppress, +const LangOptions &LangOpts) { // Don't complain about header-only stuff in mainfiles if it's a header. // FIXME: would be cleaner to suppress in clang, once we decide whether the //behavior should be to silently-ignore or respect the pragma. - if (ID == diag::pp_pragma_sysheader_in_main_file && LangOpts.IsHeaderFile) + if (Diag.getID() == diag::pp_pragma_sysheader_in_main_file && + LangOpts.IsHeaderFile) return true; - if (const char *CodePtr = getDiagnosticCode(ID)) { + if (const char *CodePtr = getDiagnosticCode(Diag.getID())) { if (Suppress.contains(normalizeSuppressedCode(CodePtr))) return true; } - StringRef Warning = DiagnosticIDs::getWarningOptionForDiag(ID); + StringRef Warning = + Diag.getDiags()->getDiagnosticIDs()->getWarningOptionForDiag( + Diag.getID()); if (!Warning.empty() && Suppress.contains(Warning)) return true; return false; diff --git a/clang-tools-extra/clangd/Diagnostics.h b/clang-tools-extra/clangd/Diagnostics.h index d4c0478c63a5cf..c45d8dc3aa6ce6 100644 --- a/clang-tools-extra/clangd/Diagnostics.h +++ b/clang-tools-extra/clangd/Diagnostics.h @@ -181,11 +181,11 @@ class StoreDiags : public DiagnosticConsumer { }; /// Determine whether a (non-clang-tidy) diagnostic is suppressed by config. -bool isBuiltinDiagnosticSuppressed(unsigned ID, - const llvm::StringSet<> &Suppressed, - const LangOptions &); +bool isDiagnosticSuppressed(const clang::Diagnostic &Di
[clang] 7d11f52 - [clang] Silence GCC warnings about control reaching end of non void function
Author: Martin Storsjö Date: 2024-09-15T00:02:39+03:00 New Revision: 7d11f5249c8068851270e94ac02c180e7b83590d URL: https://github.com/llvm/llvm-project/commit/7d11f5249c8068851270e94ac02c180e7b83590d DIFF: https://github.com/llvm/llvm-project/commit/7d11f5249c8068851270e94ac02c180e7b83590d.diff LOG: [clang] Silence GCC warnings about control reaching end of non void function This fixes GCC warnings since e39205654dc11c50bd117e8ccac243a641ebd71f. Added: Modified: clang/include/clang/Basic/DiagnosticIDs.h clang/lib/Sema/SemaOverload.cpp Removed: diff --git a/clang/include/clang/Basic/DiagnosticIDs.h b/clang/include/clang/Basic/DiagnosticIDs.h index 2402996ece5c94..daad66f499538f 100644 --- a/clang/include/clang/Basic/DiagnosticIDs.h +++ b/clang/include/clang/Basic/DiagnosticIDs.h @@ -18,6 +18,7 @@ #include "clang/Basic/LLVM.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/ErrorHandling.h" #include #include @@ -310,6 +311,7 @@ class DiagnosticIDs : public RefCountedBase { return {diag::Severity::Fatal, std::string(Message), CLASS_ERROR, /*ShowInSystemHeader*/ true}; } + llvm_unreachable("Fully covered switch above!"); }()); } diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index fbeb6ee5b052f3..d304f322aced64 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -7324,6 +7324,7 @@ static bool diagnoseDiagnoseIfAttrsWith(Sema &S, const NamedDecl *ND, case DiagnoseIfAttr::DS_error: return diag::Severity::Error; } +llvm_unreachable("Fully covered switch above!"); }; for (const auto *DIA : llvm::make_range(WarningBegin, Attrs.end())) ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
https://github.com/cor3ntin approved this pull request. Thanks, LGTM https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
a-tarasyuk wrote: @cor3ntin when you get a chance, could you merge this? I don’t have the permissions. thanks! https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
shafik wrote: So this looks like a regression: https://gcc.godbolt.org/z/h6GexT18E Can we figure out which PR this regression came from and make sure the code change makes sense in light of that PR? Also please add a more detailed summary to this PR, describing why the crash happens and the approach the PR takes to fixing it. https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
https://github.com/shafik requested changes to this pull request. So this looks like a regression: https://gcc.godbolt.org/z/h6GexT18E Can we figure out which PR this regression came from and make sure the code change makes sense in light of that PR? Also please add a more detailed summary to this PR, describing why the crash happens and the approach the PR takes to fixing it. https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
a-tarasyuk wrote: > So this looks like a regression: https://gcc.godbolt.org/z/h6GexT18E @shafik https://gcc.godbolt.org/z/Ebvnjrc8e https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
shafik wrote: > > So this looks like a regression: https://gcc.godbolt.org/z/h6GexT18E > > @shafik https://gcc.godbolt.org/z/Ebvnjrc8e Interesting it crashes in trunk w/o `c++20`: https://gcc.godbolt.org/z/h9xa8eaqo but the backtrace looks the same. We should test this in C++20 and C++23 as well then. https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
https://github.com/a-tarasyuk updated https://github.com/llvm/llvm-project/pull/108701 >From cbe06cf33277f904fe002db90d428a55b777ff5d Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Sat, 14 Sep 2024 17:02:01 +0300 Subject: [PATCH 1/2] [Clang] handle invalid close location in static assert declaration --- clang/docs/ReleaseNotes.rst| 2 +- clang/lib/Parse/ParseDeclCXX.cpp | 3 ++- clang/test/SemaCXX/static-assert-cxx26.cpp | 3 +++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 79b154ef1aef5e..27dde935ad19b9 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -388,7 +388,7 @@ Bug Fixes to C++ Support - Fixed a crash in the typo correction of an invalid CTAD guide. (#GH107887) - Fixed a crash when clang tries to subtitute parameter pack while retaining the parameter pack. #GH63819, #GH107560 - +- Fix a crash when a static assert declaration has an invalid close location. (#GH108687) Bug Fixes to AST Handling ^ diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index 6370da1fab0042..6f0f5a0311bc18 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -1109,7 +1109,8 @@ Decl *Parser::ParseStaticAssertDeclaration(SourceLocation &DeclEnd) { } } - T.consumeClose(); + if (T.consumeClose()) +return nullptr; DeclEnd = Tok.getLocation(); ExpectAndConsumeSemi(diag::err_expected_semi_after_static_assert, TokName); diff --git a/clang/test/SemaCXX/static-assert-cxx26.cpp b/clang/test/SemaCXX/static-assert-cxx26.cpp index 7d896d8b365b74..95ffa8cd68cf7c 100644 --- a/clang/test/SemaCXX/static-assert-cxx26.cpp +++ b/clang/test/SemaCXX/static-assert-cxx26.cpp @@ -415,3 +415,6 @@ static_assert( // expected-note@-1 {{read of dereferenced one-past-the-end pointer is not allowed in a constant expression}} ); } + +static_assert(true, "" // expected-note {{to match this '('}} + // expected-error {{expected ')'}} >From 77063cc64c5047144c2fa0291c38248401d33164 Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Sun, 15 Sep 2024 00:51:21 +0300 Subject: [PATCH 2/2] add test to cover c++2a compliance --- clang/test/SemaCXX/static-assert-cxx20.cpp | 5 + 1 file changed, 5 insertions(+) create mode 100644 clang/test/SemaCXX/static-assert-cxx20.cpp diff --git a/clang/test/SemaCXX/static-assert-cxx20.cpp b/clang/test/SemaCXX/static-assert-cxx20.cpp new file mode 100644 index 00..479fd14dc5bfad --- /dev/null +++ b/clang/test/SemaCXX/static-assert-cxx20.cpp @@ -0,0 +1,5 @@ +// RUN: %clang_cc1 -std=c++2a -triple=x86_64-linux -fsyntax-only %s -verify + +static_assert(true, "" // expected-warning {{'static_assert' with a user-generated message is a C++26 extension}} \ + // expected-note {{to match this '('}} + // expected-error {{expected ')'}} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [git-clang-format] Fix: make the tool backward compatible (PR #108721)
https://github.com/edenreich created https://github.com/llvm/llvm-project/pull/108721 ## Summary Currently it's not backward compatible, because the flag -list-ignored doesn't exists on older version of clang-format. To make this tool still backward compatible, we can first check if the .clang-format-ignore file exists before we run clang-format with the -list-ignored flag. >From 04bf30e5d21460d8ceef1e2ef514b3c4653530f4 Mon Sep 17 00:00:00 2001 From: Eden Reich Date: Sun, 15 Sep 2024 00:25:50 +0200 Subject: [PATCH] Fix git-clang-format Currently it's not backward compatible, because the flag -list-ignored doesn't exists on older version of clang-format. To make this tool still backward compatible, we can first check if the .clang-format-ignore file exists before we run clang-format with the -list-ignored flag. --- clang/tools/clang-format/git-clang-format | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clang/tools/clang-format/git-clang-format b/clang/tools/clang-format/git-clang-format index bacbd8de245666..997644045c1f25 100755 --- a/clang/tools/clang-format/git-clang-format +++ b/clang/tools/clang-format/git-clang-format @@ -402,6 +402,8 @@ def filter_symlinks(dictionary): def filter_ignored_files(dictionary, binary): """Delete every key in `dictionary` that is ignored by clang-format.""" + if not os.path.exists('.clang-format-ignore'): +return ignored_files = run(binary, '-list-ignored', *dictionary.keys()) if not ignored_files: return ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [git-clang-format] Fix: make the tool backward compatible (PR #108721)
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/108721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [git-clang-format] Fix: make the tool backward compatible (PR #108721)
llvmbot wrote: @llvm/pr-subscribers-clang-format Author: Eden Reich (edenreich) Changes ## Summary Currently it's not backward compatible, because the flag -list-ignored doesn't exists on older version of clang-format. To make this tool still backward compatible, we can first check if the .clang-format-ignore file exists before we run clang-format with the -list-ignored flag. --- Full diff: https://github.com/llvm/llvm-project/pull/108721.diff 1 Files Affected: - (modified) clang/tools/clang-format/git-clang-format (+2) ``diff diff --git a/clang/tools/clang-format/git-clang-format b/clang/tools/clang-format/git-clang-format index bacbd8de245666..997644045c1f25 100755 --- a/clang/tools/clang-format/git-clang-format +++ b/clang/tools/clang-format/git-clang-format @@ -402,6 +402,8 @@ def filter_symlinks(dictionary): def filter_ignored_files(dictionary, binary): """Delete every key in `dictionary` that is ignored by clang-format.""" + if not os.path.exists('.clang-format-ignore'): +return ignored_files = run(binary, '-list-ignored', *dictionary.keys()) if not ignored_files: return `` https://github.com/llvm/llvm-project/pull/108721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [git-clang-format] Fix: make the tool backward compatible (PR #108721)
https://github.com/edenreich edited https://github.com/llvm/llvm-project/pull/108721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [git-clang-format] Fix: make the tool backward compatible (PR #108721)
https://github.com/edenreich edited https://github.com/llvm/llvm-project/pull/108721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [git-clang-format] Fix: make the tool backward compatible (PR #108721)
https://github.com/edenreich edited https://github.com/llvm/llvm-project/pull/108721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Avoid transforming lambdas when rebuilding immediate expressions (PR #108693)
https://github.com/zyn0217 ready_for_review https://github.com/llvm/llvm-project/pull/108693 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Avoid transforming lambdas when rebuilding immediate expressions (PR #108693)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Younan Zhang (zyn0217) Changes When rebuilding immediate invocations inside `RemoveNestedImmediateInvocation()`, we employed a `TreeTransform` to exercise the traversal. The transformation has a side effect that, for template specialization types, their default template arguments are substituted separately, and if any lambdas are present, they will be transformed into distinct types than those used to instantiate the templates right before the `consteval` handling. This resulted in `B::func()` getting redundantly instantiated for the case in question. Since we're also in an immediate evaluation context, the body of `foo()` would also get instantiated, so we end up with a spurious friend redefinition error. Like what we have done in `ComplexRemove`, this patch also avoids the lambda's transformation in TemplateInstantiator if we know we're rebuilding immediate calls. In addition, this patch also consolidates the default argument substitution logic in `CheckTemplateArgumentList()`. Fixes #107175 --- Full diff: https://github.com/llvm/llvm-project/pull/108693.diff 6 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+2-2) - (modified) clang/lib/Sema/SemaExpr.cpp (+2-2) - (modified) clang/lib/Sema/SemaTemplate.cpp (+18-37) - (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+4) - (modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (+7-3) - (modified) clang/test/SemaCXX/cxx2a-consteval.cpp (+24) ``diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 79b154ef1aef5e..022c10e11c545a 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -387,8 +387,8 @@ Bug Fixes to C++ Support - A follow-up fix was added for (#GH61460), as the previous fix was not entirely correct. (#GH86361) - Fixed a crash in the typo correction of an invalid CTAD guide. (#GH107887) - Fixed a crash when clang tries to subtitute parameter pack while retaining the parameter - pack. #GH63819, #GH107560 - + pack. (#GH63819), (#GH107560) +- Avoided a redundant friend declaration instantiation under the certain ``consteval`` contexts. (#GH107175) Bug Fixes to AST Handling ^ diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 8f3e15cc9a9bb7..2a22b05afe1c34 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -17521,7 +17521,7 @@ static void RemoveNestedImmediateInvocation( else break; } - /// ConstantExpr are the first layer of implicit node to be removed so if + /// ConstantExprs are the first layer of implicit node to be removed so if /// Init isn't a ConstantExpr, no ConstantExpr will be skipped. if (auto *CE = dyn_cast(Init); CE && CE->isImmediateInvocation()) @@ -17534,7 +17534,7 @@ static void RemoveNestedImmediateInvocation( } ExprResult TransformLambdaExpr(LambdaExpr *E) { // Do not rebuild lambdas to avoid creating a new type. - // Lambdas have already been processed inside their eval context. + // Lambdas have already been processed inside their eval contexts. return E; } bool AlwaysRebuild() { return false; } diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index e5ea02a919f4eb..b052afede2cd67 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -5508,50 +5508,31 @@ bool Sema::CheckTemplateArgumentList( } // Check whether we have a default argument. -TemplateArgumentLoc Arg; +bool HasDefaultArg; // Retrieve the default template argument from the template // parameter. For each kind of template parameter, we substitute the // template arguments provided thus far and any "outer" template arguments // (when the template parameter was part of a nested template) into // the default argument. -if (TemplateTypeParmDecl *TTP = dyn_cast(*Param)) { - if (!hasReachableDefaultArgument(TTP)) -return diagnoseMissingArgument(*this, TemplateLoc, Template, TTP, +TemplateArgumentLoc Arg = SubstDefaultTemplateArgumentIfAvailable( +Template, TemplateLoc, RAngleLoc, *Param, SugaredConverted, +CanonicalConverted, HasDefaultArg); + +if (Arg.getArgument().isNull()) { + if (!HasDefaultArg) { +if (TemplateTypeParmDecl *TTP = dyn_cast(*Param)) + return diagnoseMissingArgument(*this, TemplateLoc, Template, TTP, + NewArgs); +if (NonTypeTemplateParmDecl *NTTP = +dyn_cast(*Param)) + return diagnoseMissingArgument(*this, TemplateLoc, Template, NTTP, + NewArgs); +return diagnoseMissingArgument(*this, TemplateLoc, Template, + cast(*Param), NewArgs); - -
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
@@ -0,0 +1,5 @@ +// RUN: %clang_cc1 -std=c++2a -triple=x86_64-linux -fsyntax-only %s -verify zyn0217 wrote: You can add another C++20 mode run to clang/test/SemaCXX/static-assert-cxx26.cpp, instead of adding a new file. https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
@@ -0,0 +1,5 @@ +// RUN: %clang_cc1 -std=c++2a -triple=x86_64-linux -fsyntax-only %s -verify a-tarasyuk wrote: The file refers to cxx26 —is it ok to add tests related to C++20 here?" https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
https://github.com/a-tarasyuk edited https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
@@ -0,0 +1,5 @@ +// RUN: %clang_cc1 -std=c++2a -triple=x86_64-linux -fsyntax-only %s -verify zyn0217 wrote: I believe so, yes. See the example: clang/test/SemaCXX/constant-expression-cxx2b.cpp https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
@@ -0,0 +1,5 @@ +// RUN: %clang_cc1 -std=c++2a -triple=x86_64-linux -fsyntax-only %s -verify zyn0217 wrote: Also, you might want to wrap the case with a namespace referring to the issue number. Furthermore, I think the test should be triaged as a Parser one. Can you please move the test to that directory? https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
zyn0217 wrote: > Can we figure out which PR this regression came from and make sure the code > change makes sense in light of that PR? https://github.com/llvm/llvm-project/pull/102044. We have backported the P2741R3 implementation to C++11. https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
https://github.com/zyn0217 commented: So I took a stab at it. In `ParseStaticAssertDeclaration()`, we parsed the message in unclosed static_assert as a constant expression rather than an unevaluated string literal per the logic on lines 1079-1089. So this approach gives up building an expression for unclosed expressions, which works but also loses the diagnoses of the condition expression where it would otherwise evaluate to false: ```cpp static_assert(false, "" ``` Prior to clang 19, with C++11 mode, we give three errors for the above code: https://gcc.godbolt.org/z/PMPab779n (note the last `error: static assertion failed`) While with this patch, we no longer diagnose it. Note that even on the assertion-free trunk with c++26 mode, we do complain about the false conditions. I'm not sure if this is acceptable for us. https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
https://github.com/zyn0217 edited https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] handle invalid close location in static assert declaration (PR #108701)
https://github.com/zyn0217 edited https://github.com/llvm/llvm-project/pull/108701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wimplicit-fallthrough to -Wextra (PR #97926)
vegerot wrote: @AaronBallman sure. Should I still use http://llvm-compile-time-tracker.com/ , or do I run the benchmark another way? https://github.com/llvm/llvm-project/pull/97926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC] Correct the misuse of the API in the Clang test-report script. (PR #108725)
https://github.com/c8ef created https://github.com/llvm/llvm-project/pull/108725 ref: https://docs.python.org/3/library/unittest.html#unittest.TestCase.assertEqual >From 1f56fd3444b6f7667a7036a29cba825f7eac77b0 Mon Sep 17 00:00:00 2001 From: c8ef Date: Sun, 15 Sep 2024 10:00:12 +0800 Subject: [PATCH] Update test_report.py --- clang/tools/scan-build-py/tests/unit/test_report.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/tools/scan-build-py/tests/unit/test_report.py b/clang/tools/scan-build-py/tests/unit/test_report.py index 4d85590a211229..88dcb965d0da34 100644 --- a/clang/tools/scan-build-py/tests/unit/test_report.py +++ b/clang/tools/scan-build-py/tests/unit/test_report.py @@ -538,7 +538,7 @@ def test_merge_updates_embedded_link(self): "test message 6-1 [link](sarif:/runs/4/results/0)", ], ) -self.assertEquals( +self.assertEqual( thread_flows, [ "test message 1-2 [link](sarif:/runs/1/results/0)", ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC] Correct the misuse of the API in the Clang test-report script. (PR #108725)
llvmbot wrote: @llvm/pr-subscribers-clang Author: None (c8ef) Changes ref: https://docs.python.org/3/library/unittest.html#unittest.TestCase.assertEqual --- Full diff: https://github.com/llvm/llvm-project/pull/108725.diff 1 Files Affected: - (modified) clang/tools/scan-build-py/tests/unit/test_report.py (+1-1) ``diff diff --git a/clang/tools/scan-build-py/tests/unit/test_report.py b/clang/tools/scan-build-py/tests/unit/test_report.py index 4d85590a211229..88dcb965d0da34 100644 --- a/clang/tools/scan-build-py/tests/unit/test_report.py +++ b/clang/tools/scan-build-py/tests/unit/test_report.py @@ -538,7 +538,7 @@ def test_merge_updates_embedded_link(self): "test message 6-1 [link](sarif:/runs/4/results/0)", ], ) -self.assertEquals( +self.assertEqual( thread_flows, [ "test message 1-2 [link](sarif:/runs/1/results/0)", `` https://github.com/llvm/llvm-project/pull/108725 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wimplicit-fallthrough to -Wextra (PR #97926)
https://github.com/vegerot updated https://github.com/llvm/llvm-project/pull/97926 >From 7d221cbcf91248f44b016aea107109756fa9f785 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20=F0=9F=91=A8=F0=9F=8F=BD=E2=80=8D=F0=9F=92=BB=20Copl?= =?UTF-8?q?an?= Date: Sat, 6 Jul 2024 17:22:55 -0700 Subject: [PATCH] [clang] Add -Wimplicit-fallthrough to -Wextra This patch adds -Wimplicit-fallthrough to -Wextra. GCC already includes it in -Wextra. This patch also adds a test to check that -Wimplicit-fallthrough is included in -Wextra. Note: This patch may regress performance when building with -Wextra. This is because -Wextra requires forming a CFG for every function. --- clang/include/clang/Basic/DiagnosticGroups.td| 1 + clang/test/Sema/fallthrough-attr.c | 1 + clang/test/SemaCXX/switch-implicit-fallthrough-macro.cpp | 1 + 3 files changed, 3 insertions(+) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index e250f81a0b52a5..78b412c68ea07c 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1067,6 +1067,7 @@ def Extra : DiagGroup<"extra", [ StringConcatation, FUseLdPath, CastFunctionTypeMismatch, +ImplicitFallthrough, ]>; def Most : DiagGroup<"most", [ diff --git a/clang/test/Sema/fallthrough-attr.c b/clang/test/Sema/fallthrough-attr.c index de50ebf39d42f6..6cc19136f30a7e 100644 --- a/clang/test/Sema/fallthrough-attr.c +++ b/clang/test/Sema/fallthrough-attr.c @@ -2,6 +2,7 @@ // RUN: %clang_cc1 -fsyntax-only -std=gnu99 -verify -Wimplicit-fallthrough %s // RUN: %clang_cc1 -fsyntax-only -std=c99 -verify -Wimplicit-fallthrough %s // RUN: %clang_cc1 -fsyntax-only -std=c11 -verify -Wimplicit-fallthrough %s +// RUN: %clang_cc1 -fsyntax-only -std=c11 -verify -Wextra %s // RUN: %clang_cc1 -fsyntax-only -std=c2x -DC2X -verify -Wimplicit-fallthrough %s int fallthrough_attribute_spelling(int n) { diff --git a/clang/test/SemaCXX/switch-implicit-fallthrough-macro.cpp b/clang/test/SemaCXX/switch-implicit-fallthrough-macro.cpp index 11df2cbfb53f09..cbbff1f1793b88 100644 --- a/clang/test/SemaCXX/switch-implicit-fallthrough-macro.cpp +++ b/clang/test/SemaCXX/switch-implicit-fallthrough-macro.cpp @@ -3,6 +3,7 @@ // RUN: %clang_cc1 -fsyntax-only -verify -std=c++1z -Wimplicit-fallthrough -DCLANG_PREFIX -DCOMMAND_LINE_FALLTHROUGH=[[clang::fallthrough]] %s // RUN: %clang_cc1 -fsyntax-only -verify -std=c++1z -Wimplicit-fallthrough -DCOMMAND_LINE_FALLTHROUGH=[[clang::fallthrough]] %s // RUN: %clang_cc1 -fsyntax-only -verify -std=c++1z -Wimplicit-fallthrough -DCOMMAND_LINE_FALLTHROUGH=[[fallthrough]] -DUNCHOSEN=[[clang::fallthrough]] %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++1z -Wextra -DCOMMAND_LINE_FALLTHROUGH=[[fallthrough]] -DUNCHOSEN=[[clang::fallthrough]] %s int fallthrough_compatibility_macro_from_command_line(int n) { switch (n) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [ThinLTO][NFC] Prep for two-codegen rounds (PR #90934)
https://github.com/kyulee-com updated https://github.com/llvm/llvm-project/pull/90934 >From 95709740e820c0efddb1fdb53436a03194a1b88e Mon Sep 17 00:00:00 2001 From: Kyungwoo Lee Date: Fri, 26 Apr 2024 20:02:52 -0700 Subject: [PATCH] [ThinLTO][NFC] Prep for two-codegen rounds --- clang/lib/CodeGen/BackendUtil.cpp | 8 ++-- llvm/include/llvm/LTO/LTOBackend.h | 1 + llvm/lib/LTO/LTO.cpp | 75 -- llvm/lib/LTO/LTOBackend.cpp| 6 ++- 4 files changed, 49 insertions(+), 41 deletions(-) diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index 7fa69420298160..a1909d45b4d944 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -1286,10 +1286,10 @@ static void runThinLTOBackend( Conf.CGFileType = getCodeGenFileType(Action); break; } - if (Error E = - thinBackend(Conf, -1, AddStream, *M, *CombinedIndex, ImportList, - ModuleToDefinedGVSummaries[M->getModuleIdentifier()], - /* ModuleMap */ nullptr, CGOpts.CmdArgs)) { + if (Error E = thinBackend( + Conf, -1, AddStream, *M, *CombinedIndex, ImportList, + ModuleToDefinedGVSummaries[M->getModuleIdentifier()], + /* ModuleMap */ nullptr, Conf.CodeGenOnly, CGOpts.CmdArgs)) { handleAllErrors(std::move(E), [&](ErrorInfoBase &EIB) { errs() << "Error running ThinLTO backend: " << EIB.message() << '\n'; }); diff --git a/llvm/include/llvm/LTO/LTOBackend.h b/llvm/include/llvm/LTO/LTOBackend.h index de89f4bb10dff2..8516398510d4b8 100644 --- a/llvm/include/llvm/LTO/LTOBackend.h +++ b/llvm/include/llvm/LTO/LTOBackend.h @@ -56,6 +56,7 @@ Error thinBackend(const Config &C, unsigned Task, AddStreamFn AddStream, const FunctionImporter::ImportMapTy &ImportList, const GVSummaryMapTy &DefinedGlobals, MapVector *ModuleMap, + bool CodeGenOnly, const std::vector &CmdArgs = std::vector()); Error finalizeOptimizationRemarks( diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp index 5d9a5cbd18f156..400e34527b6c87 100644 --- a/llvm/lib/LTO/LTO.cpp +++ b/llvm/lib/LTO/LTO.cpp @@ -1474,7 +1474,8 @@ class InProcessThinBackend : public ThinBackendProc { return MOrErr.takeError(); return thinBackend(Conf, Task, AddStream, **MOrErr, CombinedIndex, - ImportList, DefinedGlobals, &ModuleMap); + ImportList, DefinedGlobals, &ModuleMap, + Conf.CodeGenOnly); }; auto ModuleID = BM.getModuleIdentifier(); @@ -1840,45 +1841,49 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache, TimeTraceScopeExit.release(); - std::unique_ptr BackendProc = - ThinLTO.Backend(Conf, ThinLTO.CombinedIndex, ModuleToDefinedGVSummaries, - AddStream, Cache); - auto &ModuleMap = ThinLTO.ModulesToCompile ? *ThinLTO.ModulesToCompile : ThinLTO.ModuleMap; - auto ProcessOneModule = [&](int I) -> Error { -auto &Mod = *(ModuleMap.begin() + I); -// Tasks 0 through ParallelCodeGenParallelismLevel-1 are reserved for -// combined module and parallel code generation partitions. -return BackendProc->start(RegularLTO.ParallelCodeGenParallelismLevel + I, - Mod.second, ImportLists[Mod.first], - ExportLists[Mod.first], ResolvedODR[Mod.first], - ThinLTO.ModuleMap); + auto RunBackends = [&](ThinBackendProc *BackendProcess) -> Error { +auto ProcessOneModule = [&](int I) -> Error { + auto &Mod = *(ModuleMap.begin() + I); + // Tasks 0 through ParallelCodeGenParallelismLevel-1 are reserved for + // combined module and parallel code generation partitions. + return BackendProcess->start( + RegularLTO.ParallelCodeGenParallelismLevel + I, Mod.second, + ImportLists[Mod.first], ExportLists[Mod.first], + ResolvedODR[Mod.first], ThinLTO.ModuleMap); +}; + +if (BackendProcess->getThreadCount() == 1) { + // Process the modules in the order they were provided on the + // command-line. It is important for this codepath to be used for + // WriteIndexesThinBackend, to ensure the emitted LinkedObjectsFile lists + // ThinLTO objects in the same order as the inputs, which otherwise would + // affect the final link order. + for (int I = 0, E = ModuleMap.size(); I != E; ++I) +if (Error E = ProcessOneModule(I)) + return E; +} else { + // When executing in parallel, process largest bitsize modules first to + // improve parallelism, and avoid starving the thread pool near the end. + // This saves about 15 sec on a 36-core machine while link `clang.exe` + // (out of 100 sec). + std::vector ModulesVec; + ModulesVec.reserve(ModuleMap.size()); + for (aut
[clang] [llvm] [CGData][ThinLTO][NFC] Prep for two-codegen rounds (PR #90934)
https://github.com/kyulee-com edited https://github.com/llvm/llvm-project/pull/90934 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [CGData][ThinLTO][NFC] Prep for two-codegen rounds (PR #90934)
https://github.com/kyulee-com edited https://github.com/llvm/llvm-project/pull/90934 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits