[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)
@@ -0,0 +1,159 @@ +//===--- RedundantLookupCheck.cpp - clang-tidy ===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantLookupCheck.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Regex.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::performance { + +static constexpr auto DefaultContainerNameRegex = "set|map"; + +static const llvm::StringRef DefaultLookupMethodNames = +llvm::StringLiteral( // +"at;" +"contains;" +"count;" +"find_as;" +"find;" +// These are tricky, as they take the "key" at different places. +// They sometimes bundle up the key and the value together in a pair. +// "emplace;" +// "insert_or_assign;" +// "insert;" +// "try_emplace;" +) +.drop_back(); // Drops the last semicolon. + +RedundantLookupCheck::RedundantLookupCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + ContainerNameRegex( + Options.get("ContainerNameRegex", DefaultContainerNameRegex)), + LookupMethodNames(utils::options::parseStringList( + Options.get("LookupMethodNames", DefaultLookupMethodNames))) {} + +void RedundantLookupCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "ContainerNameRegex", ContainerNameRegex); + Options.store(Opts, "LookupMethodNames", +utils::options::serializeStringList(LookupMethodNames)); +} + +namespace { +/// Checks if any of the ends of the source range is in a macro expansion. +AST_MATCHER(Expr, hasMacroSourceRange) { + SourceRange R = Node.getSourceRange(); + return R.getBegin().isMacroID() || R.getEnd().isMacroID(); +} +} // namespace + +static constexpr const char *ObjKey = "obj"; +static constexpr const char *LookupKey = "key"; +static constexpr const char *LookupCallKey = "lookup"; +static constexpr const char *EnclosingFnKey = "fn"; + +void RedundantLookupCheck::registerMatchers(MatchFinder *Finder) { + auto MatchesContainerNameRegex = + matchesName(ContainerNameRegex, llvm::Regex::IgnoreCase); + + // Match that the expression is a record type with a name that contains "map" + // or "set". + auto RecordCalledMapOrSet = + expr(ignoringImpCasts(hasType(hasUnqualifiedDesugaredType(recordType( + hasDeclaration(namedDecl(MatchesContainerNameRegex))) + .bind(ObjKey); + + auto SubscriptCall = + cxxOperatorCallExpr(hasOverloadedOperatorName("[]"), argumentCountIs(2), + hasArgument(0, RecordCalledMapOrSet), + hasArgument(1, expr().bind(LookupKey))); + + auto LookupMethodCalls = + cxxMemberCallExpr(on(RecordCalledMapOrSet), argumentCountIs(1), +hasArgument(0, expr().bind(LookupKey)), +callee(cxxMethodDecl(hasAnyName(LookupMethodNames; + + // Match any lookup or subscript calls that are not in a macro expansion. + auto AnyLookup = callExpr(unless(hasMacroSourceRange()), +anyOf(SubscriptCall, LookupMethodCalls)) + .bind(LookupCallKey); + + // We need to collect all lookups in a function to be able to report them in + // batches. + Finder->addMatcher( + functionDecl(hasBody(compoundStmt(forEachDescendant(AnyLookup + .bind(EnclosingFnKey), + this); +} + +/// Hash the container object expr along with the key used for lookup and the +/// enclosing function together. +static unsigned hashLookupEvent(const ASTContext &Ctx, +const FunctionDecl *EnclosingFn, +const Expr *LookupKey, +const Expr *ContainerObject) { + llvm::FoldingSetNodeID ID; + ID.AddPointer(EnclosingFn); + + LookupKey->Profile(ID, Ctx, /*Canonical=*/true, + /*ProfileLambdaExpr=*/true); + ContainerObject->Profile(ID, Ctx, /*Canonical=*/true, + /*ProfileLambdaExpr=*/true); + return ID.ComputeHash(); +} + +void RedundantLookupCheck::check(const MatchFinder::MatchResult &Result) { + SM = Result.SourceManager; + + const auto *EnclosingFn = + Result.Nodes.getNodeAs(EnclosingFnKey); + const auto *LookupCall = Result.Nodes.getNodeAs(LookupCallKey); + const auto *Key = Result.Nodes.getNodeAs(LookupKey); + const auto *ContainerObject = Result.Nodes.getNodeAs(ObjKey);
[clang] [CodeGen] Migrate away from PointerUnion::dyn_cast (NFC) (PR #125456)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Kazu Hirata (kazutakahirata) Changes Note that PointerUnion::dyn_cast has been soft deprecated in PointerUnion.h: // FIXME: Replace the uses of is(), get() and dyn_cast() with //isa, cast and the llvm::dyn_cast Literal migration would result in dyn_cast_if_present (see the definition of PointerUnion::dyn_cast), but this patch uses dyn_cast because we expect E to be nonnull. --- Full diff: https://github.com/llvm/llvm-project/pull/125456.diff 1 Files Affected: - (modified) clang/lib/CodeGen/ItaniumCXXABI.cpp (+2-2) ``diff diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp index 7c463f51f63dc5c..7375a511809b9f8 100644 --- a/clang/lib/CodeGen/ItaniumCXXABI.cpp +++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -2239,8 +2239,8 @@ CGCallee ItaniumCXXABI::getVirtualFunctionPointer(CodeGenFunction &CGF, llvm::Value *ItaniumCXXABI::EmitVirtualDestructorCall( CodeGenFunction &CGF, const CXXDestructorDecl *Dtor, CXXDtorType DtorType, Address This, DeleteOrMemberCallExpr E, llvm::CallBase **CallOrInvoke) { - auto *CE = E.dyn_cast(); - auto *D = E.dyn_cast(); + auto *CE = dyn_cast(E); + auto *D = dyn_cast(E); assert((CE != nullptr) ^ (D != nullptr)); assert(CE == nullptr || CE->arg_begin() == CE->arg_end()); assert(DtorType == Dtor_Deleting || DtorType == Dtor_Complete); `` https://github.com/llvm/llvm-project/pull/125456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CodeGen] Migrate away from PointerUnion::dyn_cast (NFC) (PR #125456)
https://github.com/kazutakahirata created https://github.com/llvm/llvm-project/pull/125456 Note that PointerUnion::dyn_cast has been soft deprecated in PointerUnion.h: // FIXME: Replace the uses of is(), get() and dyn_cast() with //isa, cast and the llvm::dyn_cast Literal migration would result in dyn_cast_if_present (see the definition of PointerUnion::dyn_cast), but this patch uses dyn_cast because we expect E to be nonnull. >From 08923b1f3f9d10bf69cab3c788d7b45ba87b1b84 Mon Sep 17 00:00:00 2001 From: Kazu Hirata Date: Sun, 2 Feb 2025 12:17:01 -0800 Subject: [PATCH] [CodeGen] Migrate away from PointerUnion::dyn_cast (NFC) Note that PointerUnion::dyn_cast has been soft deprecated in PointerUnion.h: // FIXME: Replace the uses of is(), get() and dyn_cast() with //isa, cast and the llvm::dyn_cast Literal migration would result in dyn_cast_if_present (see the definition of PointerUnion::dyn_cast), but this patch uses dyn_cast because we expect E to be nonnull. --- clang/lib/CodeGen/ItaniumCXXABI.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp index 7c463f51f63dc5..7375a511809b9f 100644 --- a/clang/lib/CodeGen/ItaniumCXXABI.cpp +++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -2239,8 +2239,8 @@ CGCallee ItaniumCXXABI::getVirtualFunctionPointer(CodeGenFunction &CGF, llvm::Value *ItaniumCXXABI::EmitVirtualDestructorCall( CodeGenFunction &CGF, const CXXDestructorDecl *Dtor, CXXDtorType DtorType, Address This, DeleteOrMemberCallExpr E, llvm::CallBase **CallOrInvoke) { - auto *CE = E.dyn_cast(); - auto *D = E.dyn_cast(); + auto *CE = dyn_cast(E); + auto *D = dyn_cast(E); assert((CE != nullptr) ^ (D != nullptr)); assert(CE == nullptr || CE->arg_begin() == CE->arg_end()); assert(DtorType == Dtor_Deleting || DtorType == Dtor_Complete); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)
@@ -1654,11 +1654,20 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) { } case DiagnosticIDs::SFINAE_Suppress: + if (DiagnosticsEngine::Level Level = getDiagnostics().getDiagnosticLevel( + DiagInfo.getID(), DiagInfo.getLocation()); + Level == DiagnosticsEngine::Ignored) +return; // Make a copy of this suppressed diagnostic and store it with the // template-deduction information; if (*Info) { -(*Info)->addSuppressedDiagnostic(DiagInfo.getLocation(), - PartialDiagnostic(DiagInfo, Context.getDiagAllocator())); +(*Info)->addSuppressedDiagnostic( +DiagInfo.getLocation(), +PartialDiagnostic(DiagInfo, Context.getDiagAllocator())); +if (!Diags.getDiagnosticIDs()->isNote(DiagID)) + PrintContextStack([Info](SourceLocation Loc, PartialDiagnostic PD) { +(*Info)->addSuppressedDiagnostic(Loc, std::move(PD)); + }); cor3ntin wrote: Did you consider doing that in PrintInstantiationStack directly? ie just modifying `PrintInstantiationStack`'s body such that if we are in a sfinae context we call addSuppressedDiagnostic on that context there, and not have to deal with callbacks outside of this function? https://github.com/llvm/llvm-project/pull/125453 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)
@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase { /// '\#pragma clang attribute push' directives to the given declaration. void AddPragmaAttributes(Scope *S, Decl *D); - void PrintPragmaAttributeInstantiationPoint(); + using DiagFuncRef = cor3ntin wrote: I would like a more specific name, like `InstantiationContextDiagCallback` or something along these lines https://github.com/llvm/llvm-project/pull/125453 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)
@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase { /// '\#pragma clang attribute push' directives to the given declaration. void AddPragmaAttributes(Scope *S, Decl *D); - void PrintPragmaAttributeInstantiationPoint(); + using DiagFuncRef = + llvm::function_ref; + auto getDefaultDiagFunc() { +return [this](SourceLocation Loc, PartialDiagnostic PD) { + DiagnosticBuilder Builder(Diags.Report(Loc, PD.getDiagID())); + PD.Emit(Builder); +}; + } cor3ntin wrote: I think we could / should put that into SemaTemplateInstantiate - and have the function taking a DiagFuncRef have that default parameter defaulted to `{}` https://github.com/llvm/llvm-project/pull/125453 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][bytecode] Refactor Program::createGlobalString (PR #125467)
https://github.com/tbaederr created https://github.com/llvm/llvm-project/pull/125467 Remove unnecesary narrow() calls, rename a variable and initialize the array as a whole instead of each element individually. >From 20c472b39e94f51ed383a265f6c836f2b3276c50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= Date: Sun, 2 Feb 2025 19:10:08 +0100 Subject: [PATCH] [clang][bytecode] Refactor Program::createGlobalString Remove unnecesary narrow() calls, rename a variable and initialize the array as a whole instead of each element individually. --- clang/lib/AST/ByteCode/Program.cpp | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/clang/lib/AST/ByteCode/Program.cpp b/clang/lib/AST/ByteCode/Program.cpp index 1ffe7cd721f11ff..e0b86d46428a266 100644 --- a/clang/lib/AST/ByteCode/Program.cpp +++ b/clang/lib/AST/ByteCode/Program.cpp @@ -35,6 +35,7 @@ const void *Program::getNativePointer(unsigned Idx) { unsigned Program::createGlobalString(const StringLiteral *S, const Expr *Base) { const size_t CharWidth = S->getCharByteWidth(); const size_t BitWidth = CharWidth * Ctx.getCharBit(); + unsigned StringLength = S->getLength(); PrimType CharType; switch (CharWidth) { @@ -55,15 +56,15 @@ unsigned Program::createGlobalString(const StringLiteral *S, const Expr *Base) { Base = S; // Create a descriptor for the string. - Descriptor *Desc = allocateDescriptor(Base, CharType, Descriptor::GlobalMD, -S->getLength() + 1, -/*isConst=*/true, -/*isTemporary=*/false, -/*isMutable=*/false); + Descriptor *Desc = + allocateDescriptor(Base, CharType, Descriptor::GlobalMD, StringLength + 1, + /*isConst=*/true, + /*isTemporary=*/false, + /*isMutable=*/false); // Allocate storage for the string. // The byte length does not include the null terminator. - unsigned I = Globals.size(); + unsigned GlobalIndex = Globals.size(); unsigned Sz = Desc->getAllocSize(); auto *G = new (Allocator, Sz) Global(Ctx.getEvalID(), Desc, /*isStatic=*/true, /*isExtern=*/false); @@ -74,33 +75,32 @@ unsigned Program::createGlobalString(const StringLiteral *S, const Expr *Base) { // Construct the string in storage. const Pointer Ptr(G->block()); - for (unsigned I = 0, N = S->getLength(); I <= N; ++I) { -Pointer Field = Ptr.atIndex(I).narrow(); -const uint32_t CodePoint = I == N ? 0 : S->getCodeUnit(I); + for (unsigned I = 0; I <= StringLength; ++I) { +Pointer Field = Ptr.atIndex(I); +const uint32_t CodePoint = I == StringLength ? 0 : S->getCodeUnit(I); switch (CharType) { case PT_Sint8: { using T = PrimConv::T; Field.deref() = T::from(CodePoint, BitWidth); - Field.initialize(); break; } case PT_Uint16: { using T = PrimConv::T; Field.deref() = T::from(CodePoint, BitWidth); - Field.initialize(); break; } case PT_Uint32: { using T = PrimConv::T; Field.deref() = T::from(CodePoint, BitWidth); - Field.initialize(); break; } default: llvm_unreachable("unsupported character type"); } } - return I; + Ptr.initialize(); + + return GlobalIndex; } Pointer Program::getPtrGlobal(unsigned Idx) const { ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libclang] Migrate away from PointerUnion::dyn_cast (NFC) (PR #125381)
https://github.com/nikic approved this pull request. https://github.com/llvm/llvm-project/pull/125381 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Lex] Migrate away from PointerUnion::dyn_cast (NFC) (PR #125380)
https://github.com/nikic approved this pull request. https://github.com/llvm/llvm-project/pull/125380 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AST] Migrate away from PointerUnion::dyn_cast (NFC) (PR #125379)
https://github.com/nikic approved this pull request. https://github.com/llvm/llvm-project/pull/125379 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][bytecode] Ignore Namespace{Using, Alias}Decls (PR #125387)
https://github.com/tbaederr created https://github.com/llvm/llvm-project/pull/125387 These were missing here and are used in a few libc++ tests. >From 7d77e6cdb6bf651b1b19c48e41c7e182e990e5d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= Date: Sun, 2 Feb 2025 10:07:13 +0100 Subject: [PATCH] [clang][bytecode] Ignore Namespace{Using,Alias}Decls These were missing here and are used in a few libc++ tests. --- clang/lib/AST/ByteCode/Compiler.cpp | 4 ++-- clang/test/AST/ByteCode/literals.cpp | 6 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index f7f4f713c787f25..b9e22ebb7a41a90 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -4991,8 +4991,8 @@ bool Compiler::visitCompoundStmt(const CompoundStmt *S) { template bool Compiler::visitDeclStmt(const DeclStmt *DS) { for (const auto *D : DS->decls()) { -if (isa(D)) +if (isa(D)) continue; const auto *VD = dyn_cast(D); diff --git a/clang/test/AST/ByteCode/literals.cpp b/clang/test/AST/ByteCode/literals.cpp index b75ca2b19a969a9..a80ee7ad84fc748 100644 --- a/clang/test/AST/ByteCode/literals.cpp +++ b/clang/test/AST/ByteCode/literals.cpp @@ -914,12 +914,18 @@ namespace TypeTraits { } #if __cplusplus >= 201402L +namespace SomeNS { + using MyInt = int; +} + constexpr int ignoredDecls() { static_assert(true, ""); struct F { int a; }; enum E { b }; using A = int; typedef int Z; + namespace NewNS = SomeNS; + using NewNS::MyInt; return F{12}.a; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][bytecode] Ignore Namespace{Using, Alias}Decls (PR #125387)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) Changes These were missing here and are used in a few libc++ tests. --- Full diff: https://github.com/llvm/llvm-project/pull/125387.diff 2 Files Affected: - (modified) clang/lib/AST/ByteCode/Compiler.cpp (+2-2) - (modified) clang/test/AST/ByteCode/literals.cpp (+6) ``diff diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index f7f4f713c787f25..b9e22ebb7a41a90 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -4991,8 +4991,8 @@ bool Compiler::visitCompoundStmt(const CompoundStmt *S) { template bool Compiler::visitDeclStmt(const DeclStmt *DS) { for (const auto *D : DS->decls()) { -if (isa(D)) +if (isa(D)) continue; const auto *VD = dyn_cast(D); diff --git a/clang/test/AST/ByteCode/literals.cpp b/clang/test/AST/ByteCode/literals.cpp index b75ca2b19a969a9..a80ee7ad84fc748 100644 --- a/clang/test/AST/ByteCode/literals.cpp +++ b/clang/test/AST/ByteCode/literals.cpp @@ -914,12 +914,18 @@ namespace TypeTraits { } #if __cplusplus >= 201402L +namespace SomeNS { + using MyInt = int; +} + constexpr int ignoredDecls() { static_assert(true, ""); struct F { int a; }; enum E { b }; using A = int; typedef int Z; + namespace NewNS = SomeNS; + using NewNS::MyInt; return F{12}.a; } `` https://github.com/llvm/llvm-project/pull/125387 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] Issue 17812: [Atomic] the access size (8 bytes) exceeds the max lock-free size (4 bytes) for 32-bit (PR #125388)
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/125388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] Issue 17812: [Atomic] the access size (8 bytes) exceeds the max lock-free size (4 bytes) for 32-bit (PR #125388)
llvmbot wrote: @llvm/pr-subscribers-clang Author: None (honeygoyal) Changes ## Description [Atomic] the access size (8 bytes) exceeds the max lock-free size (4 bytes) for 32-bit. - Suppress -Watomic-alignment warnings by not treating them as errors - Added -latomic when sanitizer flag is enabled and processor is 32-bit - Adding the -latomic flag is essential for ensuring that atomic operations required by sanitizers are properly linked on 32-bit AIX systems. - This change addresses linker warnings, enhances build stability, and ensures the reliable functioning of sanitizers, thereby improving the overall quality and robustness of the build process. Driver Test Case (AIX.cpp) - Test Case 1: Sanitizer enabled on 32-bit AIX – -latomic should be added with diagnostic. Test Case 2: Sanitizer disabled on 32-bit AIX – -latomic should not be added. --- Full diff: https://github.com/llvm/llvm-project/pull/125388.diff 3 Files Affected: - (modified) clang/lib/Driver/ToolChains/AIX.cpp (+5-1) - (modified) clang/test/Driver/aix-ld.c (+18) - (modified) compiler-rt/lib/sanitizer_common/CMakeLists.txt (+3) ``diff diff --git a/clang/lib/Driver/ToolChains/AIX.cpp b/clang/lib/Driver/ToolChains/AIX.cpp index 09a8dc2f4fa5dd..493983468b0bcf 100644 --- a/clang/lib/Driver/ToolChains/AIX.cpp +++ b/clang/lib/Driver/ToolChains/AIX.cpp @@ -232,7 +232,7 @@ void aix::Linker::ConstructJob(Compilation &C, const JobAction &JA, // Specify linker input file(s). AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA); - + const SanitizerArgs &Sanitize = ToolChain.getSanitizerArgs(Args); if (D.isUsingLTO()) { assert(!Inputs.empty() && "Must have at least one input."); // Find the first filename InputInfo object. @@ -338,6 +338,10 @@ void aix::Linker::ConstructJob(Compilation &C, const JobAction &JA, CmdArgs.push_back("-lpthread"); } const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath()); + + if (Sanitize.hasAnySanitizer() && IsArch32Bit) { +CmdArgs.push_back("-latomic"); + } C.addCommand(std::make_unique(JA, *this, ResponseFileSupport::None(), Exec, CmdArgs, Inputs, Output)); } diff --git a/clang/test/Driver/aix-ld.c b/clang/test/Driver/aix-ld.c index 7e0f2bf91e06ee..de65597bcf5d91 100644 --- a/clang/test/Driver/aix-ld.c +++ b/clang/test/Driver/aix-ld.c @@ -1120,3 +1120,21 @@ // RUN:-c \ // RUN: | FileCheck --check-prefixes=CHECK-K-UNUSED %s // CHECK-K-UNUSED: clang: warning: -K: 'linker' input unused [-Wunused-command-line-argument] + +// Check No Sanitizer on 32-bit AIX +// RUN: %if target={{.*aix.*}} %{ \ +// RUN: %clang -target powerpc-ibm-aix -m32 %s -### 2>&1 \ +// RUN: | FileCheck -check-prefix=CHECK-LD32-NO-SANITIZER %s \ +// RUN: %} +// %if target={{.*aix.*}} %{ +// CHECK-LD32-NO-SANITIZER-NOT: "-latomic" +// %} + +// Check enable AddressSanitizer on 32-bit AIX +// RUN: %if target={{.*aix.*}} %{ \ +// RUN: %clang -target powerpc-ibm-aix -m32 -fsanitize=address %s -### 2>&1 \ +// RUN: | FileCheck -check-prefix=CHECK-LD32-ASAN %s \ +// RUN: %} +// %if target={{.*aix.*}} %{ +// CHECK-LD32-ASAN: "-latomic" +// %} diff --git a/compiler-rt/lib/sanitizer_common/CMakeLists.txt b/compiler-rt/lib/sanitizer_common/CMakeLists.txt index 09391e4f5f3704..79eeb31c035a28 100644 --- a/compiler-rt/lib/sanitizer_common/CMakeLists.txt +++ b/compiler-rt/lib/sanitizer_common/CMakeLists.txt @@ -239,6 +239,9 @@ append_list_if(SANITIZER_LIMIT_FRAME_SIZE -Wframe-larger-than=570 append_list_if(COMPILER_RT_HAS_WGLOBAL_CONSTRUCTORS_FLAG -Wglobal-constructors SANITIZER_CFLAGS) +# Suppress -Watomic-alignment warnings by not treating them as errors +list(APPEND SANITIZER_CFLAGS "-Wno-error=atomic-alignment") + if(APPLE) set(OS_OPTION OS ${SANITIZER_COMMON_SUPPORTED_OS}) endif() `` https://github.com/llvm/llvm-project/pull/125388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] Issue 17812: [Atomic] the access size (8 bytes) exceeds the max lock-free size (4 bytes) for 32-bit (PR #125388)
llvmbot wrote: @llvm/pr-subscribers-backend-powerpc Author: None (honeygoyal) Changes ## Description [Atomic] the access size (8 bytes) exceeds the max lock-free size (4 bytes) for 32-bit. - Suppress -Watomic-alignment warnings by not treating them as errors - Added -latomic when sanitizer flag is enabled and processor is 32-bit - Adding the -latomic flag is essential for ensuring that atomic operations required by sanitizers are properly linked on 32-bit AIX systems. - This change addresses linker warnings, enhances build stability, and ensures the reliable functioning of sanitizers, thereby improving the overall quality and robustness of the build process. Driver Test Case (AIX.cpp) - Test Case 1: Sanitizer enabled on 32-bit AIX – -latomic should be added with diagnostic. Test Case 2: Sanitizer disabled on 32-bit AIX – -latomic should not be added. --- Full diff: https://github.com/llvm/llvm-project/pull/125388.diff 3 Files Affected: - (modified) clang/lib/Driver/ToolChains/AIX.cpp (+5-1) - (modified) clang/test/Driver/aix-ld.c (+18) - (modified) compiler-rt/lib/sanitizer_common/CMakeLists.txt (+3) ``diff diff --git a/clang/lib/Driver/ToolChains/AIX.cpp b/clang/lib/Driver/ToolChains/AIX.cpp index 09a8dc2f4fa5dd8..493983468b0bcf9 100644 --- a/clang/lib/Driver/ToolChains/AIX.cpp +++ b/clang/lib/Driver/ToolChains/AIX.cpp @@ -232,7 +232,7 @@ void aix::Linker::ConstructJob(Compilation &C, const JobAction &JA, // Specify linker input file(s). AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA); - + const SanitizerArgs &Sanitize = ToolChain.getSanitizerArgs(Args); if (D.isUsingLTO()) { assert(!Inputs.empty() && "Must have at least one input."); // Find the first filename InputInfo object. @@ -338,6 +338,10 @@ void aix::Linker::ConstructJob(Compilation &C, const JobAction &JA, CmdArgs.push_back("-lpthread"); } const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath()); + + if (Sanitize.hasAnySanitizer() && IsArch32Bit) { +CmdArgs.push_back("-latomic"); + } C.addCommand(std::make_unique(JA, *this, ResponseFileSupport::None(), Exec, CmdArgs, Inputs, Output)); } diff --git a/clang/test/Driver/aix-ld.c b/clang/test/Driver/aix-ld.c index 7e0f2bf91e06ee5..de65597bcf5d914 100644 --- a/clang/test/Driver/aix-ld.c +++ b/clang/test/Driver/aix-ld.c @@ -1120,3 +1120,21 @@ // RUN:-c \ // RUN: | FileCheck --check-prefixes=CHECK-K-UNUSED %s // CHECK-K-UNUSED: clang: warning: -K: 'linker' input unused [-Wunused-command-line-argument] + +// Check No Sanitizer on 32-bit AIX +// RUN: %if target={{.*aix.*}} %{ \ +// RUN: %clang -target powerpc-ibm-aix -m32 %s -### 2>&1 \ +// RUN: | FileCheck -check-prefix=CHECK-LD32-NO-SANITIZER %s \ +// RUN: %} +// %if target={{.*aix.*}} %{ +// CHECK-LD32-NO-SANITIZER-NOT: "-latomic" +// %} + +// Check enable AddressSanitizer on 32-bit AIX +// RUN: %if target={{.*aix.*}} %{ \ +// RUN: %clang -target powerpc-ibm-aix -m32 -fsanitize=address %s -### 2>&1 \ +// RUN: | FileCheck -check-prefix=CHECK-LD32-ASAN %s \ +// RUN: %} +// %if target={{.*aix.*}} %{ +// CHECK-LD32-ASAN: "-latomic" +// %} diff --git a/compiler-rt/lib/sanitizer_common/CMakeLists.txt b/compiler-rt/lib/sanitizer_common/CMakeLists.txt index 09391e4f5f3704b..79eeb31c035a282 100644 --- a/compiler-rt/lib/sanitizer_common/CMakeLists.txt +++ b/compiler-rt/lib/sanitizer_common/CMakeLists.txt @@ -239,6 +239,9 @@ append_list_if(SANITIZER_LIMIT_FRAME_SIZE -Wframe-larger-than=570 append_list_if(COMPILER_RT_HAS_WGLOBAL_CONSTRUCTORS_FLAG -Wglobal-constructors SANITIZER_CFLAGS) +# Suppress -Watomic-alignment warnings by not treating them as errors +list(APPEND SANITIZER_CFLAGS "-Wno-error=atomic-alignment") + if(APPLE) set(OS_OPTION OS ${SANITIZER_COMMON_SUPPORTED_OS}) endif() `` https://github.com/llvm/llvm-project/pull/125388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] Issue 17812: [Atomic] the access size (8 bytes) exceeds the max lock-free size (4 bytes) for 32-bit (PR #125388)
https://github.com/honeygoyal created https://github.com/llvm/llvm-project/pull/125388 ## Description [Atomic] the access size (8 bytes) exceeds the max lock-free size (4 bytes) for 32-bit. - Suppress -Watomic-alignment warnings by not treating them as errors - Added -latomic when sanitizer flag is enabled and processor is 32-bit - Adding the -latomic flag is essential for ensuring that atomic operations required by sanitizers are properly linked on 32-bit AIX systems. - This change addresses linker warnings, enhances build stability, and ensures the reliable functioning of sanitizers, thereby improving the overall quality and robustness of the build process. Driver Test Case (AIX.cpp) - Test Case 1: Sanitizer enabled on 32-bit AIX – -latomic should be added with diagnostic. Test Case 2: Sanitizer disabled on 32-bit AIX – -latomic should not be added. >From f9d8e7f9c0df6beb8b4a63a01ebbc3b3ab93d091 Mon Sep 17 00:00:00 2001 From: Honey Goyal Date: Sun, 2 Feb 2025 14:27:01 +0530 Subject: [PATCH 1/3] Test Cases for adding -latomic (the access size (8 bytes) exceeds the max lock-free size --- clang/test/Driver/aix-ld.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/clang/test/Driver/aix-ld.c b/clang/test/Driver/aix-ld.c index 7e0f2bf91e06ee5..de65597bcf5d914 100644 --- a/clang/test/Driver/aix-ld.c +++ b/clang/test/Driver/aix-ld.c @@ -1120,3 +1120,21 @@ // RUN:-c \ // RUN: | FileCheck --check-prefixes=CHECK-K-UNUSED %s // CHECK-K-UNUSED: clang: warning: -K: 'linker' input unused [-Wunused-command-line-argument] + +// Check No Sanitizer on 32-bit AIX +// RUN: %if target={{.*aix.*}} %{ \ +// RUN: %clang -target powerpc-ibm-aix -m32 %s -### 2>&1 \ +// RUN: | FileCheck -check-prefix=CHECK-LD32-NO-SANITIZER %s \ +// RUN: %} +// %if target={{.*aix.*}} %{ +// CHECK-LD32-NO-SANITIZER-NOT: "-latomic" +// %} + +// Check enable AddressSanitizer on 32-bit AIX +// RUN: %if target={{.*aix.*}} %{ \ +// RUN: %clang -target powerpc-ibm-aix -m32 -fsanitize=address %s -### 2>&1 \ +// RUN: | FileCheck -check-prefix=CHECK-LD32-ASAN %s \ +// RUN: %} +// %if target={{.*aix.*}} %{ +// CHECK-LD32-ASAN: "-latomic" +// %} >From 1b7439e43798c8a1663e9cb72d61a1625b391cec Mon Sep 17 00:00:00 2001 From: Honey Goyal Date: Sun, 2 Feb 2025 14:37:11 +0530 Subject: [PATCH 2/3] Added -latomic when sanitizer flag is enabled and processor is 32-bit --- clang/lib/Driver/ToolChains/AIX.cpp | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/AIX.cpp b/clang/lib/Driver/ToolChains/AIX.cpp index 09a8dc2f4fa5dd8..493983468b0bcf9 100644 --- a/clang/lib/Driver/ToolChains/AIX.cpp +++ b/clang/lib/Driver/ToolChains/AIX.cpp @@ -232,7 +232,7 @@ void aix::Linker::ConstructJob(Compilation &C, const JobAction &JA, // Specify linker input file(s). AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA); - + const SanitizerArgs &Sanitize = ToolChain.getSanitizerArgs(Args); if (D.isUsingLTO()) { assert(!Inputs.empty() && "Must have at least one input."); // Find the first filename InputInfo object. @@ -338,6 +338,10 @@ void aix::Linker::ConstructJob(Compilation &C, const JobAction &JA, CmdArgs.push_back("-lpthread"); } const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath()); + + if (Sanitize.hasAnySanitizer() && IsArch32Bit) { +CmdArgs.push_back("-latomic"); + } C.addCommand(std::make_unique(JA, *this, ResponseFileSupport::None(), Exec, CmdArgs, Inputs, Output)); } >From 84d973a841ce1069c941b26f7466330e813797e3 Mon Sep 17 00:00:00 2001 From: Honey Goyal Date: Sun, 2 Feb 2025 14:38:49 +0530 Subject: [PATCH 3/3] Suppress -Watomic-alignment warnings by not treating them as errors --- compiler-rt/lib/sanitizer_common/CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler-rt/lib/sanitizer_common/CMakeLists.txt b/compiler-rt/lib/sanitizer_common/CMakeLists.txt index 09391e4f5f3704b..79eeb31c035a282 100644 --- a/compiler-rt/lib/sanitizer_common/CMakeLists.txt +++ b/compiler-rt/lib/sanitizer_common/CMakeLists.txt @@ -239,6 +239,9 @@ append_list_if(SANITIZER_LIMIT_FRAME_SIZE -Wframe-larger-than=570 append_list_if(COMPILER_RT_HAS_WGLOBAL_CONSTRUCTORS_FLAG -Wglobal-constructors SANITIZER_CFLAGS) +# Suppress -Watomic-alignment warnings by not treating them as errors +list(APPEND SANITIZER_CFLAGS "-Wno-error=atomic-alignment") + if(APPLE) set(OS_OPTION OS ${SANITIZER_COMMON_SUPPORTED_OS}) endif() ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Hanlde qualified type name for `QualifierAlignment` (PR #125327)
https://github.com/owenca updated https://github.com/llvm/llvm-project/pull/125327 >From af3d964d74634f0dd9f7216572c1d852b7a549dc Mon Sep 17 00:00:00 2001 From: Owen Pan Date: Fri, 31 Jan 2025 18:32:33 -0800 Subject: [PATCH 1/2] [clang-format] Hanlde qualified type names Fixes #125178. --- clang/lib/Format/QualifierAlignmentFixer.cpp | 8 +++- clang/unittests/Format/QualifierFixerTest.cpp | 12 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/clang/lib/Format/QualifierAlignmentFixer.cpp b/clang/lib/Format/QualifierAlignmentFixer.cpp index 21fb5074b4928f..f777ac2a89f755 100644 --- a/clang/lib/Format/QualifierAlignmentFixer.cpp +++ b/clang/lib/Format/QualifierAlignmentFixer.cpp @@ -132,8 +132,10 @@ static void rotateTokens(const SourceManager &SourceMgr, // Then move through the other tokens. auto *Tok = Begin; while (Tok != End) { -if (!NewText.empty() && !endsWithSpace(NewText)) +if (!NewText.empty() && !endsWithSpace(NewText) && +Tok->isNot(tok::coloncolon)) { NewText += " "; +} NewText += Tok->TokenText; Tok = Tok->Next; @@ -412,6 +414,10 @@ const FormatToken *LeftRightQualifierAlignmentFixer::analyzeLeft( // The case `const long long volatile int` -> `const volatile long long int` // The case `long volatile long int const` -> `const volatile long long int` if (TypeToken->isTypeName(LangOpts)) { +if (const auto *Prev = TypeToken->getPreviousNonComment(); +Prev && Prev->endsSequence(tok::coloncolon, tok::identifier)) { + TypeToken = Prev->getPreviousNonComment(); +} const FormatToken *LastSimpleTypeSpecifier = TypeToken; while (isConfiguredQualifierOrType( LastSimpleTypeSpecifier->getPreviousNonComment(), diff --git a/clang/unittests/Format/QualifierFixerTest.cpp b/clang/unittests/Format/QualifierFixerTest.cpp index 129828b0d187a9..530587bbf2ca74 100644 --- a/clang/unittests/Format/QualifierFixerTest.cpp +++ b/clang/unittests/Format/QualifierFixerTest.cpp @@ -1291,6 +1291,18 @@ TEST_F(QualifierFixerTest, WithCpp11Attribute) { "[[maybe_unused]] constexpr static int A", Style); } +TEST_F(QualifierFixerTest, WithQualifiedTypeName) { + auto Style = getLLVMStyle(); + Style.QualifierAlignment = FormatStyle::QAS_Custom; + Style.QualifierOrder = {"constexpr", "type", "const"}; + + verifyFormat("constexpr std::int64_t x{123};", + "std::int64_t constexpr x{123};", Style); + + Style.TypeNames.push_back("bar"); + verifyFormat("constexpr foo::bar x{12};", "foo::bar constexpr x{12};", Style); +} + TEST_F(QualifierFixerTest, DisableRegions) { FormatStyle Style = getLLVMStyle(); Style.QualifierAlignment = FormatStyle::QAS_Custom; >From 34653886f25ef3120784d3a5e6794c0bb158952d Mon Sep 17 00:00:00 2001 From: Owen Pan Date: Sun, 2 Feb 2025 01:20:45 -0800 Subject: [PATCH 2/2] Handle ::type and ::foo::type --- clang/lib/Format/QualifierAlignmentFixer.cpp | 11 --- clang/unittests/Format/QualifierFixerTest.cpp | 3 +++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/clang/lib/Format/QualifierAlignmentFixer.cpp b/clang/lib/Format/QualifierAlignmentFixer.cpp index f777ac2a89f755..edef95f965edb7 100644 --- a/clang/lib/Format/QualifierAlignmentFixer.cpp +++ b/clang/lib/Format/QualifierAlignmentFixer.cpp @@ -414,9 +414,14 @@ const FormatToken *LeftRightQualifierAlignmentFixer::analyzeLeft( // The case `const long long volatile int` -> `const volatile long long int` // The case `long volatile long int const` -> `const volatile long long int` if (TypeToken->isTypeName(LangOpts)) { -if (const auto *Prev = TypeToken->getPreviousNonComment(); -Prev && Prev->endsSequence(tok::coloncolon, tok::identifier)) { - TypeToken = Prev->getPreviousNonComment(); +for (const auto *Prev = TypeToken->getPreviousNonComment(); + Prev && Prev->is(tok::coloncolon); + Prev = Prev->getPreviousNonComment()) { + TypeToken = Prev; + Prev = Prev->getPreviousNonComment(); + if (!(Prev && Prev->is(tok::identifier))) +break; + TypeToken = Prev; } const FormatToken *LastSimpleTypeSpecifier = TypeToken; while (isConfiguredQualifierOrType( diff --git a/clang/unittests/Format/QualifierFixerTest.cpp b/clang/unittests/Format/QualifierFixerTest.cpp index 530587bbf2ca74..3eae39f267c3e0 100644 --- a/clang/unittests/Format/QualifierFixerTest.cpp +++ b/clang/unittests/Format/QualifierFixerTest.cpp @@ -1296,8 +1296,11 @@ TEST_F(QualifierFixerTest, WithQualifiedTypeName) { Style.QualifierAlignment = FormatStyle::QAS_Custom; Style.QualifierOrder = {"constexpr", "type", "const"}; + verifyFormat("constexpr ::int64_t x{1};", "::int64_t constexpr x{1};", Style); verifyFormat("constexpr std::int64_t x{123};", "std::int64_t constexpr x{123};", Style); + verifyFormat("constexpr ::std::int64_t x{123};", + "::std::int64_t constexpr x{123};",
[clang] [llvm] [MC/DC] Nested decision (PR #125403)
https://github.com/chapuni created https://github.com/llvm/llvm-project/pull/125403 None >From a6d4be0e4b05b411c7160385485cfed0f173965c Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Sun, 2 Feb 2025 21:55:43 +0900 Subject: [PATCH 1/9] [MC/DC] Update CoverageMapping tests To resolve the error, rename mcdc-error-nests.cpp -> mcdc-nested-expr.cpp at first. - `func_condop` A corner case that contains close decisions. - `func_expect` Uses `__builtin_expect`. (#124565) - `func_lnot` Contains logical not(s) `!` among MC/DC binary operators. (#124563) mcdc-single-cond.cpp is for #95336. --- .../test/CoverageMapping/mcdc-error-nests.cpp | 10 -- .../test/CoverageMapping/mcdc-nested-expr.cpp | 34 +++ .../test/CoverageMapping/mcdc-single-cond.cpp | 97 +++ 3 files changed, 131 insertions(+), 10 deletions(-) delete mode 100644 clang/test/CoverageMapping/mcdc-error-nests.cpp create mode 100644 clang/test/CoverageMapping/mcdc-nested-expr.cpp create mode 100644 clang/test/CoverageMapping/mcdc-single-cond.cpp diff --git a/clang/test/CoverageMapping/mcdc-error-nests.cpp b/clang/test/CoverageMapping/mcdc-error-nests.cpp deleted file mode 100644 index 3add2b9ccd3fb3..00 --- a/clang/test/CoverageMapping/mcdc-error-nests.cpp +++ /dev/null @@ -1,10 +0,0 @@ -// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s 2>&1| FileCheck %s - -// "Split-nest" -- boolean expressions within boolean expressions. -extern bool bar(bool); -bool func_split_nest(bool a, bool b, bool c, bool d, bool e, bool f, bool g) { - bool res = a && b && c && bar(d && e) && f && g; - return bar(res); -} - -// CHECK: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression. diff --git a/clang/test/CoverageMapping/mcdc-nested-expr.cpp b/clang/test/CoverageMapping/mcdc-nested-expr.cpp new file mode 100644 index 00..bb82873e3b600d --- /dev/null +++ b/clang/test/CoverageMapping/mcdc-nested-expr.cpp @@ -0,0 +1,34 @@ +// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s 2> %t.stderr.txt | FileCheck %s +// RUN: FileCheck %s --check-prefix=WARN < %t.stderr.txt + +// "Split-nest" -- boolean expressions within boolean expressions. +extern bool bar(bool); +// CHECK: func_split_nest{{.*}}: +bool func_split_nest(bool a, bool b, bool c, bool d, bool e, bool f, bool g) { + // WARN: :[[@LINE+1]]:14: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression. + bool res = a && b && c && bar(d && e) && f && g; + return bar(res); +} + +// The inner expr begins with the same Loc as the outer expr +// CHECK: func_condop{{.*}}: +bool func_condop(bool a, bool b, bool c) { + // WARN: :[[@LINE+1]]:10: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression. + return (a && b ? true : false) && c; +} + +// __builtin_expect +// Treated as parentheses. +// CHECK: func_expect{{.*}}: +bool func_expect(bool a, bool b, bool c) { + // WARN: :[[@LINE+1]]:10: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression. + return a || __builtin_expect(b && c, true); +} + +// LNot among BinOp(s) +// Doesn't split exprs. +// CHECK: func_lnot{{.*}}: +bool func_lnot(bool a, bool b, bool c, bool d) { + // WARN: :[[@LINE+1]]:10: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression. + return !(a || b) && !(c && d); +} diff --git a/clang/test/CoverageMapping/mcdc-single-cond.cpp b/clang/test/CoverageMapping/mcdc-single-cond.cpp new file mode 100644 index 00..b1eeea879e5217 --- /dev/null +++ b/clang/test/CoverageMapping/mcdc-single-cond.cpp @@ -0,0 +1,97 @@ +// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -disable-llvm-passes -emit-llvm -o %t2.ll %s | FileCheck %s --check-prefixes=MM,MM2 +// RUN: FileCheck %s --check-prefixes=LL,LL2 < %t2.ll + +// LL: define{{.+}}func_cond{{.+}}( +// MM: func_cond{{.*}}: +int func_cond(bool a, bool b) { + // %mcdc.addr* are emitted by static order. + // LL: %[[MA:mcdc.addr.*]] = alloca i32, align 4 + // LL: call void @llvm.instrprof.mcdc.parameters(ptr @[[PROFN:.+]], i64 [[#H:]], i32 [[#BS:]]) + int count = 0; + if (a) +// NB=2 Single cond +// MM2-NOT: Decision +++count; + if (a ? true : false) +// NB=2,2 Wider decision comes first. +// MA2 has C:2 +// MA3 has C:1 +++count; + if (a && b ? true : false) +// NB=2,3 Wider decision comes first. +// MM2: Decision,File 0, [[@LINE-2]]:7 -> [[#L:@LINE-2]]:13 = M:[[#I:3]], C:2 +// MM: Branch,File 0, [[#L]]:7 -> [[#L]]:8 = #6, (#0 - #6) [1,2,0] +
[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)
@@ -3492,10 +3492,13 @@ VarDecl *BindingDecl::getHoldingVar() const { return VD; } -llvm::ArrayRef BindingDecl::getBindingPackExprs() const { +llvm::ArrayRef BindingDecl::getBindingPackDecls() const { assert(Binding && "expecting a pack expr"); - auto *RP = cast(Binding); - return RP->getExprs(); + auto *FP = cast(Binding); + ValueDecl *const *First = FP->getNumExpansions() > 0 ? FP->begin() : nullptr; + assert((!First || isa(*First)) && "expecting a BindingDecl"); + return llvm::ArrayRef((BindingDecl *const *)First, + FP->getNumExpansions()); cor3ntin wrote: ```suggestion auto *FP = cast(Binding); if(FP->getNumExpansions() == 0) return {}; return llvm::ArrayRef(cast(FP->begin()), FP->getNumExpansions()); ``` I haven't tested but something like that looks simpler (the cast does the assert, and if there is nothing to expand we can as well return early) https://github.com/llvm/llvm-project/pull/125394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)
https://github.com/cor3ntin commented: Thanks for doing that, this is neat https://github.com/llvm/llvm-project/pull/125394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [MC/DC] Update CoverageMapping tests (PR #125404)
https://github.com/chapuni created https://github.com/llvm/llvm-project/pull/125404 To resolve the error, rename mcdc-error-nests.cpp -> mcdc-nested-expr.cpp at first. - `func_condop` A corner case that contains close decisions. - `func_expect` Uses `__builtin_expect`. (#124565) - `func_lnot` Contains logical not(s) `!` among MC/DC binary operators. (#124563) mcdc-single-cond.cpp is for #95336 . >From a6d4be0e4b05b411c7160385485cfed0f173965c Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Sun, 2 Feb 2025 21:55:43 +0900 Subject: [PATCH] [MC/DC] Update CoverageMapping tests To resolve the error, rename mcdc-error-nests.cpp -> mcdc-nested-expr.cpp at first. - `func_condop` A corner case that contains close decisions. - `func_expect` Uses `__builtin_expect`. (#124565) - `func_lnot` Contains logical not(s) `!` among MC/DC binary operators. (#124563) mcdc-single-cond.cpp is for #95336. --- .../test/CoverageMapping/mcdc-error-nests.cpp | 10 -- .../test/CoverageMapping/mcdc-nested-expr.cpp | 34 +++ .../test/CoverageMapping/mcdc-single-cond.cpp | 97 +++ 3 files changed, 131 insertions(+), 10 deletions(-) delete mode 100644 clang/test/CoverageMapping/mcdc-error-nests.cpp create mode 100644 clang/test/CoverageMapping/mcdc-nested-expr.cpp create mode 100644 clang/test/CoverageMapping/mcdc-single-cond.cpp diff --git a/clang/test/CoverageMapping/mcdc-error-nests.cpp b/clang/test/CoverageMapping/mcdc-error-nests.cpp deleted file mode 100644 index 3add2b9ccd3fb3d..000 --- a/clang/test/CoverageMapping/mcdc-error-nests.cpp +++ /dev/null @@ -1,10 +0,0 @@ -// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s 2>&1| FileCheck %s - -// "Split-nest" -- boolean expressions within boolean expressions. -extern bool bar(bool); -bool func_split_nest(bool a, bool b, bool c, bool d, bool e, bool f, bool g) { - bool res = a && b && c && bar(d && e) && f && g; - return bar(res); -} - -// CHECK: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression. diff --git a/clang/test/CoverageMapping/mcdc-nested-expr.cpp b/clang/test/CoverageMapping/mcdc-nested-expr.cpp new file mode 100644 index 000..bb82873e3b600d0 --- /dev/null +++ b/clang/test/CoverageMapping/mcdc-nested-expr.cpp @@ -0,0 +1,34 @@ +// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s 2> %t.stderr.txt | FileCheck %s +// RUN: FileCheck %s --check-prefix=WARN < %t.stderr.txt + +// "Split-nest" -- boolean expressions within boolean expressions. +extern bool bar(bool); +// CHECK: func_split_nest{{.*}}: +bool func_split_nest(bool a, bool b, bool c, bool d, bool e, bool f, bool g) { + // WARN: :[[@LINE+1]]:14: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression. + bool res = a && b && c && bar(d && e) && f && g; + return bar(res); +} + +// The inner expr begins with the same Loc as the outer expr +// CHECK: func_condop{{.*}}: +bool func_condop(bool a, bool b, bool c) { + // WARN: :[[@LINE+1]]:10: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression. + return (a && b ? true : false) && c; +} + +// __builtin_expect +// Treated as parentheses. +// CHECK: func_expect{{.*}}: +bool func_expect(bool a, bool b, bool c) { + // WARN: :[[@LINE+1]]:10: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression. + return a || __builtin_expect(b && c, true); +} + +// LNot among BinOp(s) +// Doesn't split exprs. +// CHECK: func_lnot{{.*}}: +bool func_lnot(bool a, bool b, bool c, bool d) { + // WARN: :[[@LINE+1]]:10: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression. + return !(a || b) && !(c && d); +} diff --git a/clang/test/CoverageMapping/mcdc-single-cond.cpp b/clang/test/CoverageMapping/mcdc-single-cond.cpp new file mode 100644 index 000..b1eeea879e52176 --- /dev/null +++ b/clang/test/CoverageMapping/mcdc-single-cond.cpp @@ -0,0 +1,97 @@ +// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -disable-llvm-passes -emit-llvm -o %t2.ll %s | FileCheck %s --check-prefixes=MM,MM2 +// RUN: FileCheck %s --check-prefixes=LL,LL2 < %t2.ll + +// LL: define{{.+}}func_cond{{.+}}( +// MM: func_cond{{.*}}: +int func_cond(bool a, bool b) { + // %mcdc.addr* are emitted by static order. + // LL: %[[MA:mcdc.addr.*]] = alloca i32, align 4 + // LL: call void @llvm.instrprof.mcdc.parameters(ptr @[[PROFN:.+]], i64 [[#H:]], i32 [[#BS:]]) + int count = 0; + if (a) +// NB=2 Single cond +// MM2-NOT: Decision +++count; + if (a ? true : false) +// NB=2,2
[clang] [MC/DC] Update CoverageMapping tests (PR #125404)
llvmbot wrote: @llvm/pr-subscribers-clang Author: NAKAMURA Takumi (chapuni) Changes To resolve the error, rename mcdc-error-nests.cpp -> mcdc-nested-expr.cpp at first. - `func_condop` A corner case that contains close decisions. - `func_expect` Uses `__builtin_expect`. (#124565) - `func_lnot` Contains logical not(s) `!` among MC/DC binary operators. (#124563) mcdc-single-cond.cpp is for #95336 . --- Full diff: https://github.com/llvm/llvm-project/pull/125404.diff 3 Files Affected: - (removed) clang/test/CoverageMapping/mcdc-error-nests.cpp (-10) - (added) clang/test/CoverageMapping/mcdc-nested-expr.cpp (+34) - (added) clang/test/CoverageMapping/mcdc-single-cond.cpp (+97) ``diff diff --git a/clang/test/CoverageMapping/mcdc-error-nests.cpp b/clang/test/CoverageMapping/mcdc-error-nests.cpp deleted file mode 100644 index 3add2b9ccd3fb3d..000 --- a/clang/test/CoverageMapping/mcdc-error-nests.cpp +++ /dev/null @@ -1,10 +0,0 @@ -// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s 2>&1| FileCheck %s - -// "Split-nest" -- boolean expressions within boolean expressions. -extern bool bar(bool); -bool func_split_nest(bool a, bool b, bool c, bool d, bool e, bool f, bool g) { - bool res = a && b && c && bar(d && e) && f && g; - return bar(res); -} - -// CHECK: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression. diff --git a/clang/test/CoverageMapping/mcdc-nested-expr.cpp b/clang/test/CoverageMapping/mcdc-nested-expr.cpp new file mode 100644 index 000..bb82873e3b600d0 --- /dev/null +++ b/clang/test/CoverageMapping/mcdc-nested-expr.cpp @@ -0,0 +1,34 @@ +// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s 2> %t.stderr.txt | FileCheck %s +// RUN: FileCheck %s --check-prefix=WARN < %t.stderr.txt + +// "Split-nest" -- boolean expressions within boolean expressions. +extern bool bar(bool); +// CHECK: func_split_nest{{.*}}: +bool func_split_nest(bool a, bool b, bool c, bool d, bool e, bool f, bool g) { + // WARN: :[[@LINE+1]]:14: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression. + bool res = a && b && c && bar(d && e) && f && g; + return bar(res); +} + +// The inner expr begins with the same Loc as the outer expr +// CHECK: func_condop{{.*}}: +bool func_condop(bool a, bool b, bool c) { + // WARN: :[[@LINE+1]]:10: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression. + return (a && b ? true : false) && c; +} + +// __builtin_expect +// Treated as parentheses. +// CHECK: func_expect{{.*}}: +bool func_expect(bool a, bool b, bool c) { + // WARN: :[[@LINE+1]]:10: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression. + return a || __builtin_expect(b && c, true); +} + +// LNot among BinOp(s) +// Doesn't split exprs. +// CHECK: func_lnot{{.*}}: +bool func_lnot(bool a, bool b, bool c, bool d) { + // WARN: :[[@LINE+1]]:10: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression. + return !(a || b) && !(c && d); +} diff --git a/clang/test/CoverageMapping/mcdc-single-cond.cpp b/clang/test/CoverageMapping/mcdc-single-cond.cpp new file mode 100644 index 000..b1eeea879e52176 --- /dev/null +++ b/clang/test/CoverageMapping/mcdc-single-cond.cpp @@ -0,0 +1,97 @@ +// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -disable-llvm-passes -emit-llvm -o %t2.ll %s | FileCheck %s --check-prefixes=MM,MM2 +// RUN: FileCheck %s --check-prefixes=LL,LL2 < %t2.ll + +// LL: define{{.+}}func_cond{{.+}}( +// MM: func_cond{{.*}}: +int func_cond(bool a, bool b) { + // %mcdc.addr* are emitted by static order. + // LL: %[[MA:mcdc.addr.*]] = alloca i32, align 4 + // LL: call void @llvm.instrprof.mcdc.parameters(ptr @[[PROFN:.+]], i64 [[#H:]], i32 [[#BS:]]) + int count = 0; + if (a) +// NB=2 Single cond +// MM2-NOT: Decision +++count; + if (a ? true : false) +// NB=2,2 Wider decision comes first. +// MA2 has C:2 +// MA3 has C:1 +++count; + if (a && b ? true : false) +// NB=2,3 Wider decision comes first. +// MM2: Decision,File 0, [[@LINE-2]]:7 -> [[#L:@LINE-2]]:13 = M:[[#I:3]], C:2 +// MM: Branch,File 0, [[#L]]:7 -> [[#L]]:8 = #6, (#0 - #6) [1,2,0] +// MM: Branch,File 0, [[#L]]:12 -> [[#L]]:13 = #7, (#6 - #7) [2,0,0] +// LL: store i32 0, ptr %[[MA]], align 4 +// LL: = load i32, ptr %[[MA]], align 4 +// LL: store i32 %{{.+}}, ptr %[[MA]], align 4 +// LL: = load i32, ptr %[[MA]], align 4 +// LL: store i32 %{{.+}}, ptr %[[MA]], align 4 +// L
[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)
https://github.com/cor3ntin edited https://github.com/llvm/llvm-project/pull/125394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [MC/DC] Make covmap tolerant of nested Decisions (PR #125407)
https://github.com/chapuni created https://github.com/llvm/llvm-project/pull/125407 CoverageMappingWriter reorders `Region`s by `endLoc DESC` to prioritize wider `Decision` with the same `startLoc`. In `llvm-cov`, tweak seeking Decisions by reversal order to find smaller Decision first. >From 06d0d51dce35916ebabcbb219c2d868df375e601 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Sun, 2 Feb 2025 21:58:41 +0900 Subject: [PATCH] [MC/DC] Make covmap tolerant of nested Decisions CoverageMappingWriter reorders `Region`s by `endLoc DESC` to prioritize wider `Decision` with the same `startLoc`. In `llvm-cov`, tweak seeking Decisions by reversal order to find smaller Decision first. --- clang/test/CoverageMapping/ctor.cpp| 4 ++-- clang/test/CoverageMapping/includehell.cpp | 18 +- clang/test/CoverageMapping/macros.c| 6 +++--- .../ProfileData/Coverage/CoverageMapping.cpp | 4 ++-- .../Coverage/CoverageMappingWriter.cpp | 9 - 5 files changed, 24 insertions(+), 17 deletions(-) diff --git a/clang/test/CoverageMapping/ctor.cpp b/clang/test/CoverageMapping/ctor.cpp index 1cf12cd738c2cc..191f73f5693e19 100644 --- a/clang/test/CoverageMapping/ctor.cpp +++ b/clang/test/CoverageMapping/ctor.cpp @@ -11,8 +11,8 @@ class B: public A { int c; B(int x, int y) : A(x), // CHECK: File 0, [[@LINE]]:7 -> [[@LINE]]:11 = #0 -// CHECK-NEXT: File 0, [[@LINE+1]]:7 -> [[@LINE+1]]:13 = #0 -b(x == 0? 1: 2),// CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:19 = #0 +// CHECK-NEXT: File 0, [[@LINE+1]]:7 -> [[@LINE+1]]:19 = #0 +b(x == 0? 1: 2),// CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:13 = #0 // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:7 -> [[@LINE-1]]:13 = #1, (#0 - #1) // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:14 -> [[@LINE-2]]:15 = #1 // CHECK-NEXT: File 0, [[@LINE-3]]:15 -> [[@LINE-3]]:16 = #1 diff --git a/clang/test/CoverageMapping/includehell.cpp b/clang/test/CoverageMapping/includehell.cpp index 09e03e77799d56..3c67672fabc0fc 100644 --- a/clang/test/CoverageMapping/includehell.cpp +++ b/clang/test/CoverageMapping/includehell.cpp @@ -28,14 +28,14 @@ int main() { // CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 6:12 -> 6:35 = #0 // CHECK-MAIN-NEXT: File [[MAIN]], 6:35 -> 10:33 = #1 // CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 8:14 -> 8:29 = #1 -// CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 10:12 -> 10:33 = #1 +// CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 10:12 -> 10:33 = #0 // CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 12:12 -> 12:35 = #0 // CHECK-MAIN-NEXT: File [[MAIN]], 12:35 -> 14:33 = #5 // CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 13:14 -> 13:29 = #5 -// CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 14:12 -> 14:33 = #5 +// CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 14:12 -> 14:33 = #0 // CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 16:12 -> 16:35 = #0 // CHECK-MAIN-NEXT: File [[MAIN]], 16:35 -> 17:33 = #9 -// CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 17:12 -> 17:33 = #9 +// CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 17:12 -> 17:33 = #0 // CHECK-START: File [[START1:[0-9]]], 1:1 -> 5:1 = #0 // CHECK-START: File [[START1]], 4:17 -> 4:22 = (#0 + #1) @@ -65,15 +65,15 @@ int main() { // CHECK-CODE: File [[CODE2]], 9:11 -> 11:2 = #7 // CHECK-CODE: File [[CODE2]], 11:8 -> 13:2 = (#5 - #7) -// CHECK-END: File [[END1:[0-9]]], 1:1 -> 3:2 = #1 -// CHECK-END: File [[END1]], 1:1 -> 6:1 = #0 +// CHECK-END: File [[END1:[0-9]]], 1:1 -> 6:1 = #0 +// CHECK-END: File [[END1]], 1:1 -> 3:2 = #1 // CHECK-END: File [[END1]], 5:5 -> 5:9 = #0 // CHECK-END: File [[END1]], 5:11 -> 5:16 = #4 -// CHECK-END: File [[END2:[0-9]]], 1:1 -> 3:2 = #5 -// CHECK-END: File [[END2]], 1:1 -> 6:1 = #0 +// CHECK-END: File [[END2:[0-9]]], 1:1 -> 6:1 = #0 +// CHECK-END: File [[END2]], 1:1 -> 3:2 = #5 // CHECK-END: File [[END2]], 5:5 -> 5:9 = #0 // CHECK-END: File [[END2]], 5:11 -> 5:16 = #8 -// CHECK-END: File [[END3:[0-9]]], 1:1 -> 3:2 = #9 -// CHECK-END: File [[END3]], 1:1 -> 6:1 = #0 +// CHECK-END: File [[END3:[0-9]]], 1:1 -> 6:1 = #0 +// CHECK-END: File [[END3]], 1:1 -> 3:2 = #9 // CHECK-END: File [[END3]], 5:5 -> 5:9 = #0 // CHECK-END: File [[END3]], 5:11 -> 5:16 = #10 diff --git a/clang/test/CoverageMapping/macros.c b/clang/test/CoverageMapping/macros.c index fcf21170ef135c..00139f33229d57 100644 --- a/clang/test/CoverageMapping/macros.c +++ b/clang/test/CoverageMapping/macros.c @@ -80,12 +80,12 @@ void func7(void) { // CHECK-NEXT: File 0, [[@LINE]]:18 -> [[@LINE+6]]:2 = #0 int kk,ll; // CHECK-NEXT: File 0, [[@LINE+1]]:7 -> [[@LINE+1]]:8 = #0 if (k) // CHECK-NEXT: Branch,File 0, [[@LINE]]:7 -> [[@LINE]]:8 = #1 m(k); // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:9 -> [[@LINE]]:5 = #1 - else // CHECK-NEXT: Expansion,File 0, [[@LINE
[clang] [llvm] [MC/DC] Make covmap tolerant of nested Decisions (PR #125407)
llvmbot wrote: @llvm/pr-subscribers-clang Author: NAKAMURA Takumi (chapuni) Changes CoverageMappingWriter reorders `Region`s by `endLoc DESC` to prioritize wider `Decision` with the same `startLoc`. In `llvm-cov`, tweak seeking Decisions by reversal order to find smaller Decision first. --- Full diff: https://github.com/llvm/llvm-project/pull/125407.diff 5 Files Affected: - (modified) clang/test/CoverageMapping/ctor.cpp (+2-2) - (modified) clang/test/CoverageMapping/includehell.cpp (+9-9) - (modified) clang/test/CoverageMapping/macros.c (+3-3) - (modified) llvm/lib/ProfileData/Coverage/CoverageMapping.cpp (+2-2) - (modified) llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp (+8-1) ``diff diff --git a/clang/test/CoverageMapping/ctor.cpp b/clang/test/CoverageMapping/ctor.cpp index 1cf12cd738c2cc..191f73f5693e19 100644 --- a/clang/test/CoverageMapping/ctor.cpp +++ b/clang/test/CoverageMapping/ctor.cpp @@ -11,8 +11,8 @@ class B: public A { int c; B(int x, int y) : A(x), // CHECK: File 0, [[@LINE]]:7 -> [[@LINE]]:11 = #0 -// CHECK-NEXT: File 0, [[@LINE+1]]:7 -> [[@LINE+1]]:13 = #0 -b(x == 0? 1: 2),// CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:19 = #0 +// CHECK-NEXT: File 0, [[@LINE+1]]:7 -> [[@LINE+1]]:19 = #0 +b(x == 0? 1: 2),// CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:13 = #0 // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:7 -> [[@LINE-1]]:13 = #1, (#0 - #1) // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:14 -> [[@LINE-2]]:15 = #1 // CHECK-NEXT: File 0, [[@LINE-3]]:15 -> [[@LINE-3]]:16 = #1 diff --git a/clang/test/CoverageMapping/includehell.cpp b/clang/test/CoverageMapping/includehell.cpp index 09e03e77799d56..3c67672fabc0fc 100644 --- a/clang/test/CoverageMapping/includehell.cpp +++ b/clang/test/CoverageMapping/includehell.cpp @@ -28,14 +28,14 @@ int main() { // CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 6:12 -> 6:35 = #0 // CHECK-MAIN-NEXT: File [[MAIN]], 6:35 -> 10:33 = #1 // CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 8:14 -> 8:29 = #1 -// CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 10:12 -> 10:33 = #1 +// CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 10:12 -> 10:33 = #0 // CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 12:12 -> 12:35 = #0 // CHECK-MAIN-NEXT: File [[MAIN]], 12:35 -> 14:33 = #5 // CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 13:14 -> 13:29 = #5 -// CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 14:12 -> 14:33 = #5 +// CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 14:12 -> 14:33 = #0 // CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 16:12 -> 16:35 = #0 // CHECK-MAIN-NEXT: File [[MAIN]], 16:35 -> 17:33 = #9 -// CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 17:12 -> 17:33 = #9 +// CHECK-MAIN-NEXT: Expansion,File [[MAIN]], 17:12 -> 17:33 = #0 // CHECK-START: File [[START1:[0-9]]], 1:1 -> 5:1 = #0 // CHECK-START: File [[START1]], 4:17 -> 4:22 = (#0 + #1) @@ -65,15 +65,15 @@ int main() { // CHECK-CODE: File [[CODE2]], 9:11 -> 11:2 = #7 // CHECK-CODE: File [[CODE2]], 11:8 -> 13:2 = (#5 - #7) -// CHECK-END: File [[END1:[0-9]]], 1:1 -> 3:2 = #1 -// CHECK-END: File [[END1]], 1:1 -> 6:1 = #0 +// CHECK-END: File [[END1:[0-9]]], 1:1 -> 6:1 = #0 +// CHECK-END: File [[END1]], 1:1 -> 3:2 = #1 // CHECK-END: File [[END1]], 5:5 -> 5:9 = #0 // CHECK-END: File [[END1]], 5:11 -> 5:16 = #4 -// CHECK-END: File [[END2:[0-9]]], 1:1 -> 3:2 = #5 -// CHECK-END: File [[END2]], 1:1 -> 6:1 = #0 +// CHECK-END: File [[END2:[0-9]]], 1:1 -> 6:1 = #0 +// CHECK-END: File [[END2]], 1:1 -> 3:2 = #5 // CHECK-END: File [[END2]], 5:5 -> 5:9 = #0 // CHECK-END: File [[END2]], 5:11 -> 5:16 = #8 -// CHECK-END: File [[END3:[0-9]]], 1:1 -> 3:2 = #9 -// CHECK-END: File [[END3]], 1:1 -> 6:1 = #0 +// CHECK-END: File [[END3:[0-9]]], 1:1 -> 6:1 = #0 +// CHECK-END: File [[END3]], 1:1 -> 3:2 = #9 // CHECK-END: File [[END3]], 5:5 -> 5:9 = #0 // CHECK-END: File [[END3]], 5:11 -> 5:16 = #10 diff --git a/clang/test/CoverageMapping/macros.c b/clang/test/CoverageMapping/macros.c index fcf21170ef135c..00139f33229d57 100644 --- a/clang/test/CoverageMapping/macros.c +++ b/clang/test/CoverageMapping/macros.c @@ -80,12 +80,12 @@ void func7(void) { // CHECK-NEXT: File 0, [[@LINE]]:18 -> [[@LINE+6]]:2 = #0 int kk,ll; // CHECK-NEXT: File 0, [[@LINE+1]]:7 -> [[@LINE+1]]:8 = #0 if (k) // CHECK-NEXT: Branch,File 0, [[@LINE]]:7 -> [[@LINE]]:8 = #1 m(k); // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:9 -> [[@LINE]]:5 = #1 - else // CHECK-NEXT: Expansion,File 0, [[@LINE-1]]:5 -> [[@LINE-1]]:6 = #1 + else // CHECK-NEXT: Expansion,File 0, [[@LINE-1]]:5 -> [[@LINE-1]]:6 = #0 l = m(l); // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:7 -> [[@LINE]]:5 = (#0 - #1) } // CHECK-NEXT: File 0, [[@LINE-1]]:5 -> [[@LINE-1]]:10 = (#0 - #1) // CHECK-NEXT: E
[clang] [MC/DC] Refactor MCDC::State::Decision. NFC. (PR #125408)
https://github.com/chapuni created https://github.com/llvm/llvm-project/pull/125408 Introduce `ID` and `InvalidID`. Then `DecisionByStmt` can have three states. * Not assigned if the Stmt(Expr) doesn't exist. * When `DecisionByStmt[Expr]` exists: * Invalid and should be ignored if `ID == Invalid`. * Valid if `ID != Invalid`. Other member will be filled in the Mapper. >From d414f29ed8732c77fdcd05cc3b066e9ee0d9de07 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Sun, 2 Feb 2025 21:59:33 +0900 Subject: [PATCH] [MC/DC] Refactor MCDC::State::Decision. NFC. Introduce `ID` and `InvalidID`. Then `DecisionByStmt` can have three states. * Not assigned if the Stmt(Expr) doesn't exist. * When `DecisionByStmt[Expr]` exists: * Invalid and should be ignored if `ID == Invalid`. * Valid if `ID != Invalid`. Other member will be filled in the Mapper. --- clang/lib/CodeGen/CodeGenPGO.cpp | 2 +- clang/lib/CodeGen/CoverageMappingGen.cpp | 6 ++ clang/lib/CodeGen/MCDCState.h| 16 +++- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index 792373839107f0a..0331ff83e633f75 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -310,7 +310,7 @@ struct MapRegionCounters : public RecursiveASTVisitor { } // Otherwise, allocate the Decision. - MCDCState.DecisionByStmt[BinOp].BitmapIdx = 0; + MCDCState.DecisionByStmt[BinOp].ID = MCDCState.DecisionByStmt.size(); } return true; } diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index f09157771d2b5c0..4dbc0c70e34d60b 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -2197,10 +2197,8 @@ struct CounterCoverageMappingBuilder // Update the state for CodeGenPGO assert(MCDCState.DecisionByStmt.contains(E)); -MCDCState.DecisionByStmt[E] = { -MCDCState.BitmapBits, // Top -std::move(Builder.Indices), -}; +MCDCState.DecisionByStmt[E].update(MCDCState.BitmapBits, // Top + std::move(Builder.Indices)); auto DecisionParams = mcdc::DecisionParameters{ MCDCState.BitmapBits += NumTVs, // Tail diff --git a/clang/lib/CodeGen/MCDCState.h b/clang/lib/CodeGen/MCDCState.h index e0dd28ff90ed124..0b6f5f28235f43a 100644 --- a/clang/lib/CodeGen/MCDCState.h +++ b/clang/lib/CodeGen/MCDCState.h @@ -16,6 +16,8 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ProfileData/Coverage/MCDCTypes.h" +#include +#include namespace clang { class Stmt; @@ -30,8 +32,20 @@ struct State { unsigned BitmapBits = 0; struct Decision { +using IndicesTy = llvm::SmallVector>; +static constexpr auto InvalidID = std::numeric_limits::max(); + unsigned BitmapIdx; -llvm::SmallVector> Indices; +IndicesTy Indices; +unsigned ID = InvalidID; + +bool isValid() const { return ID != InvalidID; } + +void update(unsigned I, IndicesTy &&X) { + assert(ID != InvalidID); + BitmapIdx = I; + Indices = std::move(X); +} }; llvm::DenseMap DecisionByStmt; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [MC/DC] Refactor MCDC::State::Decision. NFC. (PR #125408)
llvmbot wrote: @llvm/pr-subscribers-clang-codegen Author: NAKAMURA Takumi (chapuni) Changes Introduce `ID` and `InvalidID`. Then `DecisionByStmt` can have three states. * Not assigned if the Stmt(Expr) doesn't exist. * When `DecisionByStmt[Expr]` exists: * Invalid and should be ignored if `ID == Invalid`. * Valid if `ID != Invalid`. Other member will be filled in the Mapper. --- Full diff: https://github.com/llvm/llvm-project/pull/125408.diff 3 Files Affected: - (modified) clang/lib/CodeGen/CodeGenPGO.cpp (+1-1) - (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+2-4) - (modified) clang/lib/CodeGen/MCDCState.h (+15-1) ``diff diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index 792373839107f0a..0331ff83e633f75 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -310,7 +310,7 @@ struct MapRegionCounters : public RecursiveASTVisitor { } // Otherwise, allocate the Decision. - MCDCState.DecisionByStmt[BinOp].BitmapIdx = 0; + MCDCState.DecisionByStmt[BinOp].ID = MCDCState.DecisionByStmt.size(); } return true; } diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index f09157771d2b5c0..4dbc0c70e34d60b 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -2197,10 +2197,8 @@ struct CounterCoverageMappingBuilder // Update the state for CodeGenPGO assert(MCDCState.DecisionByStmt.contains(E)); -MCDCState.DecisionByStmt[E] = { -MCDCState.BitmapBits, // Top -std::move(Builder.Indices), -}; +MCDCState.DecisionByStmt[E].update(MCDCState.BitmapBits, // Top + std::move(Builder.Indices)); auto DecisionParams = mcdc::DecisionParameters{ MCDCState.BitmapBits += NumTVs, // Tail diff --git a/clang/lib/CodeGen/MCDCState.h b/clang/lib/CodeGen/MCDCState.h index e0dd28ff90ed124..0b6f5f28235f43a 100644 --- a/clang/lib/CodeGen/MCDCState.h +++ b/clang/lib/CodeGen/MCDCState.h @@ -16,6 +16,8 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ProfileData/Coverage/MCDCTypes.h" +#include +#include namespace clang { class Stmt; @@ -30,8 +32,20 @@ struct State { unsigned BitmapBits = 0; struct Decision { +using IndicesTy = llvm::SmallVector>; +static constexpr auto InvalidID = std::numeric_limits::max(); + unsigned BitmapIdx; -llvm::SmallVector> Indices; +IndicesTy Indices; +unsigned ID = InvalidID; + +bool isValid() const { return ID != InvalidID; } + +void update(unsigned I, IndicesTy &&X) { + assert(ID != InvalidID); + BitmapIdx = I; + Indices = std::move(X); +} }; llvm::DenseMap DecisionByStmt; `` https://github.com/llvm/llvm-project/pull/125408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [MC/DC] Nested decision (PR #125403)
https://github.com/chapuni edited https://github.com/llvm/llvm-project/pull/125403 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [MC/DC] Nested decision (PR #125403)
https://github.com/chapuni edited https://github.com/llvm/llvm-project/pull/125403 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] fix P3310 overload resolution flag propagation (PR #125372)
@@ -2525,8 +2525,11 @@ void TextNodeDumper::VisitCXXRecordDecl(const CXXRecordDecl *D) { OS << " instantiated_from"; dumpPointer(Instance); } - if (const auto *CTSD = dyn_cast(D)) + if (const auto *CTSD = dyn_cast(D)) { dumpTemplateSpecializationKind(CTSD->getSpecializationKind()); +if (CTSD->hasMatchedPackOnParmToNonPackOnArg()) + OS << " strict_match"; + } mizvekov wrote: It's for testing and dumping. Well, the clang AST does in general tell us how things were formed. Implicit casts are one example, which shows in the ast-dump as well. As a debugging aid, I'd expect the art-dump to represent the state of the object, even including states that are for caching reasons only (ie this flag can be (expensively) computed from the parameter list and the template argument list as written). https://github.com/llvm/llvm-project/pull/125372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Replace /* ... */ single-line comments with // ... comments (PR #124319)
@@ -0,0 +1,162 @@ +//===--- UseCppStyleCommentsCheck.cpp - clang-tidy-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "UseCppStyleCommentsCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" +#include "clang/Lex/Preprocessor.h" +#include + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { +class UseCppStyleCommentsCheck::CStyleCommentHandler : public CommentHandler { +public: + CStyleCommentHandler(UseCppStyleCommentsCheck &Check, bool ExcludeDoxygen) + : Check(Check), ExcludeDoxygen(ExcludeDoxygen), +CStyleCommentMatch( +"^[ \t]*/\\*+[ \t\r\n]*(.*[ \t\r\n]*)*[ \t\r\n]*\\*+/[ \t\r\n]*$") { + } + + void setExcludeDoxygen(bool Exclude) { ExcludeDoxygen = Exclude; } + + bool isExcludeDoxygen() const { return ExcludeDoxygen; } + + std::string convertToCppStyleComment(const SourceManager &SM, + const SourceRange &Range) { +const StringRef CommentText = Lexer::getSourceText( +CharSourceRange::getTokenRange(Range), SM, LangOptions()); + +std::string InnerText = CommentText.str(); +InnerText.erase(0, 2); +InnerText.erase(InnerText.size() - 2, 2); + +std::string Result; +std::istringstream Stream(InnerText); +std::string Line; + +if (std::getline(Stream, Line)) { + const size_t StartPos = Line.find_first_not_of(" \t"); + if (StartPos != std::string::npos) { +Line = Line.substr(StartPos); + } else { +Line.clear(); + } + Result += "// " + Line; +} + +while (std::getline(Stream, Line)) { + const size_t StartPos = Line.find_first_not_of(" \t"); + if (StartPos != std::string::npos) { +Line = Line.substr(StartPos); + } else { +Line.clear(); + } + Result += "\n// " + Line; +} +return Result; + } + + bool isDoxygenStyleComment(const StringRef &Text) { +StringRef Trimmed = Text.ltrim(); +return Trimmed.starts_with("/**") || Trimmed.starts_with("/*!") || + Trimmed.starts_with("///") || Trimmed.starts_with("//!") || + (Trimmed.starts_with("/*") && +Trimmed.drop_front(2).starts_with("*")); + } + + bool CheckForInlineComments(Preprocessor &PP, SourceRange Range) { +const SourceManager &SM = PP.getSourceManager(); +const SourceLocation CommentStart = Range.getBegin(); +const SourceLocation CommentEnd = Range.getEnd(); + +unsigned StartLine = SM.getSpellingLineNumber(CommentStart); +unsigned EndLine = SM.getSpellingLineNumber(CommentEnd); + +const StringRef Text = Lexer::getSourceText( +CharSourceRange::getCharRange(Range), SM, PP.getLangOpts()); + +if (StartLine == EndLine) { + const SourceLocation LineBegin = + SM.translateLineCol(SM.getFileID(CommentStart), StartLine, 1); + const SourceLocation LineEnd = + SM.translateLineCol(SM.getFileID(CommentEnd), EndLine, + std::numeric_limits::max()); + const StringRef LineContent = Lexer::getSourceText( + CharSourceRange::getCharRange(LineBegin, LineEnd), SM, + PP.getLangOpts()); + const size_t CommentStartOffset = + SM.getSpellingColumnNumber(CommentStart) - 1; + const StringRef AfterComment = + LineContent.drop_front(CommentStartOffset + Text.size()); + + if (!AfterComment.trim().empty()) { +return true; + } +} +return false; + } + + bool HandleComment(Preprocessor &PP, SourceRange Range) override { +const SourceManager &SM = PP.getSourceManager(); + +if (Range.getBegin().isMacroID() || SM.isInSystemHeader(Range.getBegin())) { + return false; +} + +const StringRef Text = Lexer::getSourceText( +CharSourceRange::getCharRange(Range), SM, PP.getLangOpts()); + +if (ExcludeDoxygen && isDoxygenStyleComment(Text)) { + return false; +} + +SmallVector Matches; +if (!CStyleCommentMatch.match(Text, &Matches)) { + return false; +} + +if (CheckForInlineComments(PP, Range)) { + return false; +} + +Check.diag( +Range.getBegin(), +"use C++ style comments '//' instead of C style comments '/*...*/'") +<< clang::FixItHint::CreateReplacement( + Range, convertToCppStyleComment(SM, Range)); + +return false; + } + +private: + UseCppStyleCommentsCheck &Check; + bool ExcludeDoxygen; + llvm::Regex CStyleCommentMatch; +}; + +UseCppStyleCommentsCheck::UseCppStyleCommentsCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + H
[clang-tools-extra] [clang-tidy] Replace /* ... */ single-line comments with // ... comments (PR #124319)
https://github.com/EugeneZelenko edited https://github.com/llvm/llvm-project/pull/124319 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] fix P3310 overload resolution flag propagation (PR #125372)
@@ -1841,15 +1841,21 @@ class ClassTemplateSpecializationDecl : public CXXRecordDecl, LLVM_PREFERRED_TYPE(TemplateSpecializationKind) unsigned SpecializationKind : 3; + /// When matching the primary template, have we matched any packs on the + /// parameter side, versus any non-packs on the argument side, in a context + /// where the opposite matching is also allowed? + bool MatchedPackOnParmToNonPackOnArg : 1; mizvekov wrote: Yeah, thanks for catching, that was a slip on my part. P3310's current wording will introduce the term 'strict pack match', so we could go ahead and also rename all of them to that on a follow up. https://github.com/llvm/llvm-project/pull/125372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][TableGen] Use PointerType::get(Context) in MVE TableGen emitter (NFC) (PR #124782)
https://github.com/junlarsen updated https://github.com/llvm/llvm-project/pull/124782 >From bc77542fe58bdc94566a9a589039856de50617b0 Mon Sep 17 00:00:00 2001 From: Mats Jun Larsen Date: Wed, 29 Jan 2025 01:29:01 +0900 Subject: [PATCH] [Clang][TableGen] Use PointerType::get(Context) in MVE TableGen emitter Follow-up to #123569 --- clang/utils/TableGen/MveEmitter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/utils/TableGen/MveEmitter.cpp b/clang/utils/TableGen/MveEmitter.cpp index 58a4d3c22ac366..334aedbb8592b5 100644 --- a/clang/utils/TableGen/MveEmitter.cpp +++ b/clang/utils/TableGen/MveEmitter.cpp @@ -210,7 +210,7 @@ class PointerType : public Type { return Name + " *"; } std::string llvmName() const override { -return "llvm::PointerType::getUnqual(" + Pointee->llvmName() + ")"; +return "llvm::PointerType::getUnqual(CGM.getLLVMContext())"; } const Type *getPointeeType() const { return Pointee; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)
@@ -0,0 +1,147 @@ +.. title:: clang-tidy - performance-redundant-lookup + +performance-redundant-lookup + + +This check warns about potential redundant container lookup operations within +a function. + +Examples + + +.. code-block:: c++ + +if (map.count(key) && map[key] < threshold) { + // do stuff +} + +In this example, we would check if the key is present in the map, +and then do a second lookup to actually load the value. +We could refactor the code into this, to use a single lookup: + +.. code-block:: c++ + +if (auto it = map.find(key); it != map.end() && it->second < threshold) { + // do stuff +} + +In this example, we do three lookups while calculating, caching and then +using the result of some expensive computation: + +.. code-block:: c++ + +if (!cache.contains(key)) { +cache[key] = computeExpensiveValue(); +} +use(cache[key]); + +We could refactor this code using ``try_emplace`` to fill up the cache entry +if wasn't present, and just use it if we already computed the value. +This way we would only have a single unavoidable lookup: + +.. code-block:: c++ + +auto [cacheSlot, inserted] cache.try_emplace(key); +if (inserted) { +cacheSlot->second = computeExpensiveValue(); +} +use(cacheSlot->second); + + +What is a "lookup"? +--- + +All container operations that walk the internal structure of the container +should be considered as "lookups". +This means that checking if an element is present or inserting an element +is also considered as a "lookup". + +For example, ``contains``, ``count`` but even the ``operator[]`` +should be considered as "lookups". + +Technically ``insert``, ``emplace`` or ``try_emplace`` are also lookups, +even if due to limitations, they are not recognized as such. + +Lookups inside macros are ignored, thus not considered as "lookups". +For example: + +.. code-block:: c++ + +assert(map.count(key) == 0); // Not considered as a "lookup". + +Options +--- + +.. option:: ContainerNameRegex + + The regular expression matching the type of the container objects. + This is matched in a case insensitive manner. + Default is ``set|map``. steakhal wrote: > How about unordered_map/unordered_set? They would match the `set|map` case-insensitive regex. https://github.com/llvm/llvm-project/pull/125420 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)
@@ -0,0 +1,159 @@ +//===--- RedundantLookupCheck.cpp - clang-tidy ===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantLookupCheck.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Regex.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::performance { + +static constexpr auto DefaultContainerNameRegex = "set|map"; + +static const llvm::StringRef DefaultLookupMethodNames = +llvm::StringLiteral( // +"at;" +"contains;" +"count;" +"find_as;" +"find;" +// These are tricky, as they take the "key" at different places. +// They sometimes bundle up the key and the value together in a pair. +// "emplace;" +// "insert_or_assign;" +// "insert;" +// "try_emplace;" +) +.drop_back(); // Drops the last semicolon. + +RedundantLookupCheck::RedundantLookupCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + ContainerNameRegex( + Options.get("ContainerNameRegex", DefaultContainerNameRegex)), + LookupMethodNames(utils::options::parseStringList( + Options.get("LookupMethodNames", DefaultLookupMethodNames))) {} + +void RedundantLookupCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "ContainerNameRegex", ContainerNameRegex); + Options.store(Opts, "LookupMethodNames", +utils::options::serializeStringList(LookupMethodNames)); +} + +namespace { +/// Checks if any of the ends of the source range is in a macro expansion. +AST_MATCHER(Expr, hasMacroSourceRange) { + SourceRange R = Node.getSourceRange(); + return R.getBegin().isMacroID() || R.getEnd().isMacroID(); +} +} // namespace + +static constexpr const char *ObjKey = "obj"; +static constexpr const char *LookupKey = "key"; +static constexpr const char *LookupCallKey = "lookup"; +static constexpr const char *EnclosingFnKey = "fn"; + +void RedundantLookupCheck::registerMatchers(MatchFinder *Finder) { + auto MatchesContainerNameRegex = + matchesName(ContainerNameRegex, llvm::Regex::IgnoreCase); + + // Match that the expression is a record type with a name that contains "map" + // or "set". + auto RecordCalledMapOrSet = + expr(ignoringImpCasts(hasType(hasUnqualifiedDesugaredType(recordType( + hasDeclaration(namedDecl(MatchesContainerNameRegex))) + .bind(ObjKey); + + auto SubscriptCall = + cxxOperatorCallExpr(hasOverloadedOperatorName("[]"), argumentCountIs(2), + hasArgument(0, RecordCalledMapOrSet), + hasArgument(1, expr().bind(LookupKey))); + + auto LookupMethodCalls = + cxxMemberCallExpr(on(RecordCalledMapOrSet), argumentCountIs(1), +hasArgument(0, expr().bind(LookupKey)), +callee(cxxMethodDecl(hasAnyName(LookupMethodNames; + + // Match any lookup or subscript calls that are not in a macro expansion. + auto AnyLookup = callExpr(unless(hasMacroSourceRange()), +anyOf(SubscriptCall, LookupMethodCalls)) + .bind(LookupCallKey); + + // We need to collect all lookups in a function to be able to report them in + // batches. + Finder->addMatcher( + functionDecl(hasBody(compoundStmt(forEachDescendant(AnyLookup + .bind(EnclosingFnKey), + this); +} + +/// Hash the container object expr along with the key used for lookup and the +/// enclosing function together. +static unsigned hashLookupEvent(const ASTContext &Ctx, +const FunctionDecl *EnclosingFn, +const Expr *LookupKey, +const Expr *ContainerObject) { + llvm::FoldingSetNodeID ID; + ID.AddPointer(EnclosingFn); + + LookupKey->Profile(ID, Ctx, /*Canonical=*/true, + /*ProfileLambdaExpr=*/true); + ContainerObject->Profile(ID, Ctx, /*Canonical=*/true, + /*ProfileLambdaExpr=*/true); + return ID.ComputeHash(); +} + +void RedundantLookupCheck::check(const MatchFinder::MatchResult &Result) { + SM = Result.SourceManager; + + const auto *EnclosingFn = + Result.Nodes.getNodeAs(EnclosingFnKey); + const auto *LookupCall = Result.Nodes.getNodeAs(LookupCallKey); + const auto *Key = Result.Nodes.getNodeAs(LookupKey); + const auto *ContainerObject = Result.Nodes.getNodeAs(ObjKey);
[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)
@@ -0,0 +1,147 @@ +.. title:: clang-tidy - performance-redundant-lookup + +performance-redundant-lookup + + +This check warns about potential redundant container lookup operations within +a function. + +Examples + + +.. code-block:: c++ + +if (map.count(key) && map[key] < threshold) { + // do stuff +} + +In this example, we would check if the key is present in the map, +and then do a second lookup to actually load the value. +We could refactor the code into this, to use a single lookup: + +.. code-block:: c++ + +if (auto it = map.find(key); it != map.end() && it->second < threshold) { + // do stuff +} + +In this example, we do three lookups while calculating, caching and then +using the result of some expensive computation: + +.. code-block:: c++ + +if (!cache.contains(key)) { +cache[key] = computeExpensiveValue(); +} +use(cache[key]); + +We could refactor this code using ``try_emplace`` to fill up the cache entry +if wasn't present, and just use it if we already computed the value. +This way we would only have a single unavoidable lookup: + +.. code-block:: c++ + +auto [cacheSlot, inserted] cache.try_emplace(key); +if (inserted) { +cacheSlot->second = computeExpensiveValue(); +} +use(cacheSlot->second); + + +What is a "lookup"? +--- + +All container operations that walk the internal structure of the container +should be considered as "lookups". +This means that checking if an element is present or inserting an element +is also considered as a "lookup". + +For example, ``contains``, ``count`` but even the ``operator[]`` +should be considered as "lookups". + +Technically ``insert``, ``emplace`` or ``try_emplace`` are also lookups, +even if due to limitations, they are not recognized as such. + +Lookups inside macros are ignored, thus not considered as "lookups". +For example: + +.. code-block:: c++ + +assert(map.count(key) == 0); // Not considered as a "lookup". + +Options +--- + +.. option:: ContainerNameRegex + + The regular expression matching the type of the container objects. + This is matched in a case insensitive manner. + Default is ``set|map``. firewave wrote: That would also match `{flat_|unordered_}{multi}{set|map}` from the standard which appear to have similar interfaces which should be fine. But maybe it should be limited to the `std` namespace so it does not apply to any implementation. And it would also match an object like `class my_mmap`. https://github.com/llvm/llvm-project/pull/125420 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Lex] Migrate away from PointerUnion::dyn_cast (NFC) (PR #125380)
https://github.com/kazutakahirata closed https://github.com/llvm/llvm-project/pull/125380 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)
@@ -0,0 +1,147 @@ +.. title:: clang-tidy - performance-redundant-lookup + +performance-redundant-lookup + + +This check warns about potential redundant container lookup operations within +a function. + +Examples + + +.. code-block:: c++ + +if (map.count(key) && map[key] < threshold) { + // do stuff +} + +In this example, we would check if the key is present in the map, +and then do a second lookup to actually load the value. +We could refactor the code into this, to use a single lookup: + +.. code-block:: c++ + +if (auto it = map.find(key); it != map.end() && it->second < threshold) { + // do stuff +} + +In this example, we do three lookups while calculating, caching and then +using the result of some expensive computation: + +.. code-block:: c++ + +if (!cache.contains(key)) { +cache[key] = computeExpensiveValue(); +} +use(cache[key]); + +We could refactor this code using ``try_emplace`` to fill up the cache entry +if wasn't present, and just use it if we already computed the value. +This way we would only have a single unavoidable lookup: + +.. code-block:: c++ + +auto [cacheSlot, inserted] cache.try_emplace(key); +if (inserted) { +cacheSlot->second = computeExpensiveValue(); +} +use(cacheSlot->second); + + +What is a "lookup"? +--- + +All container operations that walk the internal structure of the container +should be considered as "lookups". +This means that checking if an element is present or inserting an element +is also considered as a "lookup". + +For example, ``contains``, ``count`` but even the ``operator[]`` +should be considered as "lookups". + +Technically ``insert``, ``emplace`` or ``try_emplace`` are also lookups, +even if due to limitations, they are not recognized as such. + +Lookups inside macros are ignored, thus not considered as "lookups". +For example: + +.. code-block:: c++ + +assert(map.count(key) == 0); // Not considered as a "lookup". + +Options +--- + +.. option:: ContainerNameRegex + + The regular expression matching the type of the container objects. + This is matched in a case insensitive manner. + Default is ``set|map``. EugeneZelenko wrote: How about `boost`? https://github.com/llvm/llvm-project/pull/125420 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)
firewave wrote: Could you please run this with `--enable-check-profile` to see how heavy it is? https://github.com/llvm/llvm-project/pull/125420 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [nfc][analyzer] Add MemSpace trait to program state (PR #123003)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/123003 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)
@@ -0,0 +1,147 @@ +.. title:: clang-tidy - performance-redundant-lookup + +performance-redundant-lookup + + +This check warns about potential redundant container lookup operations within +a function. + +Examples + + +.. code-block:: c++ + +if (map.count(key) && map[key] < threshold) { + // do stuff +} + +In this example, we would check if the key is present in the map, +and then do a second lookup to actually load the value. +We could refactor the code into this, to use a single lookup: + +.. code-block:: c++ + +if (auto it = map.find(key); it != map.end() && it->second < threshold) { + // do stuff +} + +In this example, we do three lookups while calculating, caching and then +using the result of some expensive computation: + +.. code-block:: c++ + +if (!cache.contains(key)) { +cache[key] = computeExpensiveValue(); +} +use(cache[key]); + +We could refactor this code using ``try_emplace`` to fill up the cache entry +if wasn't present, and just use it if we already computed the value. +This way we would only have a single unavoidable lookup: + +.. code-block:: c++ + +auto [cacheSlot, inserted] cache.try_emplace(key); +if (inserted) { +cacheSlot->second = computeExpensiveValue(); +} +use(cacheSlot->second); + + +What is a "lookup"? +--- + +All container operations that walk the internal structure of the container +should be considered as "lookups". +This means that checking if an element is present or inserting an element +is also considered as a "lookup". + +For example, ``contains``, ``count`` but even the ``operator[]`` +should be considered as "lookups". + +Technically ``insert``, ``emplace`` or ``try_emplace`` are also lookups, +even if due to limitations, they are not recognized as such. + +Lookups inside macros are ignored, thus not considered as "lookups". +For example: + +.. code-block:: c++ + +assert(map.count(key) == 0); // Not considered as a "lookup". + +Options +--- + +.. option:: ContainerNameRegex + + The regular expression matching the type of the container objects. + This is matched in a case insensitive manner. + Default is ``set|map``. + +.. option:: LookupMethodNames + + Member function names to consider as **lookup** operation. + These methods must have exactly 1 argument. + Default is ``at;contains;count;find_as;find``. EugeneZelenko wrote: ```suggestion Default is `at;contains;count;find_as;find`. ``` https://github.com/llvm/llvm-project/pull/125420 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)
@@ -0,0 +1,147 @@ +.. title:: clang-tidy - performance-redundant-lookup + +performance-redundant-lookup + + +This check warns about potential redundant container lookup operations within +a function. + +Examples + + +.. code-block:: c++ + +if (map.count(key) && map[key] < threshold) { + // do stuff +} + +In this example, we would check if the key is present in the map, +and then do a second lookup to actually load the value. +We could refactor the code into this, to use a single lookup: + +.. code-block:: c++ + +if (auto it = map.find(key); it != map.end() && it->second < threshold) { + // do stuff +} + +In this example, we do three lookups while calculating, caching and then +using the result of some expensive computation: + +.. code-block:: c++ + +if (!cache.contains(key)) { +cache[key] = computeExpensiveValue(); +} +use(cache[key]); + +We could refactor this code using ``try_emplace`` to fill up the cache entry +if wasn't present, and just use it if we already computed the value. +This way we would only have a single unavoidable lookup: + +.. code-block:: c++ + +auto [cacheSlot, inserted] cache.try_emplace(key); +if (inserted) { +cacheSlot->second = computeExpensiveValue(); +} +use(cacheSlot->second); + + +What is a "lookup"? +--- + +All container operations that walk the internal structure of the container +should be considered as "lookups". +This means that checking if an element is present or inserting an element +is also considered as a "lookup". + +For example, ``contains``, ``count`` but even the ``operator[]`` +should be considered as "lookups". + +Technically ``insert``, ``emplace`` or ``try_emplace`` are also lookups, +even if due to limitations, they are not recognized as such. + +Lookups inside macros are ignored, thus not considered as "lookups". +For example: + +.. code-block:: c++ + +assert(map.count(key) == 0); // Not considered as a "lookup". + +Options +--- + +.. option:: ContainerNameRegex + + The regular expression matching the type of the container objects. + This is matched in a case insensitive manner. + Default is ``set|map``. + +.. option:: LookupMethodNames + + Member function names to consider as **lookup** operation. + These methods must have exactly 1 argument. + Default is ``at;contains;count;find_as;find``. + +Limitations +--- + + - The "redundant lookups" may span across a large chunk of code. + Such reports can be considered as false-positives because it's hard to judge + if the container is definitely not mutated between the lookups. + It would be hard to split the lookup groups in a stable and meaningful way, + and a threshold for proximity would be just an arbitrary limit. + + - The "redundant lookups" may span across different control-flow constructs, + making it impossible to refactor. + It may be that the code was deliberately structured like it was, thus the + report is considered a false-positive. + Use your best judgement to see if anything needs to be fixed or not. + For example: + + .. code-block:: c++ + +if (coin()) +map[key] = foo(); +else +map[key] = bar(); + + Could be refactored into: + + .. code-block:: c++ + +map[key] = coin() ? foo() : bar(); + + However, the following code could be considered intentional: + + .. code-block:: c++ + +// Handle the likely case. +if (auto it = map.find(key); it != map.end()) { +return process(*it); +} + +// Commit the dirty items, and check again. +for (const auto &item : dirtyList) { +commit(item, map); // Updates the "map". +} + +// Do a final check. +if (auto it = map.find(key); it != map.end()) { +return process(*it); +} + + - The key argument of a lookup may have sideffects. Sideffects are ignored when identifying lookups. + This can introduce some false-positives. For example: + + .. code-block:: c++ + +m.contains(rng(++n)); +m.contains(rng(++n)); // FP: This is considered a redundant lookup. + + - Lookup member functions must have exactly 1 argument to match. + There are technically lookup functions, such as ``insert`` or ``try_emplace``, + but it would be hard to identify the "key" part of the argument, + while leaving the implementation open for user-configuration via the + ``LookupMethodNames`` option. EugeneZelenko wrote: ```suggestion `LookupMethodNames` option. ``` https://github.com/llvm/llvm-project/pull/125420 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)
@@ -0,0 +1,147 @@ +.. title:: clang-tidy - performance-redundant-lookup + +performance-redundant-lookup + + +This check warns about potential redundant container lookup operations within +a function. + +Examples + + +.. code-block:: c++ + +if (map.count(key) && map[key] < threshold) { + // do stuff +} + +In this example, we would check if the key is present in the map, +and then do a second lookup to actually load the value. +We could refactor the code into this, to use a single lookup: + +.. code-block:: c++ + +if (auto it = map.find(key); it != map.end() && it->second < threshold) { + // do stuff +} + +In this example, we do three lookups while calculating, caching and then +using the result of some expensive computation: + +.. code-block:: c++ + +if (!cache.contains(key)) { +cache[key] = computeExpensiveValue(); +} +use(cache[key]); + +We could refactor this code using ``try_emplace`` to fill up the cache entry +if wasn't present, and just use it if we already computed the value. +This way we would only have a single unavoidable lookup: + +.. code-block:: c++ + +auto [cacheSlot, inserted] cache.try_emplace(key); +if (inserted) { +cacheSlot->second = computeExpensiveValue(); +} +use(cacheSlot->second); + + +What is a "lookup"? +--- + +All container operations that walk the internal structure of the container +should be considered as "lookups". +This means that checking if an element is present or inserting an element +is also considered as a "lookup". + +For example, ``contains``, ``count`` but even the ``operator[]`` +should be considered as "lookups". + +Technically ``insert``, ``emplace`` or ``try_emplace`` are also lookups, +even if due to limitations, they are not recognized as such. + +Lookups inside macros are ignored, thus not considered as "lookups". +For example: + +.. code-block:: c++ + +assert(map.count(key) == 0); // Not considered as a "lookup". + +Options +--- + +.. option:: ContainerNameRegex + + The regular expression matching the type of the container objects. + This is matched in a case insensitive manner. + Default is ``set|map``. EugeneZelenko wrote: ```suggestion Default is `set|map`. ``` Please use single back-ticks for option names and values. How about `unordered_map/unordered_set`? https://github.com/llvm/llvm-project/pull/125420 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/125420 >From 78390b2af01fcd5acfbd2a313852962c873e5611 Mon Sep 17 00:00:00 2001 From: Balazs Benics Date: Sun, 2 Feb 2025 17:35:39 +0100 Subject: [PATCH 1/5] [clang-tidy] Add performance-redundant-lookup check Finds redundant lookups to set|map containers. For example: `map.count(key) && map[key] < threshold` --- .../clang-tidy/performance/CMakeLists.txt | 1 + .../performance/PerformanceTidyModule.cpp | 3 + .../performance/RedundantLookupCheck.cpp | 159 ++ .../performance/RedundantLookupCheck.h| 46 + clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../checks/performance/redundant-lookup.rst | 147 .../checkers/performance/redundant-lookup.cpp | 142 8 files changed, 504 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/performance/RedundantLookupCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/performance/RedundantLookupCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/performance/redundant-lookup.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/redundant-lookup.cpp diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt index c6e547c5089fb0e..6f907852c9fd1c4 100644 --- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt @@ -21,6 +21,7 @@ add_clang_library(clangTidyPerformanceModule STATIC NoexceptMoveConstructorCheck.cpp NoexceptSwapCheck.cpp PerformanceTidyModule.cpp + RedundantLookupCheck.cpp TriviallyDestructibleCheck.cpp TypePromotionInMathFnCheck.cpp UnnecessaryCopyInitialization.cpp diff --git a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp index 9e0fa6f88b36a00..a613114e6b83bd3 100644 --- a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp @@ -24,6 +24,7 @@ #include "NoexceptDestructorCheck.h" #include "NoexceptMoveConstructorCheck.h" #include "NoexceptSwapCheck.h" +#include "RedundantLookupCheck.h" #include "TriviallyDestructibleCheck.h" #include "TypePromotionInMathFnCheck.h" #include "UnnecessaryCopyInitialization.h" @@ -62,6 +63,8 @@ class PerformanceModule : public ClangTidyModule { "performance-noexcept-move-constructor"); CheckFactories.registerCheck( "performance-noexcept-swap"); +CheckFactories.registerCheck( +"performance-redundant-lookup"); CheckFactories.registerCheck( "performance-trivially-destructible"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/performance/RedundantLookupCheck.cpp b/clang-tools-extra/clang-tidy/performance/RedundantLookupCheck.cpp new file mode 100644 index 000..fefb28682c5b8af --- /dev/null +++ b/clang-tools-extra/clang-tidy/performance/RedundantLookupCheck.cpp @@ -0,0 +1,159 @@ +//===--- RedundantLookupCheck.cpp - clang-tidy ===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantLookupCheck.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Regex.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::performance { + +static constexpr auto DefaultContainerNameRegex = "set|map"; + +static const llvm::StringRef DefaultLookupMethodNames = +llvm::StringLiteral( // +"at;" +"contains;" +"count;" +"find_as;" +"find;" +// These are tricky, as they take the "key" at different places. +// They sometimes bundle up the key and the value together in a pair. +// "emplace;" +// "insert_or_assign;" +// "insert;" +// "try_emplace;" +) +.drop_back(); // Drops the last semicolon. + +RedundantLookupCheck::RedundantLookupCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + ContainerNameRegex( + Options.get("ContainerNameRegex", DefaultContainerNameRegex)), + LookupMethodNames(utils::options::parseStringList( + Options.get("LookupMethodNames", DefaultLookupMethodNames))) {} + +void RedundantLookupCheck::storeOptions(ClangTidy
[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/125420 >From 78390b2af01fcd5acfbd2a313852962c873e5611 Mon Sep 17 00:00:00 2001 From: Balazs Benics Date: Sun, 2 Feb 2025 17:35:39 +0100 Subject: [PATCH 1/6] [clang-tidy] Add performance-redundant-lookup check Finds redundant lookups to set|map containers. For example: `map.count(key) && map[key] < threshold` --- .../clang-tidy/performance/CMakeLists.txt | 1 + .../performance/PerformanceTidyModule.cpp | 3 + .../performance/RedundantLookupCheck.cpp | 159 ++ .../performance/RedundantLookupCheck.h| 46 + clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../checks/performance/redundant-lookup.rst | 147 .../checkers/performance/redundant-lookup.cpp | 142 8 files changed, 504 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/performance/RedundantLookupCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/performance/RedundantLookupCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/performance/redundant-lookup.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/redundant-lookup.cpp diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt index c6e547c5089fb0e..6f907852c9fd1c4 100644 --- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt @@ -21,6 +21,7 @@ add_clang_library(clangTidyPerformanceModule STATIC NoexceptMoveConstructorCheck.cpp NoexceptSwapCheck.cpp PerformanceTidyModule.cpp + RedundantLookupCheck.cpp TriviallyDestructibleCheck.cpp TypePromotionInMathFnCheck.cpp UnnecessaryCopyInitialization.cpp diff --git a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp index 9e0fa6f88b36a00..a613114e6b83bd3 100644 --- a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp @@ -24,6 +24,7 @@ #include "NoexceptDestructorCheck.h" #include "NoexceptMoveConstructorCheck.h" #include "NoexceptSwapCheck.h" +#include "RedundantLookupCheck.h" #include "TriviallyDestructibleCheck.h" #include "TypePromotionInMathFnCheck.h" #include "UnnecessaryCopyInitialization.h" @@ -62,6 +63,8 @@ class PerformanceModule : public ClangTidyModule { "performance-noexcept-move-constructor"); CheckFactories.registerCheck( "performance-noexcept-swap"); +CheckFactories.registerCheck( +"performance-redundant-lookup"); CheckFactories.registerCheck( "performance-trivially-destructible"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/performance/RedundantLookupCheck.cpp b/clang-tools-extra/clang-tidy/performance/RedundantLookupCheck.cpp new file mode 100644 index 000..fefb28682c5b8af --- /dev/null +++ b/clang-tools-extra/clang-tidy/performance/RedundantLookupCheck.cpp @@ -0,0 +1,159 @@ +//===--- RedundantLookupCheck.cpp - clang-tidy ===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "RedundantLookupCheck.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Regex.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::performance { + +static constexpr auto DefaultContainerNameRegex = "set|map"; + +static const llvm::StringRef DefaultLookupMethodNames = +llvm::StringLiteral( // +"at;" +"contains;" +"count;" +"find_as;" +"find;" +// These are tricky, as they take the "key" at different places. +// They sometimes bundle up the key and the value together in a pair. +// "emplace;" +// "insert_or_assign;" +// "insert;" +// "try_emplace;" +) +.drop_back(); // Drops the last semicolon. + +RedundantLookupCheck::RedundantLookupCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + ContainerNameRegex( + Options.get("ContainerNameRegex", DefaultContainerNameRegex)), + LookupMethodNames(utils::options::parseStringList( + Options.get("LookupMethodNames", DefaultLookupMethodNames))) {} + +void RedundantLookupCheck::storeOptions(ClangTidy
[clang] [libclang] Migrate away from PointerUnion::dyn_cast (NFC) (PR #125381)
https://github.com/kazutakahirata closed https://github.com/llvm/llvm-project/pull/125381 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 09e7b40 - [libclang] Migrate away from PointerUnion::dyn_cast (NFC) (#125381)
Author: Kazu Hirata Date: 2025-02-02T09:31:39-08:00 New Revision: 09e7b40bd3d7a1a2d9153912224bcf612954ead5 URL: https://github.com/llvm/llvm-project/commit/09e7b40bd3d7a1a2d9153912224bcf612954ead5 DIFF: https://github.com/llvm/llvm-project/commit/09e7b40bd3d7a1a2d9153912224bcf612954ead5.diff LOG: [libclang] Migrate away from PointerUnion::dyn_cast (NFC) (#125381) Note that PointerUnion::dyn_cast has been soft deprecated in PointerUnion.h: // FIXME: Replace the uses of is(), get() and dyn_cast() with //isa, cast and the llvm::dyn_cast Literal migration would result in dyn_cast_if_present (see the definition of PointerUnion::dyn_cast), but this patch uses dyn_cast because we expect Storage to be nonnull. Added: Modified: clang/tools/libclang/CIndex.cpp Removed: diff --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp index 42f095fea2db26..bf7fdeec0cc51b 100644 --- a/clang/tools/libclang/CIndex.cpp +++ b/clang/tools/libclang/CIndex.cpp @@ -7415,11 +7415,11 @@ unsigned clang_getNumOverloadedDecls(CXCursor C) { return 0; OverloadedDeclRefStorage Storage = getCursorOverloadedDeclRef(C).first; - if (const OverloadExpr *E = Storage.dyn_cast()) + if (const OverloadExpr *E = dyn_cast(Storage)) return E->getNumDecls(); if (OverloadedTemplateStorage *S = - Storage.dyn_cast()) + dyn_cast(Storage)) return S->size(); const Decl *D = cast(Storage); @@ -7438,11 +7438,11 @@ CXCursor clang_getOverloadedDecl(CXCursor cursor, unsigned index) { CXTranslationUnit TU = getCursorTU(cursor); OverloadedDeclRefStorage Storage = getCursorOverloadedDeclRef(cursor).first; - if (const OverloadExpr *E = Storage.dyn_cast()) + if (const OverloadExpr *E = dyn_cast(Storage)) return MakeCXCursor(E->decls_begin()[index], TU); if (OverloadedTemplateStorage *S = - Storage.dyn_cast()) + dyn_cast(Storage)) return MakeCXCursor(S->begin()[index], TU); const Decl *D = cast(Storage); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 43b7124 - [AST] Migrate away from PointerUnion::dyn_cast (NFC) (#125379)
Author: Kazu Hirata Date: 2025-02-02T09:31:06-08:00 New Revision: 43b7124c5749b3f3276adf1b869e623e163e230e URL: https://github.com/llvm/llvm-project/commit/43b7124c5749b3f3276adf1b869e623e163e230e DIFF: https://github.com/llvm/llvm-project/commit/43b7124c5749b3f3276adf1b869e623e163e230e.diff LOG: [AST] Migrate away from PointerUnion::dyn_cast (NFC) (#125379) Note that PointerUnion::dyn_cast has been soft deprecated in PointerUnion.h: // FIXME: Replace the uses of is(), get() and dyn_cast() with //isa, cast and the llvm::dyn_cast This patch migrates the use of PointerUnion::dyn_cast to dyn_cast_if_present because the non-const variant of getInitializedFieldInUnion is known to encounter null in ArrayFillerOrUnionFieldInit. See: commit 563c7c5539f05e7f8cbb42565c1f24466019f38b Author: Kazu Hirata Date: Sat Jan 25 14:05:01 2025 -0800 FWIW, I am not aware of any test case in check-clang that triggers null here. Added: Modified: clang/include/clang/AST/ExprCXX.h Removed: diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h index 0b6c8cfb163c958..98ba2bb41bb54d9 100644 --- a/clang/include/clang/AST/ExprCXX.h +++ b/clang/include/clang/AST/ExprCXX.h @@ -5040,7 +5040,7 @@ class CXXParenListInitExpr final } const FieldDecl *getInitializedFieldInUnion() const { -return ArrayFillerOrUnionFieldInit.dyn_cast(); +return dyn_cast_if_present(ArrayFillerOrUnionFieldInit); } child_range children() { ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AST] Migrate away from PointerUnion::dyn_cast (NFC) (PR #125379)
https://github.com/kazutakahirata closed https://github.com/llvm/llvm-project/pull/125379 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 2767f4b - [Lex] Migrate away from PointerUnion::dyn_cast (NFC) (#125380)
Author: Kazu Hirata Date: 2025-02-02T09:31:23-08:00 New Revision: 2767f4bf0d5a70aa990dbfb3341a1bc03effcdbd URL: https://github.com/llvm/llvm-project/commit/2767f4bf0d5a70aa990dbfb3341a1bc03effcdbd DIFF: https://github.com/llvm/llvm-project/commit/2767f4bf0d5a70aa990dbfb3341a1bc03effcdbd.diff LOG: [Lex] Migrate away from PointerUnion::dyn_cast (NFC) (#125380) Note that PointerUnion::dyn_cast has been soft deprecated in PointerUnion.h: // FIXME: Replace the uses of is(), get() and dyn_cast() with //isa, cast and the llvm::dyn_cast This patch migrates the use of PointerUnion::dyn_cast to dyn_cast_if_present because State is not guaranteed to be nonnull elsewhere in this class. See: commit 563c7c5539f05e7f8cbb42565c1f24466019f38b Author: Kazu Hirata Date: Sat Jan 25 14:05:01 2025 -0800 FWIW, I am not aware of any test case in check-clang that triggers null here. Added: Modified: clang/include/clang/Lex/Preprocessor.h Removed: diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h index 416f403c298410c..2bf4d1a16699430 100644 --- a/clang/include/clang/Lex/Preprocessor.h +++ b/clang/include/clang/Lex/Preprocessor.h @@ -933,7 +933,7 @@ class Preprocessor { } ArrayRef getOverriddenMacros() const { - if (auto *Info = State.dyn_cast()) + if (auto *Info = dyn_cast_if_present(State)) return Info->OverriddenMacros; return {}; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [experimental] Detect return-stack-addr using CFG (PR #124133)
https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/124133 >From 22990789b61e9f9d22e88a6b008eb3166fd1cb56 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena Date: Thu, 23 Jan 2025 15:47:39 + Subject: [PATCH 1/3] [experimental] Detect return-stack-addr using CFG --- .../Analysis/Analyses/DanglingReference.h | 14 + clang/include/clang/Basic/DiagnosticGroups.td | 3 + .../clang/Basic/DiagnosticSemaKinds.td| 8 + clang/lib/Analysis/CMakeLists.txt | 1 + clang/lib/Analysis/DanglingReference.cpp | 351 ++ clang/lib/Sema/AnalysisBasedWarnings.cpp | 8 + .../test/Sema/warn-lifetime-analysis-cfg.cpp | 136 +++ 7 files changed, 521 insertions(+) create mode 100644 clang/include/clang/Analysis/Analyses/DanglingReference.h create mode 100644 clang/lib/Analysis/DanglingReference.cpp create mode 100644 clang/test/Sema/warn-lifetime-analysis-cfg.cpp diff --git a/clang/include/clang/Analysis/Analyses/DanglingReference.h b/clang/include/clang/Analysis/Analyses/DanglingReference.h new file mode 100644 index 000..c9f5753eed070e6 --- /dev/null +++ b/clang/include/clang/Analysis/Analyses/DanglingReference.h @@ -0,0 +1,14 @@ +#ifndef LLVM_CLANG_ANALYSIS_ANALYSES_DANGLING_REFERENCE_H +#define LLVM_CLANG_ANALYSIS_ANALYSES_DANGLING_REFERENCE_H +#include "clang/AST/DeclBase.h" +#include "clang/Analysis/AnalysisDeclContext.h" +#include "clang/Analysis/CFG.h" +#include "clang/Sema/Sema.h" + +namespace clang { +void runDanglingReferenceAnalysis(const DeclContext &dc, const CFG &cfg, + AnalysisDeclContext &ac, Sema &S); + +} // namespace clang + +#endif // LLVM_CLANG_ANALYSIS_ANALYSES_DANGLING_REFERENCE_H diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 594e99a19b64d61..eeddd6eb82a3019 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -472,6 +472,9 @@ def Dangling : DiagGroup<"dangling", [DanglingAssignment, DanglingInitializerList, DanglingGsl, ReturnStackAddress]>; +def ReturnStackAddressCFG : DiagGroup<"return-stack-address-cfg">; +def DanglingCFG : DiagGroup<"dangling-cfg", [ReturnStackAddressCFG]>; + def DistributedObjectModifiers : DiagGroup<"distributed-object-modifiers">; def DllexportExplicitInstantiationDecl : DiagGroup<"dllexport-explicit-instantiation-decl">; def ExcessInitializers : DiagGroup<"excess-initializers">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 8be4f946dce1ccb..846cf6b3d45f8a8 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10169,6 +10169,14 @@ def err_lifetimebound_implicit_object_parameter_void_return_type : Error< "parameter of a function that returns void; " "did you mean 'lifetime_capture_by(X)'">; +// CFG based lifetime analysis. +def warn_ret_stack_variable_ref_cfg : Warning< + "returning reference to a stack variable">, InGroup, DefaultIgnore; +def note_local_variable_declared_here : Note<"reference to this stack variable is returned">; + +def warn_ret_stack_temporary_ref_cfg : Warning< + "returning reference to a temporary object">, InGroup, DefaultIgnore; + // CHECK: returning address/reference of stack memory def warn_ret_stack_addr_ref : Warning< "%select{address of|reference to}0 stack memory associated with " diff --git a/clang/lib/Analysis/CMakeLists.txt b/clang/lib/Analysis/CMakeLists.txt index 7914c12d429ef95..d6ea1e907e7f09b 100644 --- a/clang/lib/Analysis/CMakeLists.txt +++ b/clang/lib/Analysis/CMakeLists.txt @@ -16,6 +16,7 @@ add_clang_library(clangAnalysis ConstructionContext.cpp Consumed.cpp CodeInjector.cpp + DanglingReference.cpp Dominators.cpp ExprMutationAnalyzer.cpp IntervalPartition.cpp diff --git a/clang/lib/Analysis/DanglingReference.cpp b/clang/lib/Analysis/DanglingReference.cpp new file mode 100644 index 000..2602cc597a36b86 --- /dev/null +++ b/clang/lib/Analysis/DanglingReference.cpp @@ -0,0 +1,351 @@ +#include "clang/Analysis/Analyses/DanglingReference.h" +#include "clang/AST/Attrs.inc" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclVisitor.h" +#include "clang/AST/Expr.h" +#include "clang/AST/ExprCXX.h" +#include "clang/AST/Stmt.h" +#include "clang/AST/StmtVisitor.h" +#include "clang/AST/Type.h" +#include "clang/Analysis/CFG.h" +#include "clang/Basic/DiagnosticSema.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/DenseSet.h" +#include "llvm/Support/raw_ostream.h" +#include + +namespace clang { +namespace { + +template static bool isRecordWithAttr(QualType Type) { + auto *RD = Type->getAsCXXRecordDecl(); + if (!RD) +return false; + bool Result = RD->hasAttr(); + + if (au
[clang] [clang][bytecode][NFC] Only get expr when checking for UB (PR #125397)
https://github.com/tbaederr created https://github.com/llvm/llvm-project/pull/125397 The Expr and its Type were unused otherwise. >From 928c97292c3528204b56d6d6d2e1524bedf9ca69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= Date: Sun, 2 Feb 2025 11:59:19 +0100 Subject: [PATCH] [clang][bytecode][NFC] Only get expr when checking for UB The Expr and its Type were unused otherwise. --- clang/lib/AST/ByteCode/Interp.h | 22 +- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h index 2a39e3932077f5..9f029adc708390 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -379,15 +379,14 @@ bool AddSubMulHelper(InterpState &S, CodePtr OpPC, unsigned Bits, const T &LHS, APSInt Value = OpAP()(LHS.toAPSInt(Bits), RHS.toAPSInt(Bits)); // Report undefined behaviour, stopping if required. - const Expr *E = S.Current->getExpr(OpPC); - QualType Type = E->getType(); if (S.checkingForUndefinedBehavior()) { +const Expr *E = S.Current->getExpr(OpPC); +QualType Type = E->getType(); SmallString<32> Trunc; Value.trunc(Result.bitWidth()) .toString(Trunc, 10, Result.isSigned(), /*formatAsCLiteral=*/false, /*UpperCase=*/true, /*InsertSeparators=*/true); -auto Loc = E->getExprLoc(); -S.report(Loc, diag::warn_integer_constant_overflow) +S.report(E->getExprLoc(), diag::warn_integer_constant_overflow) << Trunc << Type << E->getSourceRange(); } @@ -737,16 +736,14 @@ bool Neg(InterpState &S, CodePtr OpPC) { S.Stk.push(Result); APSInt NegatedValue = -Value.toAPSInt(Value.bitWidth() + 1); - const Expr *E = S.Current->getExpr(OpPC); - QualType Type = E->getType(); - if (S.checkingForUndefinedBehavior()) { +const Expr *E = S.Current->getExpr(OpPC); +QualType Type = E->getType(); SmallString<32> Trunc; NegatedValue.trunc(Result.bitWidth()) .toString(Trunc, 10, Result.isSigned(), /*formatAsCLiteral=*/false, /*UpperCase=*/true, /*InsertSeparators=*/true); -auto Loc = E->getExprLoc(); -S.report(Loc, diag::warn_integer_constant_overflow) +S.report(E->getExprLoc(), diag::warn_integer_constant_overflow) << Trunc << Type << E->getSourceRange(); return true; } @@ -800,15 +797,14 @@ bool IncDecHelper(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { APResult = --Value.toAPSInt(Bits); // Report undefined behaviour, stopping if required. - const Expr *E = S.Current->getExpr(OpPC); - QualType Type = E->getType(); if (S.checkingForUndefinedBehavior()) { +const Expr *E = S.Current->getExpr(OpPC); +QualType Type = E->getType(); SmallString<32> Trunc; APResult.trunc(Result.bitWidth()) .toString(Trunc, 10, Result.isSigned(), /*formatAsCLiteral=*/false, /*UpperCase=*/true, /*InsertSeparators=*/true); -auto Loc = E->getExprLoc(); -S.report(Loc, diag::warn_integer_constant_overflow) +S.report(E->getExprLoc(), diag::warn_integer_constant_overflow) << Trunc << Type << E->getSourceRange(); return true; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][bytecode][NFC] Only get expr when checking for UB (PR #125397)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) Changes The Expr and its Type were unused otherwise. --- Full diff: https://github.com/llvm/llvm-project/pull/125397.diff 1 Files Affected: - (modified) clang/lib/AST/ByteCode/Interp.h (+9-13) ``diff diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h index 2a39e3932077f57..9f029adc7083908 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -379,15 +379,14 @@ bool AddSubMulHelper(InterpState &S, CodePtr OpPC, unsigned Bits, const T &LHS, APSInt Value = OpAP()(LHS.toAPSInt(Bits), RHS.toAPSInt(Bits)); // Report undefined behaviour, stopping if required. - const Expr *E = S.Current->getExpr(OpPC); - QualType Type = E->getType(); if (S.checkingForUndefinedBehavior()) { +const Expr *E = S.Current->getExpr(OpPC); +QualType Type = E->getType(); SmallString<32> Trunc; Value.trunc(Result.bitWidth()) .toString(Trunc, 10, Result.isSigned(), /*formatAsCLiteral=*/false, /*UpperCase=*/true, /*InsertSeparators=*/true); -auto Loc = E->getExprLoc(); -S.report(Loc, diag::warn_integer_constant_overflow) +S.report(E->getExprLoc(), diag::warn_integer_constant_overflow) << Trunc << Type << E->getSourceRange(); } @@ -737,16 +736,14 @@ bool Neg(InterpState &S, CodePtr OpPC) { S.Stk.push(Result); APSInt NegatedValue = -Value.toAPSInt(Value.bitWidth() + 1); - const Expr *E = S.Current->getExpr(OpPC); - QualType Type = E->getType(); - if (S.checkingForUndefinedBehavior()) { +const Expr *E = S.Current->getExpr(OpPC); +QualType Type = E->getType(); SmallString<32> Trunc; NegatedValue.trunc(Result.bitWidth()) .toString(Trunc, 10, Result.isSigned(), /*formatAsCLiteral=*/false, /*UpperCase=*/true, /*InsertSeparators=*/true); -auto Loc = E->getExprLoc(); -S.report(Loc, diag::warn_integer_constant_overflow) +S.report(E->getExprLoc(), diag::warn_integer_constant_overflow) << Trunc << Type << E->getSourceRange(); return true; } @@ -800,15 +797,14 @@ bool IncDecHelper(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { APResult = --Value.toAPSInt(Bits); // Report undefined behaviour, stopping if required. - const Expr *E = S.Current->getExpr(OpPC); - QualType Type = E->getType(); if (S.checkingForUndefinedBehavior()) { +const Expr *E = S.Current->getExpr(OpPC); +QualType Type = E->getType(); SmallString<32> Trunc; APResult.trunc(Result.bitWidth()) .toString(Trunc, 10, Result.isSigned(), /*formatAsCLiteral=*/false, /*UpperCase=*/true, /*InsertSeparators=*/true); -auto Loc = E->getExprLoc(); -S.report(Loc, diag::warn_integer_constant_overflow) +S.report(E->getExprLoc(), diag::warn_integer_constant_overflow) << Trunc << Type << E->getSourceRange(); return true; } `` https://github.com/llvm/llvm-project/pull/125397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] cf893ba - [clang][bytecode][NFC] Add a FunctionKind enum (#125391)
Author: Timm Baeder Date: 2025-02-02T12:09:30+01:00 New Revision: cf893baf02ffe2dc1a65a6bb9334332a83a453ea URL: https://github.com/llvm/llvm-project/commit/cf893baf02ffe2dc1a65a6bb9334332a83a453ea DIFF: https://github.com/llvm/llvm-project/commit/cf893baf02ffe2dc1a65a6bb9334332a83a453ea.diff LOG: [clang][bytecode][NFC] Add a FunctionKind enum (#125391) Some function types are special to us, so add an enum and determinte the function kind once when creating the function, instead of looking at the Decl every time we need the information. Added: Modified: clang/lib/AST/ByteCode/ByteCodeEmitter.cpp clang/lib/AST/ByteCode/Function.cpp clang/lib/AST/ByteCode/Function.h Removed: diff --git a/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp b/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp index 19e2416c4c9422..5bd1b73133d654 100644 --- a/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp +++ b/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp @@ -135,11 +135,9 @@ Function *ByteCodeEmitter::compileFunc(const FunctionDecl *FuncDecl) { // Create a handle over the emitted code. Function *Func = P.getFunction(FuncDecl); if (!Func) { -unsigned BuiltinID = FuncDecl->getBuiltinID(); -Func = -P.createFunction(FuncDecl, ParamOffset, std::move(ParamTypes), - std::move(ParamDescriptors), std::move(ParamOffsets), - HasThisPointer, HasRVO, BuiltinID); +Func = P.createFunction(FuncDecl, ParamOffset, std::move(ParamTypes), +std::move(ParamDescriptors), +std::move(ParamOffsets), HasThisPointer, HasRVO); } assert(Func); @@ -212,8 +210,7 @@ Function *ByteCodeEmitter::compileObjCBlock(const BlockExpr *BE) { Function *Func = P.createFunction(BE, ParamOffset, std::move(ParamTypes), std::move(ParamDescriptors), std::move(ParamOffsets), - /*HasThisPointer=*/false, /*HasRVO=*/false, - /*IsUnevaluatedBuiltin=*/false); + /*HasThisPointer=*/false, /*HasRVO=*/false); assert(Func); Func->setDefined(true); diff --git a/clang/lib/AST/ByteCode/Function.cpp b/clang/lib/AST/ByteCode/Function.cpp index 896a4fb3f9469a..6b892dfd616c17 100644 --- a/clang/lib/AST/ByteCode/Function.cpp +++ b/clang/lib/AST/ByteCode/Function.cpp @@ -19,12 +19,28 @@ Function::Function(Program &P, FunctionDeclTy Source, unsigned ArgSize, llvm::SmallVectorImpl &&ParamTypes, llvm::DenseMap &&Params, llvm::SmallVectorImpl &&ParamOffsets, - bool HasThisPointer, bool HasRVO, unsigned BuiltinID) -: P(P), Source(Source), ArgSize(ArgSize), ParamTypes(std::move(ParamTypes)), - Params(std::move(Params)), ParamOffsets(std::move(ParamOffsets)), - HasThisPointer(HasThisPointer), HasRVO(HasRVO), BuiltinID(BuiltinID) { - if (const auto *F = Source.dyn_cast()) + bool HasThisPointer, bool HasRVO) +: P(P), Kind(FunctionKind::Normal), Source(Source), ArgSize(ArgSize), + ParamTypes(std::move(ParamTypes)), Params(std::move(Params)), + ParamOffsets(std::move(ParamOffsets)), HasThisPointer(HasThisPointer), + HasRVO(HasRVO) { + if (const auto *F = dyn_cast(Source)) { Variadic = F->isVariadic(); +BuiltinID = F->getBuiltinID(); +if (const auto *CD = dyn_cast(F)) { + Virtual = CD->isVirtual(); + Kind = FunctionKind::Ctor; +} else if (const auto *CD = dyn_cast(F)) { + Virtual = CD->isVirtual(); + Kind = FunctionKind::Dtor; +} else if (const auto *MD = dyn_cast(F)) { + Virtual = MD->isVirtual(); + if (MD->isLambdaStaticInvoker()) +Kind = FunctionKind::LambdaStaticInvoker; + else if (clang::isLambdaCallOperator(F)) +Kind = FunctionKind::LambdaCallOperator; +} + } } Function::ParamDescriptor Function::getParamDescriptor(unsigned Offset) const { @@ -45,13 +61,6 @@ SourceInfo Function::getSource(CodePtr PC) const { return It->second; } -bool Function::isVirtual() const { - if (const auto *M = dyn_cast_if_present( - Source.dyn_cast())) -return M->isVirtual(); - return false; -} - /// Unevaluated builtins don't get their arguments put on the stack /// automatically. They instead operate on the AST of their Call /// Expression. diff --git a/clang/lib/AST/ByteCode/Function.h b/clang/lib/AST/ByteCode/Function.h index 409a80f59f1e94..2d3421e5e61295 100644 --- a/clang/lib/AST/ByteCode/Function.h +++ b/clang/lib/AST/ByteCode/Function.h @@ -80,6 +80,13 @@ using FunctionDeclTy = /// class Function final { public: + enum class FunctionKind { +Normal, +Ctor, +Dtor, +LambdaStaticInvoker, +LambdaCallOperator, + }; using ParamDescriptor = std::pair; /// Returns the size of the function's local stack. @@ -141,43
[clang] [clang][bytecode][NFC] Only get expr when checking for UB (PR #125397)
llvm-ci wrote: LLVM Buildbot has detected a new failure on builder `openmp-offload-libc-amdgpu-runtime` running on `omp-vega20-1` while building `clang` at step 7 "Add check check-offload". Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/12884 Here is the relevant piece of the build log for the reference ``` Step 7 (Add check check-offload) failure: test (failure) TEST 'libomptarget :: amdgcn-amd-amdhsa :: mapping/data_member_ref.cpp' FAILED Exit Code: 2 Command Output (stdout): -- # RUN: at line 1 /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/clang++ -fopenmp-I /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test -I /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -nogpulib -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib -fopenmp-targets=amdgcn-amd-amdhsa /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/mapping/data_member_ref.cpp -o /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/mapping/Output/data_member_ref.cpp.tmp -Xoffload-linker -lc -Xoffload-linker -lm /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib/libomptarget.devicertl.a && /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/mapping/Output/data_member_ref.cpp.tmp | /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/mapping/data_member_ref.cpp # executed command: /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/clang++ -fopenmp -I /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test -I /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -nogpulib -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib -fopenmp-targets=amdgcn-amd-amdhsa /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/mapping/data_member_ref.cpp -o /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/mapping/Output/data_member_ref.cpp.tmp -Xoffload-linker -lc -Xoffload-linker -lm /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib/libomptarget.devicertl.a # executed command: /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/mapping/Output/data_member_ref.cpp.tmp # note: command had no output on stdout or stderr # error: command failed with exit status: -11 # executed command: /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/mapping/data_member_ref.cpp # .---command stderr # | FileCheck error: '' is empty. # | FileCheck command line: /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/mapping/data_member_ref.cpp # `- # error: command failed with exit status: 2 -- ``` https://github.com/llvm/llvm-project/pull/125397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][bytecode][NFC] Only get expr when checking for UB (PR #125397)
https://github.com/tbaederr closed https://github.com/llvm/llvm-project/pull/125397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 642e84f - [clang][bytecode][NFC] Only get expr when checking for UB (#125397)
Author: Timm Baeder Date: 2025-02-02T12:25:08+01:00 New Revision: 642e84f0012b6e3a0e4f187bad5ee1775c3d623a URL: https://github.com/llvm/llvm-project/commit/642e84f0012b6e3a0e4f187bad5ee1775c3d623a DIFF: https://github.com/llvm/llvm-project/commit/642e84f0012b6e3a0e4f187bad5ee1775c3d623a.diff LOG: [clang][bytecode][NFC] Only get expr when checking for UB (#125397) The Expr and its Type were unused otherwise. Added: Modified: clang/lib/AST/ByteCode/Interp.h Removed: diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h index 2a39e3932077f5..9f029adc708390 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -379,15 +379,14 @@ bool AddSubMulHelper(InterpState &S, CodePtr OpPC, unsigned Bits, const T &LHS, APSInt Value = OpAP()(LHS.toAPSInt(Bits), RHS.toAPSInt(Bits)); // Report undefined behaviour, stopping if required. - const Expr *E = S.Current->getExpr(OpPC); - QualType Type = E->getType(); if (S.checkingForUndefinedBehavior()) { +const Expr *E = S.Current->getExpr(OpPC); +QualType Type = E->getType(); SmallString<32> Trunc; Value.trunc(Result.bitWidth()) .toString(Trunc, 10, Result.isSigned(), /*formatAsCLiteral=*/false, /*UpperCase=*/true, /*InsertSeparators=*/true); -auto Loc = E->getExprLoc(); -S.report(Loc, diag::warn_integer_constant_overflow) +S.report(E->getExprLoc(), diag::warn_integer_constant_overflow) << Trunc << Type << E->getSourceRange(); } @@ -737,16 +736,14 @@ bool Neg(InterpState &S, CodePtr OpPC) { S.Stk.push(Result); APSInt NegatedValue = -Value.toAPSInt(Value.bitWidth() + 1); - const Expr *E = S.Current->getExpr(OpPC); - QualType Type = E->getType(); - if (S.checkingForUndefinedBehavior()) { +const Expr *E = S.Current->getExpr(OpPC); +QualType Type = E->getType(); SmallString<32> Trunc; NegatedValue.trunc(Result.bitWidth()) .toString(Trunc, 10, Result.isSigned(), /*formatAsCLiteral=*/false, /*UpperCase=*/true, /*InsertSeparators=*/true); -auto Loc = E->getExprLoc(); -S.report(Loc, diag::warn_integer_constant_overflow) +S.report(E->getExprLoc(), diag::warn_integer_constant_overflow) << Trunc << Type << E->getSourceRange(); return true; } @@ -800,15 +797,14 @@ bool IncDecHelper(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { APResult = --Value.toAPSInt(Bits); // Report undefined behaviour, stopping if required. - const Expr *E = S.Current->getExpr(OpPC); - QualType Type = E->getType(); if (S.checkingForUndefinedBehavior()) { +const Expr *E = S.Current->getExpr(OpPC); +QualType Type = E->getType(); SmallString<32> Trunc; APResult.trunc(Result.bitWidth()) .toString(Trunc, 10, Result.isSigned(), /*formatAsCLiteral=*/false, /*UpperCase=*/true, /*InsertSeparators=*/true); -auto Loc = E->getExprLoc(); -S.report(Loc, diag::warn_integer_constant_overflow) +S.report(E->getExprLoc(), diag::warn_integer_constant_overflow) << Trunc << Type << E->getSourceRange(); return true; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] Silence -Wunused-parameter warnings in Unwind-wasm.c (PR #125412)
llvmbot wrote: @llvm/pr-subscribers-libunwind Author: Yuriy Chernyshov (georgthegreat) Changes --- Full diff: https://github.com/llvm/llvm-project/pull/125412.diff 1 Files Affected: - (modified) libunwind/src/Unwind-wasm.c (+3-3) ``diff diff --git a/libunwind/src/Unwind-wasm.c b/libunwind/src/Unwind-wasm.c index b18b32c5d17847..fe016b8f5923ad 100644 --- a/libunwind/src/Unwind-wasm.c +++ b/libunwind/src/Unwind-wasm.c @@ -102,8 +102,8 @@ _LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) { } /// Not used in Wasm. -_LIBUNWIND_EXPORT void _Unwind_SetIP(struct _Unwind_Context *context, - uintptr_t value) {} +_LIBUNWIND_EXPORT void _Unwind_SetIP([[maybe_unused]] struct _Unwind_Context *context, + [[maybe_unused]] uintptr_t value) {} /// Called by personality handler to get LSDA for current frame. _LIBUNWIND_EXPORT uintptr_t @@ -116,7 +116,7 @@ _Unwind_GetLanguageSpecificData(struct _Unwind_Context *context) { /// Not used in Wasm. _LIBUNWIND_EXPORT uintptr_t -_Unwind_GetRegionStart(struct _Unwind_Context *context) { +_Unwind_GetRegionStart([[maybe_unused]] struct _Unwind_Context *context) { return 0; } `` https://github.com/llvm/llvm-project/pull/125412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] Silence -Wunused-parameter warnings in Unwind-wasm.c (PR #125412)
https://github.com/georgthegreat created https://github.com/llvm/llvm-project/pull/125412 None >From f3f30cca500f839a2e6647ea9acf0cea45a4e178 Mon Sep 17 00:00:00 2001 From: Yuriy Chernyshov Date: Sun, 2 Feb 2025 14:35:53 +0100 Subject: [PATCH] Silence -Wunused-parameter warnings in Unwind-wasm.c --- libunwind/src/Unwind-wasm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libunwind/src/Unwind-wasm.c b/libunwind/src/Unwind-wasm.c index b18b32c5d17847..fe016b8f5923ad 100644 --- a/libunwind/src/Unwind-wasm.c +++ b/libunwind/src/Unwind-wasm.c @@ -102,8 +102,8 @@ _LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) { } /// Not used in Wasm. -_LIBUNWIND_EXPORT void _Unwind_SetIP(struct _Unwind_Context *context, - uintptr_t value) {} +_LIBUNWIND_EXPORT void _Unwind_SetIP([[maybe_unused]] struct _Unwind_Context *context, + [[maybe_unused]] uintptr_t value) {} /// Called by personality handler to get LSDA for current frame. _LIBUNWIND_EXPORT uintptr_t @@ -116,7 +116,7 @@ _Unwind_GetLanguageSpecificData(struct _Unwind_Context *context) { /// Not used in Wasm. _LIBUNWIND_EXPORT uintptr_t -_Unwind_GetRegionStart(struct _Unwind_Context *context) { +_Unwind_GetRegionStart([[maybe_unused]] struct _Unwind_Context *context) { return 0; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] Silence -Wunused-parameter warnings in Unwind-wasm.c (PR #125412)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff 642e84f0012b6e3a0e4f187bad5ee1775c3d623a f3f30cca500f839a2e6647ea9acf0cea45a4e178 --extensions c -- libunwind/src/Unwind-wasm.c `` View the diff from clang-format here. ``diff diff --git a/libunwind/src/Unwind-wasm.c b/libunwind/src/Unwind-wasm.c index fe016b8f59..4b893c8b4f 100644 --- a/libunwind/src/Unwind-wasm.c +++ b/libunwind/src/Unwind-wasm.c @@ -102,8 +102,9 @@ _LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) { } /// Not used in Wasm. -_LIBUNWIND_EXPORT void _Unwind_SetIP([[maybe_unused]] struct _Unwind_Context *context, - [[maybe_unused]] uintptr_t value) {} +_LIBUNWIND_EXPORT void +_Unwind_SetIP([[maybe_unused]] struct _Unwind_Context *context, + [[maybe_unused]] uintptr_t value) {} /// Called by personality handler to get LSDA for current frame. _LIBUNWIND_EXPORT uintptr_t `` https://github.com/llvm/llvm-project/pull/125412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] fix P3310 overload resolution flag propagation (PR #125372)
https://github.com/mizvekov edited https://github.com/llvm/llvm-project/pull/125372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Replace /* ... */ single-line comments with // ... comments (PR #124319)
https://github.com/EugeneZelenko commented: Please rebase from `main`. Release Notes were changed significantly after 20 branch. https://github.com/llvm/llvm-project/pull/124319 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] fix P3310 overload resolution flag propagation (PR #125372)
https://github.com/mizvekov edited https://github.com/llvm/llvm-project/pull/125372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] fix P3310 overload resolution flag propagation (PR #125372)
https://github.com/mizvekov updated https://github.com/llvm/llvm-project/pull/125372 >From a624adeb9a0b0af5c9370c6a97b2e2c874cdc066 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Fri, 31 Jan 2025 20:41:39 -0300 Subject: [PATCH] [clang] fix P3310 overload resolution flag propagation Class templates might be only instantiated when they are required to be complete, but checking the template args against the primary template is immediate. This result is cached so that later when the class is instantiated, checking against the primary template is not repeated. The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon checking against the primary template, so it needs to be cached in the specialziation as well. This fixes a bug which has not been in any release, so there are no release notes. Fixes #125290 --- clang/include/clang/AST/DeclTemplate.h| 16 ++- clang/include/clang/Sema/Sema.h | 4 +- clang/lib/AST/ASTImporter.cpp | 6 +-- clang/lib/AST/DeclTemplate.cpp| 47 ++- clang/lib/AST/TextNodeDumper.cpp | 5 +- clang/lib/Sema/SemaTemplate.cpp | 8 ++-- clang/lib/Sema/SemaTemplateDeduction.cpp | 2 - .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 2 +- clang/lib/Sema/SemaType.cpp | 3 +- clang/lib/Serialization/ASTReaderDecl.cpp | 1 + clang/lib/Serialization/ASTWriterDecl.cpp | 1 + clang/test/AST/ast-dump-templates.cpp | 19 +++- clang/test/SemaTemplate/cwg2398.cpp | 17 +++ 13 files changed, 91 insertions(+), 40 deletions(-) diff --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h index 9ecff2c898acd5..03c43765206b18 100644 --- a/clang/include/clang/AST/DeclTemplate.h +++ b/clang/include/clang/AST/DeclTemplate.h @@ -1841,15 +1841,23 @@ class ClassTemplateSpecializationDecl : public CXXRecordDecl, LLVM_PREFERRED_TYPE(TemplateSpecializationKind) unsigned SpecializationKind : 3; + /// Indicate that we have matched a parameter pack with a non pack + /// argument, when the opposite match is also allowed (strict pack match). + /// This needs to be cached as deduction is performed during declaration, + /// and we need the information to be preserved so that it is consistent + /// during instantiation. + bool MatchedPackOnParmToNonPackOnArg : 1; + protected: ClassTemplateSpecializationDecl(ASTContext &Context, Kind DK, TagKind TK, DeclContext *DC, SourceLocation StartLoc, SourceLocation IdLoc, ClassTemplateDecl *SpecializedTemplate, ArrayRef Args, + bool MatchedPackOnParmToNonPackOnArg, ClassTemplateSpecializationDecl *PrevDecl); - explicit ClassTemplateSpecializationDecl(ASTContext &C, Kind DK); + ClassTemplateSpecializationDecl(ASTContext &C, Kind DK); public: friend class ASTDeclReader; @@ -1859,7 +1867,7 @@ class ClassTemplateSpecializationDecl : public CXXRecordDecl, Create(ASTContext &Context, TagKind TK, DeclContext *DC, SourceLocation StartLoc, SourceLocation IdLoc, ClassTemplateDecl *SpecializedTemplate, - ArrayRef Args, + ArrayRef Args, bool MatchedPackOnParmToNonPackOnArg, ClassTemplateSpecializationDecl *PrevDecl); static ClassTemplateSpecializationDecl *CreateDeserialized(ASTContext &C, GlobalDeclID ID); @@ -1930,6 +1938,10 @@ class ClassTemplateSpecializationDecl : public CXXRecordDecl, SpecializationKind = TSK; } + bool hasMatchedPackOnParmToNonPackOnArg() const { +return MatchedPackOnParmToNonPackOnArg; + } + /// Get the point of instantiation (if any), or null if none. SourceLocation getPointOfInstantiation() const { return PointOfInstantiation; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 472a0e25adc975..79bf6c04ee4969 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -13493,8 +13493,8 @@ class Sema final : public SemaBase { bool InstantiateClassTemplateSpecialization( SourceLocation PointOfInstantiation, ClassTemplateSpecializationDecl *ClassTemplateSpec, - TemplateSpecializationKind TSK, bool Complain = true, - bool PrimaryHasMatchedPackOnParmToNonPackOnArg = false); + TemplateSpecializationKind TSK, bool Complain, + bool PrimaryHasMatchedPackOnParmToNonPackOnArg); /// Instantiates the definitions of all of the member /// of the given class, which is an instantiation of a class template diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index c9f2f905d2134c..1057f09deda073 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -6
[clang-tools-extra] [clang-tidy] Replace /* ... */ single-line comments with // ... comments (PR #124319)
@@ -0,0 +1,162 @@ +//===--- UseCppStyleCommentsCheck.cpp - clang-tidy-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "UseCppStyleCommentsCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" +#include "clang/Lex/Preprocessor.h" +#include + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { +class UseCppStyleCommentsCheck::CStyleCommentHandler : public CommentHandler { +public: + CStyleCommentHandler(UseCppStyleCommentsCheck &Check, bool ExcludeDoxygen) + : Check(Check), ExcludeDoxygen(ExcludeDoxygen), +CStyleCommentMatch( +"^[ \t]*/\\*+[ \t\r\n]*(.*[ \t\r\n]*)*[ \t\r\n]*\\*+/[ \t\r\n]*$") { + } + + void setExcludeDoxygen(bool Exclude) { ExcludeDoxygen = Exclude; } + + bool isExcludeDoxygen() const { return ExcludeDoxygen; } + + std::string convertToCppStyleComment(const SourceManager &SM, + const SourceRange &Range) { +const StringRef CommentText = Lexer::getSourceText( +CharSourceRange::getTokenRange(Range), SM, LangOptions()); + +std::string InnerText = CommentText.str(); +InnerText.erase(0, 2); +InnerText.erase(InnerText.size() - 2, 2); + +std::string Result; +std::istringstream Stream(InnerText); +std::string Line; + +if (std::getline(Stream, Line)) { + const size_t StartPos = Line.find_first_not_of(" \t"); + if (StartPos != std::string::npos) { +Line = Line.substr(StartPos); + } else { +Line.clear(); + } + Result += "// " + Line; +} + +while (std::getline(Stream, Line)) { + const size_t StartPos = Line.find_first_not_of(" \t"); + if (StartPos != std::string::npos) { +Line = Line.substr(StartPos); + } else { +Line.clear(); + } + Result += "\n// " + Line; +} +return Result; + } + + bool isDoxygenStyleComment(const StringRef &Text) { +StringRef Trimmed = Text.ltrim(); +return Trimmed.starts_with("/**") || Trimmed.starts_with("/*!") || + Trimmed.starts_with("///") || Trimmed.starts_with("//!") || + (Trimmed.starts_with("/*") && +Trimmed.drop_front(2).starts_with("*")); + } + + bool CheckForInlineComments(Preprocessor &PP, SourceRange Range) { +const SourceManager &SM = PP.getSourceManager(); +const SourceLocation CommentStart = Range.getBegin(); +const SourceLocation CommentEnd = Range.getEnd(); + +unsigned StartLine = SM.getSpellingLineNumber(CommentStart); +unsigned EndLine = SM.getSpellingLineNumber(CommentEnd); + +const StringRef Text = Lexer::getSourceText( +CharSourceRange::getCharRange(Range), SM, PP.getLangOpts()); + +if (StartLine == EndLine) { + const SourceLocation LineBegin = + SM.translateLineCol(SM.getFileID(CommentStart), StartLine, 1); + const SourceLocation LineEnd = + SM.translateLineCol(SM.getFileID(CommentEnd), EndLine, + std::numeric_limits::max()); + const StringRef LineContent = Lexer::getSourceText( + CharSourceRange::getCharRange(LineBegin, LineEnd), SM, + PP.getLangOpts()); + const size_t CommentStartOffset = + SM.getSpellingColumnNumber(CommentStart) - 1; + const StringRef AfterComment = + LineContent.drop_front(CommentStartOffset + Text.size()); + + if (!AfterComment.trim().empty()) { +return true; + } +} +return false; + } + + bool HandleComment(Preprocessor &PP, SourceRange Range) override { +const SourceManager &SM = PP.getSourceManager(); + +if (Range.getBegin().isMacroID() || SM.isInSystemHeader(Range.getBegin())) { + return false; +} + +const StringRef Text = Lexer::getSourceText( +CharSourceRange::getCharRange(Range), SM, PP.getLangOpts()); + +if (ExcludeDoxygen && isDoxygenStyleComment(Text)) { + return false; +} + +SmallVector Matches; +if (!CStyleCommentMatch.match(Text, &Matches)) { + return false; +} + +if (CheckForInlineComments(PP, Range)) { + return false; +} + +Check.diag( +Range.getBegin(), +"use C++ style comments '//' instead of C style comments '/*...*/'") +<< clang::FixItHint::CreateReplacement( + Range, convertToCppStyleComment(SM, Range)); + +return false; + } + +private: + UseCppStyleCommentsCheck &Check; + bool ExcludeDoxygen; + llvm::Regex CStyleCommentMatch; +}; + +UseCppStyleCommentsCheck::UseCppStyleCommentsCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + H
[clang-tools-extra] [clang-tidy] Replace /* ... */ single-line comments with // ... comments (PR #124319)
4m4n-x-B4w4ne wrote: > As @PiotrZSL mentioned in the issue, adding an option called something like > `ExcludeDoxygenStyleComments` would be very helpful (or even necessary) > before releasing this check. Otherwise, it could create a lot of trouble for > projects that use doxygen for their documentation (including LLVM itself). > Doxygen comments typically look like this: > > ```c++ > /** > * ... text ... > */ > void foo() > ``` > > With this option enabled, I think we should exclude the following cases: > > ```c++ > /** > * ... text ... > */ > > /*! > * ... text ... > */ > > /*** > * ... text > ***/ > ``` > > Basically, they are comments that start with `/*` followed by more than one > `*` or start with `/*!` More detailed documentation can be found in > https://www.doxygen.nl/manual/docblocks.html. > > This option can also be tested on LLVM codebase for false positives. I have done this can you please check. https://github.com/llvm/llvm-project/pull/124319 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] fix P3310 overload resolution flag propagation (PR #125372)
@@ -1841,15 +1841,21 @@ class ClassTemplateSpecializationDecl : public CXXRecordDecl, LLVM_PREFERRED_TYPE(TemplateSpecializationKind) unsigned SpecializationKind : 3; + /// When matching the primary template, have we matched any packs on the + /// parameter side, versus any non-packs on the argument side, in a context + /// where the opposite matching is also allowed? + bool MatchedPackOnParmToNonPackOnArg : 1; mizvekov wrote: Done as a follow up in https://github.com/llvm/llvm-project/pull/125418 https://github.com/llvm/llvm-project/pull/125372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] fix P3310 overload resolution flag propagation (PR #125372)
https://github.com/mizvekov updated https://github.com/llvm/llvm-project/pull/125372 >From 36cab9c0ae9bd24b86dcfa0c260f1b0701d7468e Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Fri, 31 Jan 2025 20:41:39 -0300 Subject: [PATCH] [clang] fix P3310 overload resolution flag propagation Class templates might be only instantiated when they are required to be complete, but checking the template args against the primary template is immediate. This result is cached so that later when the class is instantiated, checking against the primary template is not repeated. The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon checking against the primary template, so it needs to be cached in the specialziation as well. This fixes a bug which has not been in any release, so there are no release notes. Fixes #125290 --- clang/include/clang/AST/DeclTemplate.h| 16 ++- clang/include/clang/Sema/Sema.h | 4 +- clang/lib/AST/ASTImporter.cpp | 6 +-- clang/lib/AST/DeclTemplate.cpp| 47 ++- clang/lib/AST/TextNodeDumper.cpp | 5 +- clang/lib/Sema/SemaTemplate.cpp | 8 ++-- clang/lib/Sema/SemaTemplateDeduction.cpp | 2 - .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 2 +- clang/lib/Sema/SemaType.cpp | 3 +- clang/lib/Serialization/ASTReaderDecl.cpp | 1 + clang/lib/Serialization/ASTWriterDecl.cpp | 1 + clang/test/AST/ast-dump-templates.cpp | 19 +++- clang/test/SemaTemplate/cwg2398.cpp | 17 +++ 13 files changed, 91 insertions(+), 40 deletions(-) diff --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h index 9ecff2c898acd5..03c43765206b18 100644 --- a/clang/include/clang/AST/DeclTemplate.h +++ b/clang/include/clang/AST/DeclTemplate.h @@ -1841,15 +1841,23 @@ class ClassTemplateSpecializationDecl : public CXXRecordDecl, LLVM_PREFERRED_TYPE(TemplateSpecializationKind) unsigned SpecializationKind : 3; + /// Indicate that we have matched a parameter pack with a non pack + /// argument, when the opposite match is also allowed (strict pack match). + /// This needs to be cached as deduction is performed during declaration, + /// and we need the information to be preserved so that it is consistent + /// during instantiation. + bool MatchedPackOnParmToNonPackOnArg : 1; + protected: ClassTemplateSpecializationDecl(ASTContext &Context, Kind DK, TagKind TK, DeclContext *DC, SourceLocation StartLoc, SourceLocation IdLoc, ClassTemplateDecl *SpecializedTemplate, ArrayRef Args, + bool MatchedPackOnParmToNonPackOnArg, ClassTemplateSpecializationDecl *PrevDecl); - explicit ClassTemplateSpecializationDecl(ASTContext &C, Kind DK); + ClassTemplateSpecializationDecl(ASTContext &C, Kind DK); public: friend class ASTDeclReader; @@ -1859,7 +1867,7 @@ class ClassTemplateSpecializationDecl : public CXXRecordDecl, Create(ASTContext &Context, TagKind TK, DeclContext *DC, SourceLocation StartLoc, SourceLocation IdLoc, ClassTemplateDecl *SpecializedTemplate, - ArrayRef Args, + ArrayRef Args, bool MatchedPackOnParmToNonPackOnArg, ClassTemplateSpecializationDecl *PrevDecl); static ClassTemplateSpecializationDecl *CreateDeserialized(ASTContext &C, GlobalDeclID ID); @@ -1930,6 +1938,10 @@ class ClassTemplateSpecializationDecl : public CXXRecordDecl, SpecializationKind = TSK; } + bool hasMatchedPackOnParmToNonPackOnArg() const { +return MatchedPackOnParmToNonPackOnArg; + } + /// Get the point of instantiation (if any), or null if none. SourceLocation getPointOfInstantiation() const { return PointOfInstantiation; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 472a0e25adc975..79bf6c04ee4969 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -13493,8 +13493,8 @@ class Sema final : public SemaBase { bool InstantiateClassTemplateSpecialization( SourceLocation PointOfInstantiation, ClassTemplateSpecializationDecl *ClassTemplateSpec, - TemplateSpecializationKind TSK, bool Complain = true, - bool PrimaryHasMatchedPackOnParmToNonPackOnArg = false); + TemplateSpecializationKind TSK, bool Complain, + bool PrimaryHasMatchedPackOnParmToNonPackOnArg); /// Instantiates the definitions of all of the member /// of the given class, which is an instantiation of a class template diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index c9f2f905d2134c..1057f09deda073 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -6
[libunwind] Silence -Wunused-parameter warnings in Unwind-wasm.c (PR #125412)
https://github.com/georgthegreat updated https://github.com/llvm/llvm-project/pull/125412 >From 69307d52fc749c847da74e98624ce1c39f2fe2d9 Mon Sep 17 00:00:00 2001 From: Yuriy Chernyshov Date: Sun, 2 Feb 2025 14:35:53 +0100 Subject: [PATCH] Silence -Wunused-parameter warnings in Unwind-wasm.c --- libunwind/src/Unwind-wasm.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libunwind/src/Unwind-wasm.c b/libunwind/src/Unwind-wasm.c index b18b32c5d17847a..4b893c8b4f623e3 100644 --- a/libunwind/src/Unwind-wasm.c +++ b/libunwind/src/Unwind-wasm.c @@ -102,8 +102,9 @@ _LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) { } /// Not used in Wasm. -_LIBUNWIND_EXPORT void _Unwind_SetIP(struct _Unwind_Context *context, - uintptr_t value) {} +_LIBUNWIND_EXPORT void +_Unwind_SetIP([[maybe_unused]] struct _Unwind_Context *context, + [[maybe_unused]] uintptr_t value) {} /// Called by personality handler to get LSDA for current frame. _LIBUNWIND_EXPORT uintptr_t @@ -116,7 +117,7 @@ _Unwind_GetLanguageSpecificData(struct _Unwind_Context *context) { /// Not used in Wasm. _LIBUNWIND_EXPORT uintptr_t -_Unwind_GetRegionStart(struct _Unwind_Context *context) { +_Unwind_GetRegionStart([[maybe_unused]] struct _Unwind_Context *context) { return 0; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add BreakBeforeTemplateClose option (PR #118046)
@@ -11220,6 +11220,333 @@ TEST_F(FormatTest, WrapsTemplateDeclarationsWithComments) { Style); } +TEST_F(FormatTest, BreakBeforeTemplateCloser) { + FormatStyle Style = getGoogleStyle(FormatStyle::LK_Cpp); + // Begin with tests covering the case where there is no constraint on the + // column limit. + Style.ColumnLimit = 0; + // When BreakBeforeTemplateCloser is turned off, the line break that it adds + // shall be removed: + verifyFormat("template <\n" + "typename Foo,\n" + "typename Bar>\n" + "void foo() {}", + "template <\n" + "typename Foo,\n" + "typename Bar\n" + ">\n" + "void foo() {}", + Style); + + Style.BreakBeforeTemplateCloser = FormatStyle::BBTCS_BlockIndent; + // BreakBeforeTemplateCloser should NOT force template declarations onto + // multiple lines. + verifyFormat("template \n" + "void foo() {}", + Style); + verifyFormat("template \n" + "void foo() {}", + Style); + // It should allow a line break, even when the typename is short. + // verifyNoChange is needed because the default behavior is one line. + verifyNoChange("template <\n" + "typename Foo\n" + ">\n" + "void foo() {}", + Style); + verifyNoChange("template <\n" + "typename Foo,\n" + "typename Bar\n" + ">\n" + "void foo() {}", + Style); + verifyNoChange("template \n" + "void foo() {}", + Style); + // It should add a line break before > if not already present: + verifyFormat("template <\n" + "typename Foo\n" + ">\n" + "void foo() {}", + "template <\n" + "typename Foo>\n" + "void foo() {}", + Style); + verifyFormat("template <\n" + "typename Foo,\n" + "typename Bar\n" + ">\n" + "void foo() {}", + "template <\n" + "typename Foo,\n" + "typename Bar>\n" + "void foo() {}", + Style); + // When within an indent scope, the > should be placed accordingly: + verifyFormat("struct Baz {\n" + " template <\n" + " typename Foo,\n" + " typename Bar\n" + " >\n" + " void foo() {}\n" + "};", + "struct Baz {\n" + " template <\n" + " typename Foo,\n" + " typename Bar>\n" + " void foo() {}\n" + "};", + Style); + + // Test from issue #80049: + verifyFormat( + "void foo() {\n" + " using type = std::remove_cv_t<\n" + " add_common_cv_reference<\n" + " std::common_type_t, std::decay_t>,\n" + " T0,\n" + " T1\n" + " >\n" + " >;\n" + "}", + "void foo() {\n" + " using type = std::remove_cv_t<\n" + " add_common_cv_reference<\n" + " std::common_type_t, std::decay_t>,\n" + " T0,\n" + " T1>>;\n" + "}", + Style); + + // Test lambda goes to next line: + verifyFormat("void foo() {\n" + " auto lambda = []<\n" + "typename T\n" + ">(T t) {\n" + " };\n" + "}", + "void foo() {\n" + " auto lambda = []<\n" + " typename T>(T t){\n" + " };\n" + "}", + Style); + // With no column limit, two parameters can go on the same line: + verifyFormat("void foo() {\n" + " auto lambda = []<\n" + "typename T, typename Foo\n" + ">(T t) {\n" + " };\n" + "}", + "void foo() {\n" + " auto lambda = []<\n" + " typename T, typename Foo>(T t){\n" + " };\n" + "}", + Style); + // Or on different lines: + verifyFormat("void foo() {\n" + " auto lambda = []<\n" + "typename T,\n" + "typename Foo\n" + ">(T t) {\n" + " };\n" + "}", + "void foo() {\n" + " auto lambda = []<\n" + " typename T,\n" + " typename Foo>(T t){\n" + " };\n" + "}", + Style); + + // Note that this is the same line (no \n): + verifyFormat("void foo() {\n" + " auto lambd
[clang] [clang-format] Add BreakBeforeTemplateClose option (PR #118046)
@@ -11220,6 +11220,333 @@ TEST_F(FormatTest, WrapsTemplateDeclarationsWithComments) { Style); } +TEST_F(FormatTest, BreakBeforeTemplateCloser) { + FormatStyle Style = getGoogleStyle(FormatStyle::LK_Cpp); + // Begin with tests covering the case where there is no constraint on the + // column limit. + Style.ColumnLimit = 0; + // When BreakBeforeTemplateCloser is turned off, the line break that it adds + // shall be removed: + verifyFormat("template <\n" + "typename Foo,\n" + "typename Bar>\n" + "void foo() {}", + "template <\n" + "typename Foo,\n" + "typename Bar\n" + ">\n" + "void foo() {}", + Style); + + Style.BreakBeforeTemplateCloser = FormatStyle::BBTCS_BlockIndent; + // BreakBeforeTemplateCloser should NOT force template declarations onto + // multiple lines. + verifyFormat("template \n" + "void foo() {}", + Style); + verifyFormat("template \n" + "void foo() {}", + Style); + // It should allow a line break, even when the typename is short. + // verifyNoChange is needed because the default behavior is one line. + verifyNoChange("template <\n" + "typename Foo\n" + ">\n" + "void foo() {}", + Style); + verifyNoChange("template <\n" + "typename Foo,\n" + "typename Bar\n" + ">\n" + "void foo() {}", + Style); + verifyNoChange("template \n" + "void foo() {}", + Style); + // It should add a line break before > if not already present: + verifyFormat("template <\n" + "typename Foo\n" + ">\n" + "void foo() {}", + "template <\n" + "typename Foo>\n" + "void foo() {}", + Style); + verifyFormat("template <\n" + "typename Foo,\n" + "typename Bar\n" + ">\n" + "void foo() {}", + "template <\n" + "typename Foo,\n" + "typename Bar>\n" + "void foo() {}", + Style); + // When within an indent scope, the > should be placed accordingly: + verifyFormat("struct Baz {\n" + " template <\n" + " typename Foo,\n" + " typename Bar\n" + " >\n" + " void foo() {}\n" + "};", + "struct Baz {\n" + " template <\n" + " typename Foo,\n" + " typename Bar>\n" + " void foo() {}\n" + "};", + Style); + + // Test from issue #80049: + verifyFormat( + "void foo() {\n" + " using type = std::remove_cv_t<\n" + " add_common_cv_reference<\n" + " std::common_type_t, std::decay_t>,\n" + " T0,\n" + " T1\n" + " >\n" + " >;\n" + "}", + "void foo() {\n" + " using type = std::remove_cv_t<\n" + " add_common_cv_reference<\n" + " std::common_type_t, std::decay_t>,\n" + " T0,\n" + " T1>>;\n" + "}", + Style); + + // Test lambda goes to next line: + verifyFormat("void foo() {\n" + " auto lambda = []<\n" + "typename T\n" + ">(T t) {\n" + " };\n" + "}", + "void foo() {\n" + " auto lambda = []<\n" + " typename T>(T t){\n" + " };\n" + "}", + Style); + // With no column limit, two parameters can go on the same line: + verifyFormat("void foo() {\n" + " auto lambda = []<\n" + "typename T, typename Foo\n" + ">(T t) {\n" + " };\n" + "}", + "void foo() {\n" + " auto lambda = []<\n" + " typename T, typename Foo>(T t){\n" + " };\n" + "}", + Style); + // Or on different lines: + verifyFormat("void foo() {\n" + " auto lambda = []<\n" + "typename T,\n" + "typename Foo\n" + ">(T t) {\n" + " };\n" + "}", + "void foo() {\n" + " auto lambda = []<\n" + " typename T,\n" + " typename Foo>(T t){\n" + " };\n" + "}", + Style); + + // Note that this is the same line (no \n): + verifyFormat("void foo() {\n" + " auto lambd
[clang] [clang-format] Hanlde qualified type name for `QualifierAlignment` (PR #125327)
owenca wrote: > Seems to also work for top-level types (`::int_64_t constexpr x{123};` works > correctly) It didn't work either. https://github.com/llvm/llvm-project/pull/125327 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Hanlde qualified type name for `QualifierAlignment` (PR #125327)
owenca wrote: > > Seems to also work for top-level types (`::int_64_t constexpr x{123};` > > works correctly) but breaks for fully qualified types (`::std::int64_t > > constexpr x{123};` becomes `::constexpr std::int64_t x{123};`) > > Yeah, I intentionally didn't want to use a loop for names like `A1::A2:: ... > ::An` and instead just wanted to handle single-level nested names like `A::B`. Now it handles the general pattern above (and also `::A1::A2:: ... An`). https://github.com/llvm/llvm-project/pull/125327 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] Rtsan fbsd (PR #125389)
https://github.com/devnexen created https://github.com/llvm/llvm-project/pull/125389 None >From 19d4d1b3501d8524a6d88d62317dd0ea0022ebfb Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sun, 2 Feb 2025 09:36:50 + Subject: [PATCH 1/2] [compiler-rt][rtsan] porting the sanitizer to FreeBSD. Most of the apple api exceptions also apply to freebsd, however to create a per-thread realtime context pthread api cannot be used since freebsd calls calloc in _thr_alloc(). --- compiler-rt/lib/rtsan/rtsan_context.cpp | 16 .../lib/rtsan/rtsan_interceptors_posix.cpp | 4 ++-- .../tests/rtsan_test_interceptors_posix.cpp | 4 +++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/compiler-rt/lib/rtsan/rtsan_context.cpp b/compiler-rt/lib/rtsan/rtsan_context.cpp index 536d62e81e2fb6..dacc9b6dedf402 100644 --- a/compiler-rt/lib/rtsan/rtsan_context.cpp +++ b/compiler-rt/lib/rtsan/rtsan_context.cpp @@ -19,6 +19,7 @@ using namespace __sanitizer; using namespace __rtsan; +#if !SANITIZER_FREEBSD static pthread_key_t context_key; static pthread_once_t key_once = PTHREAD_ONCE_INIT; @@ -43,6 +44,21 @@ static __rtsan::Context &GetContextForThisThreadImpl() { return *current_thread_context; } +#else + +// On FreeBSD, pthread api cannot be used as calloc is called under the hood +// at library initialization time. +static __thread Context *ctx = nullptr; + +static __rtsan::Context &GetContextForThisThreadImpl() { + if (ctx == nullptr) { +ctx = static_cast(MmapOrDie(sizeof(Context), "RtsanContext")); +new (ctx) Context(); + } + + return *ctx; +} +#endif __rtsan::Context::Context() = default; diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp index 3ea9e54a046cf8..2bd2f4c7ea8dd1 100644 --- a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp +++ b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp @@ -929,7 +929,7 @@ INTERCEPTOR(int, msync, void *addr, size_t length, int flag) { return REAL(msync)(addr, length, flag); } -#if SANITIZER_APPLE +#if SANITIZER_APPLE || SANITIZER_FREEBSD INTERCEPTOR(int, mincore, const void *addr, size_t length, char *vec) { #else INTERCEPTOR(int, mincore, void *addr, size_t length, unsigned char *vec) { @@ -1324,7 +1324,7 @@ INTERCEPTOR(ssize_t, process_vm_writev, pid_t pid, // the test. Revisit this in the future, but hopefully intercepting fork/exec is // enough to dissuade usage of wait by proxy. -#if SANITIZER_APPLE +#if SANITIZER_APPLE || SANITIZER_FREEBSD #define INT_TYPE_SYSCALL int #else #define INT_TYPE_SYSCALL long diff --git a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp index e3688157a842c7..ea81e510fd7173 100644 --- a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp +++ b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp @@ -280,7 +280,7 @@ TEST_F(RtsanOpenedMmapTest, MsyncDiesWhenRealtime) { } TEST_F(RtsanOpenedMmapTest, MincoreDiesWhenRealtime) { -#if SANITIZER_APPLE +#if SANITIZER_APPLE || SANITIZER_FREEBSD std::vector vec(GetSize() / 1024); #else std::vector vec(GetSize() / 1024); @@ -1527,6 +1527,7 @@ TEST_F(KqueueTest, KeventDiesWhenRealtime) { ExpectNonRealtimeSurvival(Func); } +#if SANITIZER_APPLE TEST_F(KqueueTest, Kevent64DiesWhenRealtime) { struct kevent64_s event; EV_SET64(&event, 0, EVFILT_READ, EV_ADD, 0, 0, 0, 0, 0); @@ -1539,6 +1540,7 @@ TEST_F(KqueueTest, Kevent64DiesWhenRealtime) { ExpectRealtimeDeath(Func, "kevent64"); ExpectNonRealtimeSurvival(Func); } +#endif // SANITIZER_APPLE #endif // SANITIZER_INTERCEPT_KQUEUE #if SANITIZER_LINUX >From c962385b9eac1abc002c0aa80b50f07efaa7897e Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sun, 2 Feb 2025 09:38:52 + Subject: [PATCH 2/2] freebsd clang frontend update. --- clang/lib/Driver/ToolChains/FreeBSD.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/lib/Driver/ToolChains/FreeBSD.cpp b/clang/lib/Driver/ToolChains/FreeBSD.cpp index a6d859f0ebfec2..baabfabf26267f 100644 --- a/clang/lib/Driver/ToolChains/FreeBSD.cpp +++ b/clang/lib/Driver/ToolChains/FreeBSD.cpp @@ -497,6 +497,7 @@ SanitizerMask FreeBSD::getSupportedSanitizers() const { Res |= SanitizerKind::PointerCompare; Res |= SanitizerKind::PointerSubtract; Res |= SanitizerKind::Vptr; + Res |= SanitizerKind::Realtime; if (IsAArch64 || IsX86_64 || IsMIPS64) { Res |= SanitizerKind::Leak; Res |= SanitizerKind::Thread; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] Rtsan fbsd (PR #125389)
https://github.com/devnexen ready_for_review https://github.com/llvm/llvm-project/pull/125389 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][bytecode] Ignore Namespace{Using, Alias}Decls (PR #125387)
https://github.com/tbaederr closed https://github.com/llvm/llvm-project/pull/125387 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] Rtsan fbsd (PR #125389)
llvmbot wrote: @llvm/pr-subscribers-compiler-rt-sanitizer Author: David CARLIER (devnexen) Changes --- Full diff: https://github.com/llvm/llvm-project/pull/125389.diff 4 Files Affected: - (modified) clang/lib/Driver/ToolChains/FreeBSD.cpp (+1) - (modified) compiler-rt/lib/rtsan/rtsan_context.cpp (+16) - (modified) compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp (+2-2) - (modified) compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp (+3-1) ``diff diff --git a/clang/lib/Driver/ToolChains/FreeBSD.cpp b/clang/lib/Driver/ToolChains/FreeBSD.cpp index a6d859f0ebfec2..baabfabf26267f 100644 --- a/clang/lib/Driver/ToolChains/FreeBSD.cpp +++ b/clang/lib/Driver/ToolChains/FreeBSD.cpp @@ -497,6 +497,7 @@ SanitizerMask FreeBSD::getSupportedSanitizers() const { Res |= SanitizerKind::PointerCompare; Res |= SanitizerKind::PointerSubtract; Res |= SanitizerKind::Vptr; + Res |= SanitizerKind::Realtime; if (IsAArch64 || IsX86_64 || IsMIPS64) { Res |= SanitizerKind::Leak; Res |= SanitizerKind::Thread; diff --git a/compiler-rt/lib/rtsan/rtsan_context.cpp b/compiler-rt/lib/rtsan/rtsan_context.cpp index 536d62e81e2fb6..dacc9b6dedf402 100644 --- a/compiler-rt/lib/rtsan/rtsan_context.cpp +++ b/compiler-rt/lib/rtsan/rtsan_context.cpp @@ -19,6 +19,7 @@ using namespace __sanitizer; using namespace __rtsan; +#if !SANITIZER_FREEBSD static pthread_key_t context_key; static pthread_once_t key_once = PTHREAD_ONCE_INIT; @@ -43,6 +44,21 @@ static __rtsan::Context &GetContextForThisThreadImpl() { return *current_thread_context; } +#else + +// On FreeBSD, pthread api cannot be used as calloc is called under the hood +// at library initialization time. +static __thread Context *ctx = nullptr; + +static __rtsan::Context &GetContextForThisThreadImpl() { + if (ctx == nullptr) { +ctx = static_cast(MmapOrDie(sizeof(Context), "RtsanContext")); +new (ctx) Context(); + } + + return *ctx; +} +#endif __rtsan::Context::Context() = default; diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp index 3ea9e54a046cf8..2bd2f4c7ea8dd1 100644 --- a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp +++ b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp @@ -929,7 +929,7 @@ INTERCEPTOR(int, msync, void *addr, size_t length, int flag) { return REAL(msync)(addr, length, flag); } -#if SANITIZER_APPLE +#if SANITIZER_APPLE || SANITIZER_FREEBSD INTERCEPTOR(int, mincore, const void *addr, size_t length, char *vec) { #else INTERCEPTOR(int, mincore, void *addr, size_t length, unsigned char *vec) { @@ -1324,7 +1324,7 @@ INTERCEPTOR(ssize_t, process_vm_writev, pid_t pid, // the test. Revisit this in the future, but hopefully intercepting fork/exec is // enough to dissuade usage of wait by proxy. -#if SANITIZER_APPLE +#if SANITIZER_APPLE || SANITIZER_FREEBSD #define INT_TYPE_SYSCALL int #else #define INT_TYPE_SYSCALL long diff --git a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp index e3688157a842c7..ea81e510fd7173 100644 --- a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp +++ b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp @@ -280,7 +280,7 @@ TEST_F(RtsanOpenedMmapTest, MsyncDiesWhenRealtime) { } TEST_F(RtsanOpenedMmapTest, MincoreDiesWhenRealtime) { -#if SANITIZER_APPLE +#if SANITIZER_APPLE || SANITIZER_FREEBSD std::vector vec(GetSize() / 1024); #else std::vector vec(GetSize() / 1024); @@ -1527,6 +1527,7 @@ TEST_F(KqueueTest, KeventDiesWhenRealtime) { ExpectNonRealtimeSurvival(Func); } +#if SANITIZER_APPLE TEST_F(KqueueTest, Kevent64DiesWhenRealtime) { struct kevent64_s event; EV_SET64(&event, 0, EVFILT_READ, EV_ADD, 0, 0, 0, 0, 0); @@ -1539,6 +1540,7 @@ TEST_F(KqueueTest, Kevent64DiesWhenRealtime) { ExpectRealtimeDeath(Func, "kevent64"); ExpectNonRealtimeSurvival(Func); } +#endif // SANITIZER_APPLE #endif // SANITIZER_INTERCEPT_KQUEUE #if SANITIZER_LINUX `` https://github.com/llvm/llvm-project/pull/125389 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] Rtsan fbsd (PR #125389)
llvmbot wrote: @llvm/pr-subscribers-clang Author: David CARLIER (devnexen) Changes --- Full diff: https://github.com/llvm/llvm-project/pull/125389.diff 4 Files Affected: - (modified) clang/lib/Driver/ToolChains/FreeBSD.cpp (+1) - (modified) compiler-rt/lib/rtsan/rtsan_context.cpp (+16) - (modified) compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp (+2-2) - (modified) compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp (+3-1) ``diff diff --git a/clang/lib/Driver/ToolChains/FreeBSD.cpp b/clang/lib/Driver/ToolChains/FreeBSD.cpp index a6d859f0ebfec2..baabfabf26267f 100644 --- a/clang/lib/Driver/ToolChains/FreeBSD.cpp +++ b/clang/lib/Driver/ToolChains/FreeBSD.cpp @@ -497,6 +497,7 @@ SanitizerMask FreeBSD::getSupportedSanitizers() const { Res |= SanitizerKind::PointerCompare; Res |= SanitizerKind::PointerSubtract; Res |= SanitizerKind::Vptr; + Res |= SanitizerKind::Realtime; if (IsAArch64 || IsX86_64 || IsMIPS64) { Res |= SanitizerKind::Leak; Res |= SanitizerKind::Thread; diff --git a/compiler-rt/lib/rtsan/rtsan_context.cpp b/compiler-rt/lib/rtsan/rtsan_context.cpp index 536d62e81e2fb6..dacc9b6dedf402 100644 --- a/compiler-rt/lib/rtsan/rtsan_context.cpp +++ b/compiler-rt/lib/rtsan/rtsan_context.cpp @@ -19,6 +19,7 @@ using namespace __sanitizer; using namespace __rtsan; +#if !SANITIZER_FREEBSD static pthread_key_t context_key; static pthread_once_t key_once = PTHREAD_ONCE_INIT; @@ -43,6 +44,21 @@ static __rtsan::Context &GetContextForThisThreadImpl() { return *current_thread_context; } +#else + +// On FreeBSD, pthread api cannot be used as calloc is called under the hood +// at library initialization time. +static __thread Context *ctx = nullptr; + +static __rtsan::Context &GetContextForThisThreadImpl() { + if (ctx == nullptr) { +ctx = static_cast(MmapOrDie(sizeof(Context), "RtsanContext")); +new (ctx) Context(); + } + + return *ctx; +} +#endif __rtsan::Context::Context() = default; diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp index 3ea9e54a046cf8..2bd2f4c7ea8dd1 100644 --- a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp +++ b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp @@ -929,7 +929,7 @@ INTERCEPTOR(int, msync, void *addr, size_t length, int flag) { return REAL(msync)(addr, length, flag); } -#if SANITIZER_APPLE +#if SANITIZER_APPLE || SANITIZER_FREEBSD INTERCEPTOR(int, mincore, const void *addr, size_t length, char *vec) { #else INTERCEPTOR(int, mincore, void *addr, size_t length, unsigned char *vec) { @@ -1324,7 +1324,7 @@ INTERCEPTOR(ssize_t, process_vm_writev, pid_t pid, // the test. Revisit this in the future, but hopefully intercepting fork/exec is // enough to dissuade usage of wait by proxy. -#if SANITIZER_APPLE +#if SANITIZER_APPLE || SANITIZER_FREEBSD #define INT_TYPE_SYSCALL int #else #define INT_TYPE_SYSCALL long diff --git a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp index e3688157a842c7..ea81e510fd7173 100644 --- a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp +++ b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp @@ -280,7 +280,7 @@ TEST_F(RtsanOpenedMmapTest, MsyncDiesWhenRealtime) { } TEST_F(RtsanOpenedMmapTest, MincoreDiesWhenRealtime) { -#if SANITIZER_APPLE +#if SANITIZER_APPLE || SANITIZER_FREEBSD std::vector vec(GetSize() / 1024); #else std::vector vec(GetSize() / 1024); @@ -1527,6 +1527,7 @@ TEST_F(KqueueTest, KeventDiesWhenRealtime) { ExpectNonRealtimeSurvival(Func); } +#if SANITIZER_APPLE TEST_F(KqueueTest, Kevent64DiesWhenRealtime) { struct kevent64_s event; EV_SET64(&event, 0, EVFILT_READ, EV_ADD, 0, 0, 0, 0, 0); @@ -1539,6 +1540,7 @@ TEST_F(KqueueTest, Kevent64DiesWhenRealtime) { ExpectRealtimeDeath(Func, "kevent64"); ExpectNonRealtimeSurvival(Func); } +#endif // SANITIZER_APPLE #endif // SANITIZER_INTERCEPT_KQUEUE #if SANITIZER_LINUX `` https://github.com/llvm/llvm-project/pull/125389 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 00bdce1 - [clang][bytecode] Ignore Namespace{Using,Alias}Decls (#125387)
Author: Timm Baeder Date: 2025-02-02T10:43:15+01:00 New Revision: 00bdce1c373a1c5b756f4cf694a952ef702d0294 URL: https://github.com/llvm/llvm-project/commit/00bdce1c373a1c5b756f4cf694a952ef702d0294 DIFF: https://github.com/llvm/llvm-project/commit/00bdce1c373a1c5b756f4cf694a952ef702d0294.diff LOG: [clang][bytecode] Ignore Namespace{Using,Alias}Decls (#125387) These were missing here and are used in a few libc++ tests. Added: Modified: clang/lib/AST/ByteCode/Compiler.cpp clang/test/AST/ByteCode/literals.cpp Removed: diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index f7f4f713c787f25..b9e22ebb7a41a90 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -4991,8 +4991,8 @@ bool Compiler::visitCompoundStmt(const CompoundStmt *S) { template bool Compiler::visitDeclStmt(const DeclStmt *DS) { for (const auto *D : DS->decls()) { -if (isa(D)) +if (isa(D)) continue; const auto *VD = dyn_cast(D); diff --git a/clang/test/AST/ByteCode/literals.cpp b/clang/test/AST/ByteCode/literals.cpp index b75ca2b19a969a9..a80ee7ad84fc748 100644 --- a/clang/test/AST/ByteCode/literals.cpp +++ b/clang/test/AST/ByteCode/literals.cpp @@ -914,12 +914,18 @@ namespace TypeTraits { } #if __cplusplus >= 201402L +namespace SomeNS { + using MyInt = int; +} + constexpr int ignoredDecls() { static_assert(true, ""); struct F { int a; }; enum E { b }; using A = int; typedef int Z; + namespace NewNS = SomeNS; + using NewNS::MyInt; return F{12}.a; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [nfc][analyzer] Add MemSpace trait to program state (PR #123003)
steakhal wrote: I've pushed a couple of commits to sync this PR with what I had in mind. I hope you don't mind @Flandini. Please anyone do a thorough review. --- I've considered uplifiting a couple of the `VarRegion::getStackFrame()`, and similar `getStackFrame()` functions, because internally they would use the memory space to resolve to the stack frame. I still consider it, but it would need a similar migration: introduce a new overload that takes the State, and try to eradicate the overload that doesn't take any State. Then keep the cases that cannot be uplifted. I found this tedious, and not strictly necessary as a first step to merge this PR. It would also "pollute" the API surface with the leftover APIs, which isn't ideal. https://github.com/llvm/llvm-project/pull/123003 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
owenca wrote: > > From #123921, it seems that you only want the new option to work with > > `--staged`, but should it also work with other options that may print a > > list of filenames? > > I don't know; I only use `git clang-format` as in #123921 and I am not > familiar with all its uses. I was hoping that you or someone other who is > familiar could determine this. > > What worries me is conditions where more than one list of files is printed, > e.g. unchanged + changed files. It's difficult to parse these messages from > the shell; when a null-separated list is printed from other command line > utilities, it has one meaning, e.g. "here's the list of files you asked for", > whereas `git clang-format` prints other diagnostics too. > > If someone is familiar with the usages of this tool maybe they can tell me > what is the logic that must be followed in terms of **which list** is the > important one according to the options passed, so that only that list is > printed with `--print0`. It seems that `git-clang-format` was not designed for its output to be consumed by another utility as it prints everything to `stdout` instead of `stderr`. Adding an option like`--print0` (especially just for `--staged`) looks odd to me. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)
@@ -0,0 +1,147 @@ +.. title:: clang-tidy - performance-redundant-lookup + +performance-redundant-lookup + + +This check warns about potential redundant container lookup operations within +a function. + +Examples + + +.. code-block:: c++ + +if (map.count(key) && map[key] < threshold) { + // do stuff +} + +In this example, we would check if the key is present in the map, +and then do a second lookup to actually load the value. +We could refactor the code into this, to use a single lookup: + +.. code-block:: c++ + +if (auto it = map.find(key); it != map.end() && it->second < threshold) { + // do stuff +} + +In this example, we do three lookups while calculating, caching and then +using the result of some expensive computation: + +.. code-block:: c++ + +if (!cache.contains(key)) { +cache[key] = computeExpensiveValue(); +} +use(cache[key]); + +We could refactor this code using ``try_emplace`` to fill up the cache entry +if wasn't present, and just use it if we already computed the value. +This way we would only have a single unavoidable lookup: + +.. code-block:: c++ + +auto [cacheSlot, inserted] cache.try_emplace(key); +if (inserted) { +cacheSlot->second = computeExpensiveValue(); +} +use(cacheSlot->second); + + +What is a "lookup"? +--- + +All container operations that walk the internal structure of the container +should be considered as "lookups". +This means that checking if an element is present or inserting an element +is also considered as a "lookup". + +For example, ``contains``, ``count`` but even the ``operator[]`` +should be considered as "lookups". + +Technically ``insert``, ``emplace`` or ``try_emplace`` are also lookups, +even if due to limitations, they are not recognized as such. + +Lookups inside macros are ignored, thus not considered as "lookups". +For example: + +.. code-block:: c++ + +assert(map.count(key) == 0); // Not considered as a "lookup". + +Options +--- + +.. option:: ContainerNameRegex + + The regular expression matching the type of the container objects. + This is matched in a case insensitive manner. + Default is ``set|map``. steakhal wrote: I share your concern. However, there is more to consider here. I think, this check should work with user-defined containers out of the box, to have a better off the shelf experience. If they find FPs, they can override this regex, and that's it. I think the chances as slim to have container-like APIs like I'm matching here. For example, exatcly single argument "insert". I could restrict the matches by requiring more things. For example, `key_type` type alias defined in the container, or something like that. However, for doing that, I'd first ask if this misclassification really happens in practice to justify the complexity. My experience with the FPs is that there are sharp edges with the check, but this one is not one of those. https://github.com/llvm/llvm-project/pull/125420 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)
https://github.com/ricejasonf updated https://github.com/llvm/llvm-project/pull/125394 >From a323e058b2c8adf97f7f9a55a9187f74de9b8d17 Mon Sep 17 00:00:00 2001 From: Jason Rice Date: Sun, 2 Feb 2025 00:52:47 -0800 Subject: [PATCH 1/3] [Clang][P1061] Consolidate ResolvedUnexpandedPackExpr into FunctionParmPackExpr --- clang/include/clang/AST/DeclCXX.h | 28 + clang/include/clang/AST/ExprCXX.h | 21 +- clang/include/clang/Sema/Template.h | 2 +- clang/lib/AST/DeclCXX.cpp | 9 +++-- clang/lib/AST/ExprCXX.cpp | 14 +++ clang/lib/Sema/SemaDeclCXX.cpp| 26 ++-- clang/lib/Sema/SemaExpr.cpp | 6 +-- clang/lib/Sema/SemaTemplateInstantiate.cpp| 30 +- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 40 +++ clang/lib/Sema/SemaTemplateVariadic.cpp | 32 +-- clang/lib/Serialization/ASTReaderStmt.cpp | 6 +-- clang/test/AST/ast-dump-binding-pack.cpp | 13 ++ clang/test/SemaCXX/cxx2c-binding-pack.cpp | 11 + 13 files changed, 103 insertions(+), 135 deletions(-) diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index 766821b4fb25cb..1c630d61690355 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -4194,8 +4194,8 @@ class BindingDecl : public ValueDecl { /// decomposition declaration, and when the initializer is type-dependent. Expr *getBinding() const { return Binding; } - // Get the array of Exprs when the binding represents a pack. - llvm::ArrayRef getBindingPackExprs() const; + // Get the array of nested BindingDecls when the binding represents a pack. + llvm::ArrayRef getBindingPackDecls() const; /// Get the decomposition declaration that this binding represents a /// decomposition of. @@ -4246,10 +4246,8 @@ class DecompositionDecl final for (auto *B : Bindings) { B->setDecomposedDecl(this); if (B->isParameterPack() && B->getBinding()) { -for (Expr *E : B->getBindingPackExprs()) { - auto *DRE = cast(E); - auto *NestedB = cast(DRE->getDecl()); - NestedB->setDecomposedDecl(this); +for (BindingDecl *NestedBD : B->getBindingPackDecls()) { + NestedBD->setDecomposedDecl(this); } } } @@ -4278,25 +4276,21 @@ class DecompositionDecl final // Provide a flattened range to visit each binding. auto flat_bindings() const { llvm::ArrayRef Bindings = bindings(); -llvm::ArrayRef PackExprs; +llvm::ArrayRef PackBindings; // Split the bindings into subranges split by the pack. -auto S1 = Bindings.take_until( +auto BeforePackBindings = Bindings.take_until( [](BindingDecl *BD) { return BD->isParameterPack(); }); -Bindings = Bindings.drop_front(S1.size()); +Bindings = Bindings.drop_front(BeforePackBindings.size()); if (!Bindings.empty()) { - PackExprs = Bindings.front()->getBindingPackExprs(); + PackBindings = Bindings.front()->getBindingPackDecls(); Bindings = Bindings.drop_front(); } -auto S2 = llvm::map_range(PackExprs, [](Expr *E) { - auto *DRE = cast(E); - return cast(DRE->getDecl()); -}); - -return llvm::concat(std::move(S1), std::move(S2), - std::move(Bindings)); +return llvm::concat(std::move(BeforePackBindings), +std::move(PackBindings), +std::move(Bindings)); } void printName(raw_ostream &OS, const PrintingPolicy &Policy) const override; diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h index 0b6c8cfb163c95..2b23fa51c6232b 100644 --- a/clang/include/clang/AST/ExprCXX.h +++ b/clang/include/clang/AST/ExprCXX.h @@ -4649,13 +4649,13 @@ class SubstNonTypeTemplateParmPackExpr : public Expr { /// \endcode class FunctionParmPackExpr final : public Expr, - private llvm::TrailingObjects { + private llvm::TrailingObjects { friend class ASTReader; friend class ASTStmtReader; friend TrailingObjects; /// The function parameter pack which was referenced. - VarDecl *ParamPack; + ValueDecl *ParamPack; /// The location of the function parameter pack reference. SourceLocation NameLoc; @@ -4663,35 +4663,34 @@ class FunctionParmPackExpr final /// The number of expansions of this pack. unsigned NumParameters; - FunctionParmPackExpr(QualType T, VarDecl *ParamPack, - SourceLocation NameLoc, unsigned NumParams, - VarDecl *const *Params); + FunctionParmPackExpr(QualType T, ValueDecl *ParamPack, SourceLocation NameLoc, + unsigned NumParams, ValueDecl *const *Params); public: static FunctionParmPackExpr *Create(const ASTContext &Context, QualType T, -
[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff d5a7a483a65f830a0c7a931781bc90046dc67ff4 b2b85fdbe11dce02c6ba842b5a4949b70c47207b --extensions cpp,h -- clang/include/clang/AST/DeclCXX.h clang/include/clang/AST/ExprCXX.h clang/include/clang/AST/RecursiveASTVisitor.h clang/include/clang/Sema/Sema.h clang/include/clang/Sema/Template.h clang/include/clang/Serialization/ASTBitCodes.h clang/lib/AST/DeclCXX.cpp clang/lib/AST/Expr.cpp clang/lib/AST/ExprCXX.cpp clang/lib/AST/ExprClassification.cpp clang/lib/AST/ExprConstant.cpp clang/lib/AST/ItaniumMangle.cpp clang/lib/AST/StmtPrinter.cpp clang/lib/AST/StmtProfile.cpp clang/lib/Sema/SemaDeclCXX.cpp clang/lib/Sema/SemaExceptionSpec.cpp clang/lib/Sema/SemaExpr.cpp clang/lib/Sema/SemaTemplateInstantiate.cpp clang/lib/Sema/SemaTemplateInstantiateDecl.cpp clang/lib/Sema/SemaTemplateVariadic.cpp clang/lib/Sema/TreeTransform.h clang/lib/Serialization/ASTReaderStmt.cpp clang/lib/Serialization/ASTWriter.cpp clang/lib/Serialization/ASTWriterStmt.cpp clang/lib/StaticAnalyzer/Core/ExprEngine.cpp clang/test/AST/ast-dump-binding-pack.cpp clang/test/SemaCXX/cxx2c-binding-pack.cpp clang/tools/libclang/CXCursor.cpp `` View the diff from clang-format here. ``diff diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp index 77cbf1362a..b0a14629f2 100644 --- a/clang/lib/AST/DeclCXX.cpp +++ b/clang/lib/AST/DeclCXX.cpp @@ -3495,9 +3495,10 @@ VarDecl *BindingDecl::getHoldingVar() const { llvm::ArrayRef BindingDecl::getBindingPackDecls() const { assert(Binding && "expecting a pack expr"); auto *FP = cast(Binding); - if(FP->getNumExpansions() == 0) - return {}; - return llvm::ArrayRef(cast(FP->begin()), FP->getNumExpansions()); + if (FP->getNumExpansions() == 0) +return {}; + return llvm::ArrayRef(cast(FP->begin()), + FP->getNumExpansions()); } void DecompositionDecl::anchor() {} `` https://github.com/llvm/llvm-project/pull/125394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add BreakBeforeTemplateClose option (PR #118046)
@@ -11220,6 +11220,333 @@ TEST_F(FormatTest, WrapsTemplateDeclarationsWithComments) { Style); } +TEST_F(FormatTest, BreakBeforeTemplateCloser) { + FormatStyle Style = getGoogleStyle(FormatStyle::LK_Cpp); + // Begin with tests covering the case where there is no constraint on the + // column limit. + Style.ColumnLimit = 0; + // When BreakBeforeTemplateCloser is turned off, the line break that it adds + // shall be removed: + verifyFormat("template <\n" + "typename Foo,\n" + "typename Bar>\n" + "void foo() {}", + "template <\n" + "typename Foo,\n" + "typename Bar\n" + ">\n" + "void foo() {}", + Style); + + Style.BreakBeforeTemplateCloser = FormatStyle::BBTCS_BlockIndent; + // BreakBeforeTemplateCloser should NOT force template declarations onto + // multiple lines. + verifyFormat("template \n" + "void foo() {}", + Style); + verifyFormat("template \n" + "void foo() {}", + Style); + // It should allow a line break, even when the typename is short. + // verifyNoChange is needed because the default behavior is one line. + verifyNoChange("template <\n" + "typename Foo\n" + ">\n" + "void foo() {}", + Style); + verifyNoChange("template <\n" + "typename Foo,\n" + "typename Bar\n" + ">\n" + "void foo() {}", + Style); + verifyNoChange("template \n" + "void foo() {}", + Style); + // It should add a line break before > if not already present: + verifyFormat("template <\n" + "typename Foo\n" + ">\n" + "void foo() {}", + "template <\n" + "typename Foo>\n" + "void foo() {}", + Style); + verifyFormat("template <\n" + "typename Foo,\n" + "typename Bar\n" + ">\n" + "void foo() {}", + "template <\n" + "typename Foo,\n" + "typename Bar>\n" + "void foo() {}", + Style); + // When within an indent scope, the > should be placed accordingly: + verifyFormat("struct Baz {\n" + " template <\n" + " typename Foo,\n" + " typename Bar\n" + " >\n" + " void foo() {}\n" + "};", + "struct Baz {\n" + " template <\n" + " typename Foo,\n" + " typename Bar>\n" + " void foo() {}\n" + "};", + Style); + + // Test from issue #80049: + verifyFormat( + "void foo() {\n" + " using type = std::remove_cv_t<\n" + " add_common_cv_reference<\n" + " std::common_type_t, std::decay_t>,\n" + " T0,\n" + " T1\n" + " >\n" + " >;\n" + "}", + "void foo() {\n" + " using type = std::remove_cv_t<\n" + " add_common_cv_reference<\n" + " std::common_type_t, std::decay_t>,\n" + " T0,\n" + " T1>>;\n" + "}", + Style); + + // Test lambda goes to next line: + verifyFormat("void foo() {\n" + " auto lambda = []<\n" + "typename T\n" + ">(T t) {\n" + " };\n" + "}", + "void foo() {\n" + " auto lambda = []<\n" + " typename T>(T t){\n" + " };\n" + "}", + Style); + // With no column limit, two parameters can go on the same line: + verifyFormat("void foo() {\n" + " auto lambda = []<\n" + "typename T, typename Foo\n" + ">(T t) {\n" + " };\n" + "}", + "void foo() {\n" + " auto lambda = []<\n" + " typename T, typename Foo>(T t){\n" + " };\n" + "}", + Style); + // Or on different lines: + verifyFormat("void foo() {\n" + " auto lambda = []<\n" + "typename T,\n" + "typename Foo\n" + ">(T t) {\n" + " };\n" + "}", + "void foo() {\n" + " auto lambda = []<\n" + " typename T,\n" + " typename Foo>(T t){\n" + " };\n" + "}", + Style); + + // Note that this is the same line (no \n): + verifyFormat("void foo() {\n" + " auto lambd
[clang] [clang] Do not emit template parameter objects as COMDATs when they have internal linkage. (PR #125448)
@@ -0,0 +1,38 @@ +// RUN: %clang_cc1 -emit-llvm -std=c++20 -triple x86_64-unknown-linux-gnu %s -o - | FileCheck %s + +// COMDAT groups in ELF objects are not permitted to contain local symbols. While template parameter +// objects are normally emitted in COMDATs, we shouldn't do so if they would have internal linkage. + +extern "C" int printf(...); +typedef __typeof__(sizeof(0)) size_t; + +namespace { +template +struct DebugContext +{ +char value[N]; +constexpr DebugContext(const char (&str)[N]) { MaskRay wrote: The test can be reduced. https://github.com/llvm/llvm-project/pull/125448 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Do not emit template parameter objects as COMDATs when they have internal linkage. (PR #125448)
@@ -3765,7 +3765,7 @@ ConstantAddress CodeGenModule::GetAddrOfTemplateParamObject( auto *GV = new llvm::GlobalVariable(getModule(), Init->getType(), /*isConstant=*/true, Linkage, Init, Name); setGVProperties(GV, TPO); - if (supportsCOMDAT()) + if (supportsCOMDAT() && Linkage == llvm::GlobalValue::LinkOnceODRLinkage) MaskRay wrote: I recall that in some cases WeakODRLinkage is used. Should the condition be something like `!isLocalLinkage()`? https://github.com/llvm/llvm-project/pull/125448 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Do not emit template parameter objects as COMDATs when they have internal linkage. (PR #125448)
@@ -0,0 +1,38 @@ +// RUN: %clang_cc1 -emit-llvm -std=c++20 -triple x86_64-unknown-linux-gnu %s -o - | FileCheck %s MaskRay wrote: Probably best to figure out where the primary test of the neighbor lines is and adds the extra test there. https://github.com/llvm/llvm-project/pull/125448 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Do not emit template parameter objects as COMDATs when they have internal linkage. (PR #125448)
@@ -3765,7 +3765,7 @@ ConstantAddress CodeGenModule::GetAddrOfTemplateParamObject( auto *GV = new llvm::GlobalVariable(getModule(), Init->getType(), /*isConstant=*/true, Linkage, Init, Name); setGVProperties(GV, TPO); - if (supportsCOMDAT()) + if (supportsCOMDAT() && Linkage == llvm::GlobalValue::LinkOnceODRLinkage) resistor wrote: Linkage is set to either LinkOnceODRLinkage or InternalLinkage a few lines above this. https://github.com/llvm/llvm-project/pull/125448 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) Changes This patch makes it so the correct instantiation context is printed for diagnostics suppessed by template argument deduction. The context is saved along with the suppressed diagnostic, and when the declaration they were attached to becomes used, we print the correct context, instead of whatever context was at this point. --- Patch is 38.76 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/125453.diff 18 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+3) - (modified) clang/include/clang/Sema/Sema.h (+21-5) - (modified) clang/lib/Sema/Sema.cpp (+11-2) - (modified) clang/lib/Sema/SemaAttr.cpp (+3-3) - (modified) clang/lib/Sema/SemaExpr.cpp (+4-3) - (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+136-135) - (modified) clang/test/CXX/drs/cwg0xx.cpp (+5) - (modified) clang/test/CXX/drs/cwg4xx.cpp (+2-1) - (modified) clang/test/CXX/temp/temp.arg/temp.arg.type/p2.cpp (+6-3) - (modified) clang/test/SemaCXX/anonymous-struct.cpp (+2-3) - (modified) clang/test/SemaCXX/bool-increment-SFINAE.cpp (+2-2) - (modified) clang/test/SemaCXX/cxx98-compat-flags.cpp (+2) - (modified) clang/test/SemaCXX/cxx98-compat.cpp (+2) - (modified) clang/test/SemaCXX/deprecated.cpp (+2-2) - (modified) clang/test/SemaCXX/lambda-expressions.cpp (+2) - (modified) clang/test/SemaCXX/undefined-internal.cpp (+4-2) - (modified) clang/test/SemaTemplate/recovery-crash.cpp (+1) - (modified) clang/test/SemaTemplate/temp_arg_nontype.cpp (+1) ``diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 8be1ea2fb01455..70c3b062d97717 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -128,6 +128,9 @@ Bug Fixes to Attribute Support Bug Fixes to C++ Support +- Clang now prints the correct instantiation context for diagnostics suppressed + by template argument deduction. + Bug Fixes to AST Handling ^ diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 472a0e25adc975..7d01dc1aa4c00b 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -1909,7 +1909,19 @@ class Sema final : public SemaBase { /// '\#pragma clang attribute push' directives to the given declaration. void AddPragmaAttributes(Scope *S, Decl *D); - void PrintPragmaAttributeInstantiationPoint(); + using DiagFuncRef = + llvm::function_ref; + auto getDefaultDiagFunc() { +return [this](SourceLocation Loc, PartialDiagnostic PD) { + DiagnosticBuilder Builder(Diags.Report(Loc, PD.getDiagID())); + PD.Emit(Builder); +}; + } + + void PrintPragmaAttributeInstantiationPoint(DiagFuncRef DiagFunc); + void PrintPragmaAttributeInstantiationPoint() { +PrintPragmaAttributeInstantiationPoint(getDefaultDiagFunc()); + } void DiagnoseUnterminatedPragmaAttribute(); @@ -13260,18 +13272,22 @@ class Sema final : public SemaBase { void pushCodeSynthesisContext(CodeSynthesisContext Ctx); void popCodeSynthesisContext(); - void PrintContextStack() { + void PrintContextStack(DiagFuncRef DiagFunc) { if (!CodeSynthesisContexts.empty() && CodeSynthesisContexts.size() != LastEmittedCodeSynthesisContextDepth) { - PrintInstantiationStack(); + PrintInstantiationStack(DiagFunc); LastEmittedCodeSynthesisContextDepth = CodeSynthesisContexts.size(); } if (PragmaAttributeCurrentTargetDecl) - PrintPragmaAttributeInstantiationPoint(); + PrintPragmaAttributeInstantiationPoint(DiagFunc); } + void PrintContextStack() { PrintContextStack(getDefaultDiagFunc()); } /// Prints the current instantiation stack through a series of /// notes. - void PrintInstantiationStack(); + void PrintInstantiationStack(DiagFuncRef DiagFunc); + void PrintInstantiationStack() { +PrintInstantiationStack(getDefaultDiagFunc()); + } /// Determines whether we are currently in a context where /// template argument substitution failures are not considered diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index 9507d7602aa401..33e2bd1e030513 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -1654,11 +1654,20 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) { } case DiagnosticIDs::SFINAE_Suppress: + if (DiagnosticsEngine::Level Level = getDiagnostics().getDiagnosticLevel( + DiagInfo.getID(), DiagInfo.getLocation()); + Level == DiagnosticsEngine::Ignored) +return; // Make a copy of this suppressed diagnostic and store it with the // template-deduction information; if (*Info) { -(*Info)->addSuppressedDiagnostic(DiagInfo.getLocation(), - PartialDiagnostic(DiagInfo, Context.getDiagAllocator())); +(*Info)->addSuppressedDiagnost
[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)
https://github.com/mizvekov created https://github.com/llvm/llvm-project/pull/125453 This patch makes it so the correct instantiation context is printed for diagnostics suppessed by template argument deduction. The context is saved along with the suppressed diagnostic, and when the declaration they were attached to becomes used, we print the correct context, instead of whatever context was at this point. >From f6ea38d9ab756eaf85b5943a12056f4f534d4ca0 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Sun, 2 Feb 2025 23:47:15 -0300 Subject: [PATCH] [clang] print correct context for diagnostics suppressed by deduction This patch makes it so the correct instantiation context is printed for diagnostics suppessed by template argument deduction. The context is saved along with the suppressed diagnostic, and when the declaration they were attached to becomes used, we print the correct context, instead of whatever context was at this point. --- clang/docs/ReleaseNotes.rst | 3 + clang/include/clang/Sema/Sema.h | 26 +- clang/lib/Sema/Sema.cpp | 13 +- clang/lib/Sema/SemaAttr.cpp | 6 +- clang/lib/Sema/SemaExpr.cpp | 7 +- clang/lib/Sema/SemaTemplateInstantiate.cpp| 271 +- clang/test/CXX/drs/cwg0xx.cpp | 5 + clang/test/CXX/drs/cwg4xx.cpp | 3 +- .../CXX/temp/temp.arg/temp.arg.type/p2.cpp| 9 +- clang/test/SemaCXX/anonymous-struct.cpp | 5 +- clang/test/SemaCXX/bool-increment-SFINAE.cpp | 4 +- clang/test/SemaCXX/cxx98-compat-flags.cpp | 2 + clang/test/SemaCXX/cxx98-compat.cpp | 2 + clang/test/SemaCXX/deprecated.cpp | 4 +- clang/test/SemaCXX/lambda-expressions.cpp | 2 + clang/test/SemaCXX/undefined-internal.cpp | 6 +- clang/test/SemaTemplate/recovery-crash.cpp| 1 + clang/test/SemaTemplate/temp_arg_nontype.cpp | 1 + 18 files changed, 209 insertions(+), 161 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 8be1ea2fb01455..70c3b062d97717 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -128,6 +128,9 @@ Bug Fixes to Attribute Support Bug Fixes to C++ Support +- Clang now prints the correct instantiation context for diagnostics suppressed + by template argument deduction. + Bug Fixes to AST Handling ^ diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 472a0e25adc975..7d01dc1aa4c00b 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -1909,7 +1909,19 @@ class Sema final : public SemaBase { /// '\#pragma clang attribute push' directives to the given declaration. void AddPragmaAttributes(Scope *S, Decl *D); - void PrintPragmaAttributeInstantiationPoint(); + using DiagFuncRef = + llvm::function_ref; + auto getDefaultDiagFunc() { +return [this](SourceLocation Loc, PartialDiagnostic PD) { + DiagnosticBuilder Builder(Diags.Report(Loc, PD.getDiagID())); + PD.Emit(Builder); +}; + } + + void PrintPragmaAttributeInstantiationPoint(DiagFuncRef DiagFunc); + void PrintPragmaAttributeInstantiationPoint() { +PrintPragmaAttributeInstantiationPoint(getDefaultDiagFunc()); + } void DiagnoseUnterminatedPragmaAttribute(); @@ -13260,18 +13272,22 @@ class Sema final : public SemaBase { void pushCodeSynthesisContext(CodeSynthesisContext Ctx); void popCodeSynthesisContext(); - void PrintContextStack() { + void PrintContextStack(DiagFuncRef DiagFunc) { if (!CodeSynthesisContexts.empty() && CodeSynthesisContexts.size() != LastEmittedCodeSynthesisContextDepth) { - PrintInstantiationStack(); + PrintInstantiationStack(DiagFunc); LastEmittedCodeSynthesisContextDepth = CodeSynthesisContexts.size(); } if (PragmaAttributeCurrentTargetDecl) - PrintPragmaAttributeInstantiationPoint(); + PrintPragmaAttributeInstantiationPoint(DiagFunc); } + void PrintContextStack() { PrintContextStack(getDefaultDiagFunc()); } /// Prints the current instantiation stack through a series of /// notes. - void PrintInstantiationStack(); + void PrintInstantiationStack(DiagFuncRef DiagFunc); + void PrintInstantiationStack() { +PrintInstantiationStack(getDefaultDiagFunc()); + } /// Determines whether we are currently in a context where /// template argument substitution failures are not considered diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index 9507d7602aa401..33e2bd1e030513 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -1654,11 +1654,20 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) { } case DiagnosticIDs::SFINAE_Suppress: + if (DiagnosticsEngine::Level Level = getDiagnostics().getDiagnosticLevel( + DiagInfo.ge
[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)
steakhal wrote: > Could you please run this with `--enable-check-profile` to see how heavy it > is? I plan to re-run it on clang soon, and share the results. https://github.com/llvm/llvm-project/pull/125420 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -260,15 +265,13 @@ def main(): "Ignoring the following files (wrong extension, symlink, or " "ignored by clang-format):" ) -for filename in ignored_files: -print("%s" % filename) +print_filenames(ignored_files, opts.print0) createyourpersonalaccount wrote: You have a good point, should they be mutually exclusive? I.e. if both are specified an error message is printed. I don't think `--verbose` is meaningful because `--null` does not have extra information to print. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
https://github.com/createyourpersonalaccount edited https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
createyourpersonalaccount wrote: > > > From #123921, it seems that you only want the new option to work with > > > `--staged`, but should it also work with other options that may print a > > > list of filenames? > > > > > > I don't know; I only use `git clang-format` as in #123921 and I am not > > familiar with all its uses. I was hoping that you or someone other who is > > familiar could determine this. > > What worries me is conditions where more than one list of files is printed, > > e.g. unchanged + changed files. It's difficult to parse these messages from > > the shell; when a null-separated list is printed from other command line > > utilities, it has one meaning, e.g. "here's the list of files you asked > > for", whereas `git clang-format` prints other diagnostics too. > > If someone is familiar with the usages of this tool maybe they can tell me > > what is the logic that must be followed in terms of **which list** is the > > important one according to the options passed, so that only that list is > > printed with `--print0`. > > It seems that `git-clang-format` was not designed for its output to be > consumed by another utility as it prints everything to `stdout` instead of > `stderr`. Adding an option like`--print0` (especially just for `--staged`) > looks odd to me. No, the approach of printing to `stderr` for utilities is not accurate. I've heard of this before, even the claim that this is the original UNIX way, but it's not. I don't know the origin of this [cargo-culting](https://en.wikipedia.org/wiki/Cargo_cult) factoid but I assure you that it does not hold true. (The origin I suspect may be Microsoft with its Powershell, i.e. see [about_Output_Streams](https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_output_streams?view=powershell-7.5)) There is a lot of value in `--null` since it allows `git clang-format` to be used in git hooks. Why would a Python script designed as a git subcommand not work well with git? https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] da2b415 - [clang] Add tracking source deduction guide for the explicitly-written
Author: Haojian Wu Date: 2025-02-02T23:06:25+01:00 New Revision: da2b415b19d178d6b250bb6468ed4a207860adf7 URL: https://github.com/llvm/llvm-project/commit/da2b415b19d178d6b250bb6468ed4a207860adf7 DIFF: https://github.com/llvm/llvm-project/commit/da2b415b19d178d6b250bb6468ed4a207860adf7.diff LOG: [clang] Add tracking source deduction guide for the explicitly-written deduction guide. We miss this case in the original f94c481543bdd3b11a668ad78d46593cf974788f commit. Added: Modified: clang/lib/Sema/SemaTemplateDeductionGuide.cpp clang/unittests/AST/ASTImporterTest.cpp Removed: diff --git a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp index 63592c335d211be..0d079677eecc568 100644 --- a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp +++ b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp @@ -1227,11 +1227,14 @@ void DeclareImplicitDeductionGuidesForTypeAlias( NewParam->setScopeInfo(0, I); FPTL.setParam(I, NewParam); } - auto *Transformed = cast(buildDeductionGuide( + auto *Transformed = cast(buildDeductionGuide( SemaRef, AliasTemplate, /*TemplateParams=*/nullptr, /*Constructor=*/nullptr, DG->getExplicitSpecifier(), FunctionType, AliasTemplate->getBeginLoc(), AliasTemplate->getLocation(), AliasTemplate->getEndLoc(), DG->isImplicit())); + Transformed->setSourceDeductionGuide(DG); + Transformed->setSourceDeductionGuideKind( + CXXDeductionGuideDecl::SourceDeductionGuideKind::Alias); // FIXME: Here the synthesized deduction guide is not a templated // function. Per [dcl.decl]p4, the requires-clause shall be present only diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 114d0b461dae880..0bac95eb40b2096 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -8193,6 +8193,29 @@ TEST_P(ImportFunctions, CTADAliasTemplate) { EXPECT_TRUE(ToD->getSourceDeductionGuide()); } +TEST_P(ImportFunctions, CTADAliasTemplateWithExplicitSourceDeductionGuide) { + Decl *TU = getTuDecl( + R"( + template struct A { +A(T); + }; + template + using B = A; + A(int) -> A; // explicit + B b{(int)0}; + )", + Lang_CXX20, "input.cc"); + auto *FromD = FirstDeclMatcher().match( + TU, cxxDeductionGuideDecl(hasParameter(0, hasType(asString("int"))), +hasName(""), +hasReturnTypeLoc(loc(asString("A"); + auto *ToD = Import(FromD, Lang_CXX20); + ASSERT_TRUE(ToD); + EXPECT_TRUE(ToD->getSourceDeductionGuideKind() == + CXXDeductionGuideDecl::SourceDeductionGuideKind::Alias); + EXPECT_TRUE(ToD->getSourceDeductionGuide()); +} + TEST_P(ImportFunctions, ParmVarDeclDeclContext) { constexpr auto FromTUCode = R"( void f(int P); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)
Sirraide wrote: Can you test if that also fixes #125165? https://github.com/llvm/llvm-project/pull/125394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Fix the indent of the ternary operator when AlignOperands and BreakBeforeTernaryOperators is specified. (PR #100860)
Ericson2314 wrote: In Nix (github.com/nixos/nix) I am getting formatting like ```c++ auto variant = scheme == "auto" ? (StoreReference::Variant{Specified::Auto{}}) : (StoreReference::Variant{Specified::Specified{ .scheme = getString, .authority = getString(valueAt(obj, "authority")), }}); ``` and I am not really sure why, but I gather it might have something to do with this. IMO a key rule of formatting to me is that it should not depend on the the exact lengths of identifiers and literals. Making the `:` in match `?` when `?` like this is a no-go because the horizontal position of `?` matches `variant`, `scheme`, and `"auto"`. Whereas something like ```c++ auto variant = scheme == "auto" ? (StoreReference::Variant{Specified::Auto{}}) : (StoreReference::Variant{Specified::Specified{ .scheme = getString, .authority = getString(valueAt(obj, "authority")), }}); ``` doesn't depend on those things, and so is a a valid choice. I would hope there would at least be some combination of settings to enforce this invariant. https://github.com/llvm/llvm-project/pull/100860 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)
ricejasonf wrote: @Sirraide I did try it and the bug persists. The `LHS` is a RecoveryExpr, so I am not sure how to approach that. https://github.com/llvm/llvm-project/pull/125394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits