diff --git a/.github/workflows/llvm-bugs.yml b/.github/workflows/llvm-bugs.yml index bfa0c9af946d9..f592dd6ccd903 100644 --- a/.github/workflows/llvm-bugs.yml +++ b/.github/workflows/llvm-bugs.yml @@ -12,6 +12,7 @@ on: jobs: auto-subscribe: runs-on: ubuntu-latest + if: github.repository == 'llvm/llvm-project' steps: - uses: actions/setup-node@v3 with: diff --git a/bolt/include/bolt/Core/BinarySection.h b/bolt/include/bolt/Core/BinarySection.h index 20b4b5c92576f..d3c3ba485c75b 100644 --- a/bolt/include/bolt/Core/BinarySection.h +++ b/bolt/include/bolt/Core/BinarySection.h @@ -275,6 +275,7 @@ class BinarySection { bool isTBSS() const { return isBSS() && isTLS(); } bool isVirtual() const { return ELFType == ELF::SHT_NOBITS; } bool isRela() const { return ELFType == ELF::SHT_RELA; } + bool isRelr() const { return ELFType == ELF::SHT_RELR; } bool isWritable() const { return (ELFFlags & ELF::SHF_WRITE); } bool isAllocatable() const { if (isELF()) { diff --git a/bolt/include/bolt/Core/Relocation.h b/bolt/include/bolt/Core/Relocation.h index ed56d41beb05b..51dd769387565 100644 --- a/bolt/include/bolt/Core/Relocation.h +++ b/bolt/include/bolt/Core/Relocation.h @@ -109,6 +109,9 @@ struct Relocation { /// Return code for a ABS 8-byte relocation static uint64_t getAbs64(); + /// Return code for a RELATIVE relocation + static uint64_t getRelative(); + /// Return true if this relocation is PC-relative. Return false otherwise. bool isPCRelative() const { return isPCRelative(Type); } diff --git a/bolt/include/bolt/Rewrite/RewriteInstance.h b/bolt/include/bolt/Rewrite/RewriteInstance.h index 54837aaccee3e..7e4f7c3e2187e 100644 --- a/bolt/include/bolt/Rewrite/RewriteInstance.h +++ b/bolt/include/bolt/Rewrite/RewriteInstance.h @@ -138,6 +138,9 @@ class RewriteInstance { /// Read relocations from a given section. void readDynamicRelocations(const object::SectionRef &Section, bool IsJmpRel); + /// Read relocations from a given RELR section. + void readDynamicRelrRelocations(BinarySection &Section); + /// Print relocation information. void printRelocationInfo(const RelocationRef &Rel, StringRef SymbolName, uint64_t SymbolAddress, uint64_t Addend, @@ -312,6 +315,9 @@ class RewriteInstance { /// Patch allocatable relocation sections. ELF_FUNCTION(void, patchELFAllocatableRelaSections); + /// Patch allocatable relr section. + ELF_FUNCTION(void, patchELFAllocatableRelrSection); + /// Finalize memory image of section header string table. ELF_FUNCTION(void, finalizeSectionStringTable); @@ -486,6 +492,11 @@ class RewriteInstance { uint64_t DynamicRelocationsSize{0}; uint64_t DynamicRelativeRelocationsCount{0}; + // Location and size of .relr.dyn relocations. + std::optional DynamicRelrAddress; + uint64_t DynamicRelrSize{0}; + uint64_t DynamicRelrEntrySize{0}; + /// PLT relocations are special kind of dynamic relocations stored separately. std::optional PLTRelocationsAddress; uint64_t PLTRelocationsSize{0}; diff --git a/bolt/lib/Core/Relocation.cpp b/bolt/lib/Core/Relocation.cpp index eceecf3ef03c4..b36e321f4e342 100644 --- a/bolt/lib/Core/Relocation.cpp +++ b/bolt/lib/Core/Relocation.cpp @@ -642,6 +642,12 @@ uint64_t Relocation::getAbs64() { return ELF::R_X86_64_64; } +uint64_t Relocation::getRelative() { + if (Arch == Triple::aarch64) + return ELF::R_AARCH64_RELATIVE; + return ELF::R_X86_64_RELATIVE; +} + size_t Relocation::emit(MCStreamer *Streamer) const { const size_t Size = getSizeForType(Type); MCContext &Ctx = Streamer->getContext(); diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp index 447c5d0aab932..57e41d5b5724d 100644 --- a/bolt/lib/Rewrite/RewriteInstance.cpp +++ b/bolt/lib/Rewrite/RewriteInstance.cpp @@ -1050,6 +1050,16 @@ void RewriteInstance::discoverFileObjects() { LLVM_DEBUG(dbgs() << "BOLT-DEBUG: considering symbol " << UniqueName << " for function\n"); + if (Address == Section->getAddress() + Section->getSize()) { + assert(SymbolSize == 0 && + "unexpect non-zero sized symbol at end of section"); + LLVM_DEBUG( + dbgs() + << "BOLT-DEBUG: rejecting as symbol points to end of its section\n"); + registerName(SymbolSize); + continue; + } + if (!Section->isText()) { assert(SymbolType != SymbolRef::ST_Function && "unexpected function inside non-code section"); @@ -2074,6 +2084,19 @@ bool RewriteInstance::analyzeRelocation( } void RewriteInstance::processDynamicRelocations() { + // Read .relr.dyn section containing compressed R_*_RELATIVE relocations. + if (DynamicRelrSize > 0) { + ErrorOr DynamicRelrSectionOrErr = + BC->getSectionForAddress(*DynamicRelrAddress); + if (!DynamicRelrSectionOrErr) + report_error("unable to find section corresponding to DT_RELR", + DynamicRelrSectionOrErr.getError()); + if (DynamicRelrSectionOrErr->getSize() != DynamicRelrSize) + report_error("section size mismatch for DT_RELRSZ", + errc::executable_format_error); + readDynamicRelrRelocations(*DynamicRelrSectionOrErr); + } + // Read relocations for PLT - DT_JMPREL. if (PLTRelocationsSize > 0) { ErrorOr PLTRelSectionOrErr = @@ -2372,6 +2395,60 @@ void RewriteInstance::readDynamicRelocations(const SectionRef &Section, } } +void RewriteInstance::readDynamicRelrRelocations(BinarySection &Section) { + assert(Section.isAllocatable() && "allocatable expected"); + + LLVM_DEBUG({ + StringRef SectionName = Section.getName(); + dbgs() << "BOLT-DEBUG: reading relocations in section " << SectionName + << ":\n"; + }); + + const uint64_t RType = Relocation::getRelative(); + const uint8_t PSize = BC->AsmInfo->getCodePointerSize(); + const uint64_t MaxDelta = ((CHAR_BIT * DynamicRelrEntrySize) - 1) * PSize; + + auto ExtractAddendValue = [&](uint64_t Address) -> uint64_t { + ErrorOr Section = BC->getSectionForAddress(Address); + assert(Section && "cannot get section for data address from RELR"); + DataExtractor DE = DataExtractor(Section->getContents(), + BC->AsmInfo->isLittleEndian(), PSize); + uint64_t Offset = Address - Section->getAddress(); + return DE.getUnsigned(&Offset, PSize); + }; + + auto AddRelocation = [&](uint64_t Address) { + uint64_t Addend = ExtractAddendValue(Address); + LLVM_DEBUG(dbgs() << "BOLT-DEBUG: R_*_RELATIVE relocation at 0x" + << Twine::utohexstr(Address) << " to 0x" + << Twine::utohexstr(Addend) << '\n';); + BC->addDynamicRelocation(Address, nullptr, RType, Addend); + }; + + DataExtractor DE = DataExtractor(Section.getContents(), + BC->AsmInfo->isLittleEndian(), PSize); + uint64_t Offset = 0, Address = 0; + uint64_t RelrCount = DynamicRelrSize / DynamicRelrEntrySize; + while (RelrCount--) { + assert(DE.isValidOffset(Offset)); + uint64_t Entry = DE.getUnsigned(&Offset, DynamicRelrEntrySize); + if ((Entry & 1) == 0) { + AddRelocation(Entry); + Address = Entry + PSize; + } else { + const uint64_t StartAddress = Address; + while (Entry >>= 1) { + if (Entry & 1) + AddRelocation(Address); + + Address += PSize; + } + + Address = StartAddress + MaxDelta; + } + } +} + void RewriteInstance::printRelocationInfo(const RelocationRef &Rel, StringRef SymbolName, uint64_t SymbolAddress, @@ -5203,6 +5280,101 @@ void RewriteInstance::patchELFSymTabs(ELFObjectFile *File) { ELF::SHT_STRTAB); } +template +void RewriteInstance::patchELFAllocatableRelrSection( + ELFObjectFile *File) { + if (!DynamicRelrAddress) + return; + + raw_fd_ostream &OS = Out->os(); + const uint8_t PSize = BC->AsmInfo->getCodePointerSize(); + const uint64_t MaxDelta = ((CHAR_BIT * DynamicRelrEntrySize) - 1) * PSize; + + auto FixAddend = [&](const BinarySection &Section, const Relocation &Rel) { + // Fix relocation symbol value in place if no static relocation found + // on the same address + if (Section.getRelocationAt(Rel.Offset)) + return; + + // No fixup needed if symbol address was not changed + const uint64_t Addend = getNewFunctionOrDataAddress(Rel.Addend); + if (!Addend) + return; + + uint64_t FileOffset = Section.getOutputFileOffset(); + if (!FileOffset) + FileOffset = Section.getInputFileOffset(); + + FileOffset += Rel.Offset; + OS.pwrite(reinterpret_cast(&Addend), PSize, FileOffset); + }; + + // Fill new relative relocation offsets set + std::set RelOffsets; + for (const BinarySection &Section : BC->allocatableSections()) { + const uint64_t SectionInputAddress = Section.getAddress(); + uint64_t SectionAddress = Section.getOutputAddress(); + if (!SectionAddress) + SectionAddress = SectionInputAddress; + + for (const Relocation &Rel : Section.dynamicRelocations()) { + if (!Rel.isRelative()) + continue; + + uint64_t RelOffset = + getNewFunctionOrDataAddress(SectionInputAddress + Rel.Offset); + + RelOffset = RelOffset == 0 ? SectionAddress + Rel.Offset : RelOffset; + assert((RelOffset & 1) == 0 && "Wrong relocation offset"); + RelOffsets.emplace(RelOffset); + FixAddend(Section, Rel); + } + } + + ErrorOr Section = + BC->getSectionForAddress(*DynamicRelrAddress); + assert(Section && "cannot get .relr.dyn section"); + assert(Section->isRelr() && "Expected section to be SHT_RELR type"); + uint64_t RelrDynOffset = Section->getInputFileOffset(); + const uint64_t RelrDynEndOffset = RelrDynOffset + Section->getSize(); + + auto WriteRelr = [&](uint64_t Value) { + if (RelrDynOffset + DynamicRelrEntrySize > RelrDynEndOffset) { + errs() << "BOLT-ERROR: Offset overflow for relr.dyn section\n"; + exit(1); + } + + OS.pwrite(reinterpret_cast(&Value), DynamicRelrEntrySize, + RelrDynOffset); + RelrDynOffset += DynamicRelrEntrySize; + }; + + for (auto RelIt = RelOffsets.begin(); RelIt != RelOffsets.end();) { + WriteRelr(*RelIt); + uint64_t Base = *RelIt++ + PSize; + while (1) { + uint64_t Bitmap = 0; + for (; RelIt != RelOffsets.end(); ++RelIt) { + const uint64_t Delta = *RelIt - Base; + if (Delta >= MaxDelta || Delta % PSize) + break; + + Bitmap |= (1ULL << (Delta / PSize)); + } + + if (!Bitmap) + break; + + WriteRelr((Bitmap << 1) | 1); + Base += MaxDelta; + } + } + + // Fill the rest of the section with empty bitmap value + while (RelrDynOffset != RelrDynEndOffset) + WriteRelr(1); +} + template void RewriteInstance::patchELFAllocatableRelaSections(ELFObjectFile *File) { @@ -5294,8 +5466,11 @@ RewriteInstance::patchELFAllocatableRelaSections(ELFObjectFile *File) { } }; - // The dynamic linker expects R_*_RELATIVE relocations to be emitted first - writeRelocations(/* PatchRelative */ true); + // Place R_*_RELATIVE relocations in RELA section if RELR is not presented. + // The dynamic linker expects all R_*_RELATIVE relocations in RELA + // to be emitted first. + if (!DynamicRelrAddress) + writeRelocations(/* PatchRelative */ true); writeRelocations(/* PatchRelative */ false); auto fillNone = [&](uint64_t &Offset, uint64_t EndOffset) { @@ -5501,6 +5676,15 @@ Error RewriteInstance::readELFDynamic(ELFObjectFile *File) { case ELF::DT_RELACOUNT: DynamicRelativeRelocationsCount = Dyn.getVal(); break; + case ELF::DT_RELR: + DynamicRelrAddress = Dyn.getPtr(); + break; + case ELF::DT_RELRSZ: + DynamicRelrSize = Dyn.getVal(); + break; + case ELF::DT_RELRENT: + DynamicRelrEntrySize = Dyn.getVal(); + break; } } @@ -5513,6 +5697,20 @@ Error RewriteInstance::readELFDynamic(ELFObjectFile *File) { PLTRelocationsAddress.reset(); PLTRelocationsSize = 0; } + + if (!DynamicRelrAddress || !DynamicRelrSize) { + DynamicRelrAddress.reset(); + DynamicRelrSize = 0; + } else if (!DynamicRelrEntrySize) { + errs() << "BOLT-ERROR: expected DT_RELRENT to be presented " + << "in DYNAMIC section\n"; + exit(1); + } else if (DynamicRelrSize % DynamicRelrEntrySize) { + errs() << "BOLT-ERROR: expected RELR table size to be divisible " + << "by RELR entry size\n"; + exit(1); + } + return Error::success(); } @@ -5724,6 +5922,7 @@ void RewriteInstance::rewriteFile() { if (BC->HasRelocations) { patchELFAllocatableRelaSections(); + patchELFAllocatableRelrSection(); patchELFGOT(); } diff --git a/bolt/test/AArch64/constant_island_pie_update.s b/bolt/test/AArch64/constant_island_pie_update.s index 100f1a8074dee..c6856988d52f7 100644 --- a/bolt/test/AArch64/constant_island_pie_update.s +++ b/bolt/test/AArch64/constant_island_pie_update.s @@ -1,29 +1,51 @@ -// This test checks that the constant island value is updated if it -// has dynamic relocation. +// This test checks that the constant island offset and value is updated if +// it has dynamic relocation. The tests checks both .rela.dyn and relr.dyn +// dynamic relocations. // Also check that we don't duplicate CI if it has dynamic relocations. # RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o +// .rela.dyn # RUN: %clang %cflags -fPIC -pie %t.o -o %t.rela.exe -nostdlib \ # RUN: -Wl,-q -Wl,-z,notext # RUN: llvm-bolt %t.rela.exe -o %t.rela.bolt --use-old-text=0 --lite=0 # RUN: llvm-objdump -j .text -d %t.rela.bolt | FileCheck %s # RUN: llvm-readelf -rsW %t.rela.bolt | FileCheck --check-prefix=ELFCHECK %s +// .relr.dyn +# RUN: %clang %cflags -fPIC -pie %t.o -o %t.relr.exe -nostdlib \ +# RUN: -Wl,-q -Wl,-z,notext -Wl,--pack-dyn-relocs=relr +# RUN: llvm-bolt %t.relr.exe -o %t.relr.bolt --use-old-text=0 --lite=0 +# RUN: llvm-objdump -j .text -d %t.relr.bolt | FileCheck %s +# RUN: llvm-readelf -rsW %t.relr.bolt | FileCheck --check-prefix=ELFCHECK %s +# RUN: llvm-readelf -SW %t.relr.bolt | FileCheck --check-prefix=RELRSZCHECK %s // Check that the CI value was updated # CHECK: [[#%x,ADDR:]] : # CHECK: {{.*}} <$d>: # CHECK-NEXT: {{.*}} .word 0x{{[0]+}}[[#ADDR]] # CHECK-NEXT: {{.*}} .word 0x00000000 +# CHECK-NEXT: {{.*}} .word 0x{{[0]+}}[[#ADDR]] +# CHECK-NEXT: {{.*}} .word 0x00000000 +# CHECK-NEXT: {{.*}} .word 0x00000000 +# CHECK-NEXT: {{.*}} .word 0x00000000 +# CHECK-NEXT: {{.*}} .word 0x{{[0]+}}[[#ADDR]] +# CHECK-NEXT: {{.*}} .word 0x00000000 + // Check that we've relaxed adr to adrp + add to refer external CI # CHECK: : # CHECK-NEXT: adrp # CHECK-NEXT: add -// Check that relocation offset was updated +// Check that relocation offsets were updated # ELFCHECK: [[#%x,OFF:]] [[#%x,INFO_DYN:]] R_AARCH64_RELATIVE +# ELFCHECK-NEXT: [[#OFF + 8]] {{0*}}[[#INFO_DYN]] R_AARCH64_RELATIVE +# ELFCHECK-NEXT: [[#OFF + 24]] {{0*}}[[#INFO_DYN]] R_AARCH64_RELATIVE # ELFCHECK: {{.*}}[[#OFF]] {{.*}} $d +// Check that .relr.dyn size is 2 bytes to ensure that last 2 relocations were +// encoded as a bitmap so the total section size for 3 relocations is 2 bytes. +# RELRSZCHECK: .relr.dyn RELR [[#%x,ADDR:]] [[#%x,OFF:]] {{0*}}10 + .text .align 4 .local exitLocal @@ -47,6 +69,9 @@ _start: bl exitLocal nop .Lci: + .xword exitLocal + .xword exitLocal + .xword 0 .xword exitLocal .size _start, .-_start diff --git a/bolt/test/X86/section-end-sym.s b/bolt/test/X86/section-end-sym.s new file mode 100644 index 0000000000000..a9bca5604ec16 --- /dev/null +++ b/bolt/test/X86/section-end-sym.s @@ -0,0 +1,29 @@ +## Check that BOLT doesn't consider end-of-section symbols (e.g., _etext) as +## functions. + +# REQUIRES: system-linux + +# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-linux %s -o %t.o +# RUN: ld.lld %t.o -o %t.exe -q +# RUN: llvm-bolt %t.exe -o /dev/null --print-cfg --debug-only=bolt 2>&1 \ +# RUN: | FileCheck %s + +# CHECK: considering symbol etext for function +# CHECK-NEXT: rejecting as symbol points to end of its section +# CHECK-NOT: Binary Function "etext{{.*}}" after building cfg + + + .text + .globl _start + .type _start,@function +_start: + retq + .size _start, .-_start + + .align 0x1000 + .globl etext +etext: + + .data +.Lfoo: + .word 0 diff --git a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp index 133fcabd78392..8ba8b893e03a6 100644 --- a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp @@ -34,6 +34,11 @@ struct MagnitudeBits { bool operator<(const MagnitudeBits &Other) const noexcept { return WidthWithoutSignBit < Other.WidthWithoutSignBit; } + + bool operator!=(const MagnitudeBits &Other) const noexcept { + return WidthWithoutSignBit != Other.WidthWithoutSignBit || + BitFieldWidth != Other.BitFieldWidth; + } }; } // namespace @@ -184,13 +189,19 @@ void TooSmallLoopVariableCheck::check(const MatchFinder::MatchResult &Result) { if (LoopVar->getType() != LoopIncrement->getType()) return; - const QualType LoopVarType = LoopVar->getType(); - const QualType UpperBoundType = UpperBound->getType(); - ASTContext &Context = *Result.Context; + const QualType LoopVarType = LoopVar->getType(); const MagnitudeBits LoopVarMagnitudeBits = calcMagnitudeBits(Context, LoopVarType, LoopVar); + + const MagnitudeBits LoopIncrementMagnitudeBits = + calcMagnitudeBits(Context, LoopIncrement->getType(), LoopIncrement); + // We matched the loop variable incorrectly. + if (LoopIncrementMagnitudeBits != LoopVarMagnitudeBits) + return; + + const QualType UpperBoundType = UpperBound->getType(); const MagnitudeBits UpperBoundMagnitudeBits = calcUpperBoundMagnitudeBits(Context, UpperBound, UpperBoundType); diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp index 61421a13d73ba..81d05a8a93cd9 100644 --- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp @@ -230,9 +230,8 @@ static StringRef const HungarainNotationUserDefinedTypes[] = { // clang-format on IdentifierNamingCheck::NamingStyle::NamingStyle( - std::optional Case, - const std::string &Prefix, const std::string &Suffix, - const std::string &IgnoredRegexpStr, HungarianPrefixType HPType) + std::optional Case, StringRef Prefix, + StringRef Suffix, StringRef IgnoredRegexpStr, HungarianPrefixType HPType) : Case(Case), Prefix(Prefix), Suffix(Suffix), IgnoredRegexpStr(IgnoredRegexpStr), HPType(HPType) { if (!IgnoredRegexpStr.empty()) { @@ -265,22 +264,21 @@ IdentifierNamingCheck::FileStyle IdentifierNamingCheck::getFileStyleFromOptions( memcpy(&StyleString[StyleSize], "IgnoredRegexp", 13); StyleString.truncate(StyleSize + 13); - StringRef IgnoredRegexpStr = Options.get(StyleString, ""); + std::optional IgnoredRegexpStr = Options.get(StyleString); memcpy(&StyleString[StyleSize], "Prefix", 6); StyleString.truncate(StyleSize + 6); - std::string Prefix(Options.get(StyleString, "")); + std::optional Prefix(Options.get(StyleString)); // Fast replacement of [Pre]fix -> [Suf]fix. memcpy(&StyleString[StyleSize], "Suf", 3); - std::string Postfix(Options.get(StyleString, "")); + std::optional Postfix(Options.get(StyleString)); memcpy(&StyleString[StyleSize], "Case", 4); StyleString.pop_back_n(2); - auto CaseOptional = + std::optional CaseOptional = Options.get(StyleString); - if (CaseOptional || !Prefix.empty() || !Postfix.empty() || - !IgnoredRegexpStr.empty() || HPTOpt) - Styles[I].emplace(std::move(CaseOptional), std::move(Prefix), - std::move(Postfix), IgnoredRegexpStr.str(), + if (CaseOptional || Prefix || Postfix || IgnoredRegexpStr || HPTOpt) + Styles[I].emplace(std::move(CaseOptional), Prefix.value_or(""), + Postfix.value_or(""), IgnoredRegexpStr.value_or(""), HPTOpt.value_or(IdentifierNamingCheck::HPT_Off)); } bool IgnoreMainLike = Options.get("IgnoreMainLikeFunctions", false); @@ -650,7 +648,9 @@ std::string IdentifierNamingCheck::HungarianNotation::getClassPrefix( std::string IdentifierNamingCheck::HungarianNotation::getEnumPrefix( const EnumConstantDecl *ECD) const { - std::string Name = ECD->getType().getAsString(); + const EnumDecl *ED = cast(ECD->getDeclContext()); + + std::string Name = ED->getName().str(); if (std::string::npos != Name.find("enum")) { Name = Name.substr(strlen("enum"), Name.length() - strlen("enum")); Name = Name.erase(0, Name.find_first_not_of(" ")); diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h index cbf0653a280b6..b1db919902e22 100644 --- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h +++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h @@ -68,8 +68,8 @@ class IdentifierNamingCheck final : public RenamerClangTidyCheck { struct NamingStyle { NamingStyle() = default; - NamingStyle(std::optional Case, const std::string &Prefix, - const std::string &Suffix, const std::string &IgnoredRegexpStr, + NamingStyle(std::optional Case, StringRef Prefix, + StringRef Suffix, StringRef IgnoredRegexpStr, HungarianPrefixType HPType); NamingStyle(const NamingStyle &O) = delete; NamingStyle &operator=(NamingStyle &&O) = default; diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 98fa285f71051..260c44b2064b5 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -436,9 +436,8 @@ struct CodeCompletionBuilder { Bundled.emplace_back(); BundledEntry &S = Bundled.back(); if (C.SemaResult) { - bool IsPattern = C.SemaResult->Kind == CodeCompletionResult::RK_Pattern; - getSignature(*SemaCCS, &S.Signature, &S.SnippetSuffix, - &Completion.RequiredQualifier, IsPattern); + getSignature(*SemaCCS, &S.Signature, &S.SnippetSuffix, C.SemaResult->Kind, + C.SemaResult->CursorKind, &Completion.RequiredQualifier); if (!C.SemaResult->FunctionCanBeCall) S.SnippetSuffix.clear(); S.ReturnType = getReturnType(*SemaCCS); diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.cpp b/clang-tools-extra/clangd/CodeCompletionStrings.cpp index 21f83451f7014..26f8e0e602b2b 100644 --- a/clang-tools-extra/clangd/CodeCompletionStrings.cpp +++ b/clang-tools-extra/clangd/CodeCompletionStrings.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "CodeCompletionStrings.h" +#include "clang-c/Index.h" #include "clang/AST/ASTContext.h" #include "clang/AST/RawCommentList.h" #include "clang/Basic/SourceManager.h" @@ -56,6 +57,26 @@ bool looksLikeDocComment(llvm::StringRef CommentText) { return CommentText.find_first_not_of("/*-= \t\r\n") != llvm::StringRef::npos; } +// Determine whether the completion string should be patched +// to replace the last placeholder with $0. +bool shouldPatchPlaceholder0(CodeCompletionResult::ResultKind ResultKind, + CXCursorKind CursorKind) { + bool CompletingPattern = ResultKind == CodeCompletionResult::RK_Pattern; + + if (!CompletingPattern) + return false; + + // If the result kind of CodeCompletionResult(CCR) is `RK_Pattern`, it doesn't + // always mean we're completing a chunk of statements. Constructors defined + // in base class, for example, are considered as a type of pattern, with the + // cursor type set to CXCursor_Constructor. + if (CursorKind == CXCursorKind::CXCursor_Constructor || + CursorKind == CXCursorKind::CXCursor_Destructor) + return false; + + return true; +} + } // namespace std::string getDocComment(const ASTContext &Ctx, @@ -95,17 +116,20 @@ std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &Decl) { } void getSignature(const CodeCompletionString &CCS, std::string *Signature, - std::string *Snippet, std::string *RequiredQualifiers, - bool CompletingPattern) { - // Placeholder with this index will be ${0:…} to mark final cursor position. + std::string *Snippet, + CodeCompletionResult::ResultKind ResultKind, + CXCursorKind CursorKind, std::string *RequiredQualifiers) { + // Placeholder with this index will be $0 to mark final cursor position. // Usually we do not add $0, so the cursor is placed at end of completed text. unsigned CursorSnippetArg = std::numeric_limits::max(); - if (CompletingPattern) { - // In patterns, it's best to place the cursor at the last placeholder, to - // handle cases like - // namespace ${1:name} { - // ${0:decls} - // } + + // If the snippet contains a group of statements, we replace the + // last placeholder with $0 to leave the cursor there, e.g. + // namespace ${1:name} { + // ${0:decls} + // } + // We try to identify such cases using the ResultKind and CursorKind. + if (shouldPatchPlaceholder0(ResultKind, CursorKind)) { CursorSnippetArg = llvm::count_if(CCS, [](const CodeCompletionString::Chunk &C) { return C.Kind == CodeCompletionString::CK_Placeholder; @@ -185,7 +209,7 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature, *Snippet += Chunk.Text; break; case CodeCompletionString::CK_Optional: - assert(Chunk.Optional); + assert(Chunk.Optional); // No need to create placeholders for default arguments in Snippet. appendOptionalChunk(*Chunk.Optional, Signature); break; diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.h b/clang-tools-extra/clangd/CodeCompletionStrings.h index 6733d0231df49..566756ac9c8cc 100644 --- a/clang-tools-extra/clangd/CodeCompletionStrings.h +++ b/clang-tools-extra/clangd/CodeCompletionStrings.h @@ -42,12 +42,15 @@ std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &D); /// If set, RequiredQualifiers is the text that must be typed before the name. /// e.g "Base::" when calling a base class member function that's hidden. /// -/// When \p CompletingPattern is true, the last placeholder will be of the form -/// ${0:…}, indicating the cursor should stay there. +/// When \p ResultKind is RK_Pattern, the last placeholder will be $0, +/// indicating the cursor should stay there. +/// Note that for certain \p CursorKind like \p CXCursor_Constructor, $0 won't +/// be emitted in order to avoid overlapping normal parameters. void getSignature(const CodeCompletionString &CCS, std::string *Signature, std::string *Snippet, - std::string *RequiredQualifiers = nullptr, - bool CompletingPattern = false); + CodeCompletionResult::ResultKind ResultKind, + CXCursorKind CursorKind, + std::string *RequiredQualifiers = nullptr); /// Assembles formatted documentation for a completion result. This includes /// documentation comments and other relevant information like annotations. diff --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp index bcd39b2d38ba5..39b180e002a68 100644 --- a/clang-tools-extra/clangd/CompileCommands.cpp +++ b/clang-tools-extra/clangd/CompileCommands.cpp @@ -14,6 +14,7 @@ #include "clang/Driver/Options.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Tooling/CompilationDatabase.h" +#include "clang/Tooling/Tooling.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallVector.h" @@ -27,6 +28,7 @@ #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Path.h" #include "llvm/Support/Program.h" +#include "llvm/TargetParser/Host.h" #include #include #include @@ -185,6 +187,12 @@ static std::string resolveDriver(llvm::StringRef Driver, bool FollowSymlink, } // namespace +CommandMangler::CommandMangler() { + Tokenizer = llvm::Triple(llvm::sys::getProcessTriple()).isOSWindows() + ? llvm::cl::TokenizeWindowsCommandLine + : llvm::cl::TokenizeGNUCommandLine; +} + CommandMangler CommandMangler::detect() { CommandMangler Result; Result.ClangPath = detectClangPath(); @@ -201,9 +209,18 @@ void CommandMangler::operator()(tooling::CompileCommand &Command, trace::Span S("AdjustCompileFlags"); // Most of the modifications below assumes the Cmd starts with a driver name. // We might consider injecting a generic driver name like "cc" or "c++", but - // a Cmd missing the driver is probably rare enough in practice and errnous. + // a Cmd missing the driver is probably rare enough in practice and erroneous. if (Cmd.empty()) return; + + // FS used for expanding response files. + // FIXME: ExpandResponseFiles appears not to provide the usual + // thread-safety guarantees, as the access to FS is not locked! + // For now, use the real FS, which is known to be threadsafe (if we don't + // use/change working directory, which ExpandResponseFiles doesn't). + auto FS = llvm::vfs::getRealFileSystem(); + tooling::addExpandedResponseFiles(Cmd, Command.Directory, Tokenizer, *FS); + auto &OptTable = clang::driver::getDriverOptTable(); // OriginalArgs needs to outlive ArgList. llvm::SmallVector OriginalArgs; @@ -212,7 +229,7 @@ void CommandMangler::operator()(tooling::CompileCommand &Command, OriginalArgs.push_back(S.c_str()); bool IsCLMode = driver::IsClangCL(driver::getDriverMode( OriginalArgs[0], llvm::ArrayRef(OriginalArgs).slice(1))); - // ParseArgs propagates missig arg/opt counts on error, but preserves + // ParseArgs propagates missing arg/opt counts on error, but preserves // everything it could parse in ArgList. So we just ignore those counts. unsigned IgnoredCount; // Drop the executable name, as ParseArgs doesn't expect it. This means @@ -307,12 +324,16 @@ void CommandMangler::operator()(tooling::CompileCommand &Command, // necessary for the system include extractor to identify the file type // - AFTER applying CompileFlags.Edits, because the name of the compiler // that needs to be invoked may come from the CompileFlags->Compiler key + // - BEFORE addTargetAndModeForProgramName(), because gcc doesn't support + // the target flag that might be added. // - BEFORE resolveDriver() because that can mess up the driver path, // e.g. changing gcc to /path/to/clang/bin/gcc if (SystemIncludeExtractor) { SystemIncludeExtractor(Command, File); } + tooling::addTargetAndModeForProgramName(Cmd, Cmd.front()); + // Check whether the flag exists, either as -flag or -flag=* auto Has = [&](llvm::StringRef Flag) { for (llvm::StringRef Arg : Cmd) { diff --git a/clang-tools-extra/clangd/CompileCommands.h b/clang-tools-extra/clangd/CompileCommands.h index eb318d18baf63..1b37f44f0b9db 100644 --- a/clang-tools-extra/clangd/CompileCommands.h +++ b/clang-tools-extra/clangd/CompileCommands.h @@ -12,6 +12,7 @@ #include "support/Threading.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/CommandLine.h" #include #include #include @@ -50,10 +51,11 @@ struct CommandMangler { llvm::StringRef TargetFile) const; private: - CommandMangler() = default; + CommandMangler(); Memoize> ResolvedDrivers; Memoize> ResolvedDriversNoFollow; + llvm::cl::TokenizerCallback Tokenizer; }; // Removes args from a command-line in a semantically-aware way. diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index 2eab7ca27033e..790ee9af8f4ac 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -223,8 +223,8 @@ std::string getSymbolDetail(ASTContext &Ctx, const NamedDecl &ND) { std::optional declToSym(ASTContext &Ctx, const NamedDecl &ND) { auto &SM = Ctx.getSourceManager(); - SourceLocation BeginLoc = SM.getSpellingLoc(SM.getFileLoc(ND.getBeginLoc())); - SourceLocation EndLoc = SM.getSpellingLoc(SM.getFileLoc(ND.getEndLoc())); + SourceLocation BeginLoc = SM.getFileLoc(ND.getBeginLoc()); + SourceLocation EndLoc = SM.getFileLoc(ND.getEndLoc()); const auto SymbolRange = toHalfOpenFileRange(SM, Ctx.getLangOpts(), {BeginLoc, EndLoc}); if (!SymbolRange) diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp index def43683e0ab7..d1833759917a3 100644 --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp @@ -244,15 +244,7 @@ static std::unique_ptr parseJSON(PathRef Path, llvm::StringRef Data, std::string &Error) { if (auto CDB = tooling::JSONCompilationDatabase::loadFromBuffer( Data, Error, tooling::JSONCommandLineSyntax::AutoDetect)) { - // FS used for expanding response files. - // FIXME: ExpandResponseFilesDatabase appears not to provide the usual - // thread-safety guarantees, as the access to FS is not locked! - // For now, use the real FS, which is known to be threadsafe (if we don't - // use/change working directory, which ExpandResponseFilesDatabase doesn't). - auto FS = llvm::vfs::getRealFileSystem(); - return tooling::inferTargetAndDriverMode( - tooling::inferMissingCompileCommands( - expandResponseFiles(std::move(CDB), std::move(FS)))); + return tooling::inferMissingCompileCommands(std::move(CDB)); } return nullptr; } diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index 19f79f793420d..2122cb86c2834 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -156,7 +156,7 @@ std::optional kindForDecl(const NamedDecl *D, return HighlightingKind::Concept; if (const auto *UUVD = dyn_cast(D)) { auto Targets = Resolver->resolveUsingValueDecl(UUVD); - if (!Targets.empty()) { + if (!Targets.empty() && Targets[0] != UUVD) { return kindForDecl(Targets[0], Resolver); } return HighlightingKind::Unknown; diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 1b1220c8aa7a0..d4442c11a8ed0 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1611,8 +1611,8 @@ declToHierarchyItem(const NamedDecl &ND, llvm::StringRef TUPath) { ASTContext &Ctx = ND.getASTContext(); auto &SM = Ctx.getSourceManager(); SourceLocation NameLoc = nameLocation(ND, Ctx.getSourceManager()); - SourceLocation BeginLoc = SM.getSpellingLoc(SM.getFileLoc(ND.getBeginLoc())); - SourceLocation EndLoc = SM.getSpellingLoc(SM.getFileLoc(ND.getEndLoc())); + SourceLocation BeginLoc = SM.getFileLoc(ND.getBeginLoc()); + SourceLocation EndLoc = SM.getFileLoc(ND.getEndLoc()); const auto DeclRange = toHalfOpenFileRange(SM, Ctx.getLangOpts(), {BeginLoc, EndLoc}); if (!DeclRange) diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 24d01043a42d6..519aceec15a18 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -687,8 +687,10 @@ bool SymbolCollector::handleMacroOccurrence(const IdentifierInfo *Name, const auto &SM = PP->getSourceManager(); auto DefLoc = MI->getDefinitionLoc(); - // Also avoid storing predefined macros like __DBL_MIN__. + // Also avoid storing macros that aren't defined in any file, i.e. predefined + // macros like __DBL_MIN__ and those defined on the command line. if (SM.isWrittenInBuiltinFile(DefLoc) || + SM.isWrittenInCommandLineFile(DefLoc) || Name->getName() == "__GCC_HAVE_DWARF2_CFI_ASM") return true; @@ -756,7 +758,8 @@ bool SymbolCollector::handleMacroOccurrence(const IdentifierInfo *Name, *PP, *CompletionAllocator, *CompletionTUInfo); std::string Signature; std::string SnippetSuffix; - getSignature(*CCS, &Signature, &SnippetSuffix); + getSignature(*CCS, &Signature, &SnippetSuffix, SymbolCompletion.Kind, + SymbolCompletion.CursorKind); S.Signature = Signature; S.CompletionSnippetSuffix = SnippetSuffix; @@ -933,7 +936,8 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID, S.Documentation = Documentation; std::string Signature; std::string SnippetSuffix; - getSignature(*CCS, &Signature, &SnippetSuffix); + getSignature(*CCS, &Signature, &SnippetSuffix, SymbolCompletion.Kind, + SymbolCompletion.CursorKind); S.Signature = Signature; S.CompletionSnippetSuffix = SnippetSuffix; std::string ReturnType = getReturnType(*CCS); diff --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp index 103e13f44d060..1e51d8fb9a518 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp @@ -8,10 +8,25 @@ #include "AST.h" #include "Config.h" +#include "SourceCode.h" #include "refactor/Tweak.h" #include "support/Logger.h" #include "clang/AST/Decl.h" +#include "clang/AST/Expr.h" +#include "clang/AST/NestedNameSpecifier.h" #include "clang/AST/RecursiveASTVisitor.h" +#include "clang/AST/Type.h" +#include "clang/AST/TypeLoc.h" +#include "clang/Basic/LLVM.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Tooling/Core/Replacement.h" +#include "clang/Tooling/Syntax/Tokens.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/FormatVariadic.h" +#include "llvm/Support/raw_ostream.h" +#include +#include +#include namespace clang { namespace clangd { @@ -45,8 +60,12 @@ class AddUsing : public Tweak { // All of the following are set by prepare(). // The qualifier to remove. NestedNameSpecifierLoc QualifierToRemove; - // The name following QualifierToRemove. - llvm::StringRef Name; + // Qualified name to use when spelling the using declaration. This might be + // different than SpelledQualifier in presence of error correction. + std::string QualifierToSpell; + // The name and qualifier as spelled in the code. + llvm::StringRef SpelledQualifier; + llvm::StringRef SpelledName; // If valid, the insertion point for "using" statement must come after this. // This is relevant when the type is defined in the main file, to make sure // the type/function is already defined at the point where "using" is added. @@ -56,7 +75,7 @@ REGISTER_TWEAK(AddUsing) std::string AddUsing::title() const { return std::string(llvm::formatv( - "Add using-declaration for {0} and remove qualifier", Name)); + "Add using-declaration for {0} and remove qualifier", SpelledName)); } // Locates all "using" statements relevant to SelectionDeclContext. @@ -269,36 +288,23 @@ bool AddUsing::prepare(const Selection &Inputs) { if (Node == nullptr) return false; + SourceRange SpelledNameRange; if (auto *D = Node->ASTNode.get()) { if (auto *II = D->getDecl()->getIdentifier()) { QualifierToRemove = D->getQualifierLoc(); - Name = II->getName(); + SpelledNameRange = D->getSourceRange(); MustInsertAfterLoc = D->getDecl()->getBeginLoc(); } } else if (auto *T = Node->ASTNode.get()) { if (auto E = T->getAs()) { QualifierToRemove = E.getQualifierLoc(); - if (!QualifierToRemove) - return false; - auto NameRange = E.getSourceRange(); + SpelledNameRange = E.getSourceRange(); if (auto T = E.getNamedTypeLoc().getAs()) { // Remove the template arguments from the name. - NameRange.setEnd(T.getLAngleLoc().getLocWithOffset(-1)); + SpelledNameRange.setEnd(T.getLAngleLoc().getLocWithOffset(-1)); } - auto SpelledTokens = TB.spelledForExpanded(TB.expandedTokens(NameRange)); - if (!SpelledTokens) - return false; - auto SpelledRange = syntax::Token::range(SM, SpelledTokens->front(), - SpelledTokens->back()); - Name = SpelledRange.text(SM); - - std::string QualifierToRemoveStr = getNNSLAsString( - QualifierToRemove, Inputs.AST->getASTContext().getPrintingPolicy()); - if (!Name.consume_front(QualifierToRemoveStr)) - return false; // What's spelled doesn't match the qualifier. - if (const auto *ET = E.getTypePtr()) { if (const auto *TDT = dyn_cast(ET->getNamedType().getTypePtr())) { @@ -309,19 +315,14 @@ bool AddUsing::prepare(const Selection &Inputs) { } } } - - // FIXME: This only supports removing qualifiers that are made up of just - // namespace names. If qualifier contains a type, we could take the longest - // namespace prefix and remove that. - if (!QualifierToRemove.hasQualifier() || + if (!QualifierToRemove || + // FIXME: This only supports removing qualifiers that are made up of just + // namespace names. If qualifier contains a type, we could take the + // longest namespace prefix and remove that. !QualifierToRemove.getNestedNameSpecifier()->getAsNamespace() || - Name.empty()) { - return false; - } - - if (isNamespaceForbidden(Inputs, *QualifierToRemove.getNestedNameSpecifier())) + // Respect user config. + isNamespaceForbidden(Inputs, *QualifierToRemove.getNestedNameSpecifier())) return false; - // Macros are difficult. We only want to offer code action when what's spelled // under the cursor is a namespace qualifier. If it's a macro that expands to // a qualifier, user would not know what code action will actually change. @@ -333,23 +334,35 @@ bool AddUsing::prepare(const Selection &Inputs) { return false; } + auto SpelledTokens = + TB.spelledForExpanded(TB.expandedTokens(SpelledNameRange)); + if (!SpelledTokens) + return false; + auto SpelledRange = + syntax::Token::range(SM, SpelledTokens->front(), SpelledTokens->back()); + // We only drop qualifiers that're namespaces, so this is safe. + std::tie(SpelledQualifier, SpelledName) = + splitQualifiedName(SpelledRange.text(SM)); + QualifierToSpell = getNNSLAsString( + QualifierToRemove, Inputs.AST->getASTContext().getPrintingPolicy()); + if (!llvm::StringRef(QualifierToSpell).endswith(SpelledQualifier) || + SpelledName.empty()) + return false; // What's spelled doesn't match the qualifier. return true; } Expected AddUsing::apply(const Selection &Inputs) { auto &SM = Inputs.AST->getSourceManager(); - std::string QualifierToRemoveStr = getNNSLAsString( - QualifierToRemove, Inputs.AST->getASTContext().getPrintingPolicy()); tooling::Replacements R; if (auto Err = R.add(tooling::Replacement( SM, SM.getSpellingLoc(QualifierToRemove.getBeginLoc()), - QualifierToRemoveStr.length(), ""))) { + SpelledQualifier.size(), ""))) { return std::move(Err); } - auto InsertionPoint = - findInsertionPoint(Inputs, QualifierToRemove, Name, MustInsertAfterLoc); + auto InsertionPoint = findInsertionPoint(Inputs, QualifierToRemove, + SpelledName, MustInsertAfterLoc); if (!InsertionPoint) { return InsertionPoint.takeError(); } @@ -362,7 +375,7 @@ Expected AddUsing::apply(const Selection &Inputs) { if (InsertionPoint->AlwaysFullyQualify && !isFullyQualified(QualifierToRemove.getNestedNameSpecifier())) UsingTextStream << "::"; - UsingTextStream << QualifierToRemoveStr << Name << ";" + UsingTextStream << QualifierToSpell << SpelledName << ";" << InsertionPoint->Suffix; assert(SM.getFileID(InsertionPoint->Loc) == SM.getMainFileID()); diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt index 39fd6ee85378d..056e5803d711b 100644 --- a/clang-tools-extra/clangd/unittests/CMakeLists.txt +++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -157,6 +157,7 @@ clang_target_link_libraries(ClangdTests clangLex clangSema clangSerialization + clangTesting clangTooling clangToolingCore clangToolingInclusions diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index f57f266c3c511..25d6a7b189ed5 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -3450,6 +3450,22 @@ TEST(CompletionTest, CursorInSnippets) { EXPECT_THAT(Results.Completions, Contains(AllOf(named("while_foo"), snippetSuffix("(${1:int a}, ${2:int b})")))); + + Results = completions(R"cpp( + struct Base { + Base(int a, int b) {} + }; + + struct Derived : Base { + Derived() : Base^ + }; + )cpp", + /*IndexSymbols=*/{}, Options); + // Constructors from base classes are a kind of pattern that shouldn't end + // with $0. + EXPECT_THAT(Results.Completions, + Contains(AllOf(named("Base"), + snippetSuffix("(${1:int a}, ${2:int b})")))); } TEST(CompletionTest, WorksWithNullType) { diff --git a/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp index 329a213c66984..faab6253e2832 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp @@ -24,11 +24,13 @@ class CompletionStringTest : public ::testing::Test { protected: void computeSignature(const CodeCompletionString &CCS, - bool CompletingPattern = false) { + CodeCompletionResult::ResultKind ResultKind = + CodeCompletionResult::ResultKind::RK_Declaration) { Signature.clear(); Snippet.clear(); - getSignature(CCS, &Signature, &Snippet, /*RequiredQualifier=*/nullptr, - CompletingPattern); + getSignature(CCS, &Signature, &Snippet, ResultKind, + /*CursorKind=*/CXCursorKind::CXCursor_NotImplemented, + /*RequiredQualifiers=*/nullptr); } std::shared_ptr Allocator; @@ -145,11 +147,12 @@ TEST_F(CompletionStringTest, SnippetsInPatterns) { Builder.AddChunk(CodeCompletionString::CK_SemiColon); return *Builder.TakeString(); }; - computeSignature(MakeCCS(), /*CompletingPattern=*/false); + computeSignature(MakeCCS()); EXPECT_EQ(Snippet, " ${1:name} = ${2:target};"); // When completing a pattern, the last placeholder holds the cursor position. - computeSignature(MakeCCS(), /*CompletingPattern=*/true); + computeSignature(MakeCCS(), + /*ResultKind=*/CodeCompletionResult::ResultKind::RK_Pattern); EXPECT_EQ(Snippet, " ${1:name} = $0;"); } diff --git a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp index be44ce59f6657..28f0d85d332ca 100644 --- a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp +++ b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp @@ -11,6 +11,7 @@ #include "TestFS.h" #include "support/Context.h" +#include "clang/Testing/CommandLineArgs.h" #include "clang/Tooling/ArgumentsAdjusters.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/STLExtras.h" @@ -20,6 +21,7 @@ #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" #include "llvm/Support/Process.h" +#include "llvm/Support/TargetSelect.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -41,15 +43,18 @@ using ::testing::Not; // Make use of all features and assert the exact command we get out. // Other tests just verify presence/absence of certain args. TEST(CommandMangler, Everything) { + llvm::InitializeAllTargetInfos(); // As in ClangdMain + std::string Target = getAnyTargetForTesting(); auto Mangler = CommandMangler::forTests(); Mangler.ClangPath = testPath("fake/clang"); Mangler.ResourceDir = testPath("fake/resources"); Mangler.Sysroot = testPath("fake/sysroot"); tooling::CompileCommand Cmd; - Cmd.CommandLine = {"clang++", "--", "foo.cc", "bar.cc"}; + Cmd.CommandLine = {Target + "-clang++", "--", "foo.cc", "bar.cc"}; Mangler(Cmd, "foo.cc"); EXPECT_THAT(Cmd.CommandLine, - ElementsAre(testPath("fake/clang++"), + ElementsAre(testPath("fake/" + Target + "-clang++"), + "--target=" + Target, "--driver-mode=g++", "-resource-dir=" + testPath("fake/resources"), "-isysroot", testPath("fake/sysroot"), "--", "foo.cc")); @@ -197,8 +202,26 @@ TEST(CommandMangler, ConfigEdits) { Mangler(Cmd, "foo.cc"); } // Edits are applied in given order and before other mangling and they always - // go before filename. - EXPECT_THAT(Cmd.CommandLine, ElementsAre(_, "--hello", "--", "FOO.CC")); + // go before filename. `--driver-mode=g++` here is in lower case because + // options inserted by addTargetAndModeForProgramName are not editable, + // see discussion in https://reviews.llvm.org/D138546 + EXPECT_THAT(Cmd.CommandLine, + ElementsAre(_, "--driver-mode=g++", "--hello", "--", "FOO.CC")); +} + +TEST(CommandMangler, ExpandedResponseFiles) { + SmallString<1024> Path; + int FD; + ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("args", "", FD, Path)); + llvm::raw_fd_ostream OutStream(FD, true); + OutStream << "-Wall"; + OutStream.close(); + + auto Mangler = CommandMangler::forTests(); + tooling::CompileCommand Cmd; + Cmd.CommandLine = {"clang", ("@" + Path).str(), "foo.cc"}; + Mangler(Cmd, "foo.cc"); + EXPECT_THAT(Cmd.CommandLine, ElementsAre(_, "-Wall", "--", "foo.cc")); } static std::string strip(llvm::StringRef Arg, llvm::StringRef Argv) { diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp index cba318df3d2fe..259efcf54a6b2 100644 --- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -1045,7 +1045,15 @@ sizeof...($TemplateParameter[[Elements]]); auto $LocalVariable_def[[s]] = $Operator[[new]] $Class[[Foo]]$Bracket[[<]]$TemplateParameter[[T]]$Bracket[[>]](); $Operator[[delete]] $LocalVariable[[s]]; } - )cpp"}; + )cpp", + // Recursive UsingValueDecl + R"cpp( + template $Bracket[[<]]int$Bracket[[>]] class $Class_def[[X]] { + template $Bracket[[<]]int$Bracket[[>]] class $Class_def[[Y]] { + using $Class[[Y]]$Bracket[[<]]0$Bracket[[>]]::$Unknown_dependentName[[xxx]]; + }; + }; + )cpp"}; for (const auto &TestCase : TestCases) // Mask off scope modifiers to keep the tests manageable. // They're tested separately. diff --git a/clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp index adfd018f56d27..f3a479a9a240f 100644 --- a/clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp +++ b/clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp @@ -8,8 +8,12 @@ #include "Config.h" #include "TweakTesting.h" -#include "gmock/gmock.h" +#include "support/Context.h" +#include "llvm/ADT/StringMap.h" +#include "llvm/ADT/StringRef.h" #include "gtest/gtest.h" +#include +#include namespace clang { namespace clangd { @@ -30,7 +34,7 @@ namespace one { void oo() {} template class tt {}; namespace two { -enum ee {}; +enum ee { ee_enum_value }; void ff() {} class cc { public: @@ -64,9 +68,6 @@ class cc { EXPECT_UNAVAILABLE(Header + "void fun() { ::ban::fo^o(); }"); EXPECT_AVAILABLE(Header + "void fun() { banana::fo^o(); }"); - // Do not offer code action on typo-corrections. - EXPECT_UNAVAILABLE(Header + "/*error-ok*/c^c C;"); - // NestedNameSpecifier, but no namespace. EXPECT_UNAVAILABLE(Header + "class Foo {}; class F^oo foo;"); @@ -96,16 +97,17 @@ TEST_F(AddUsingTest, Apply) { struct { llvm::StringRef TestSource; llvm::StringRef ExpectedSource; - } Cases[]{{ - // Function, no other using, namespace. - R"cpp( + } Cases[]{ + { + // Function, no other using, namespace. + R"cpp( #include "test.hpp" namespace { void fun() { ^one::two::ff(); } })cpp", - R"cpp( + R"cpp( #include "test.hpp" namespace {using one::two::ff; @@ -113,17 +115,17 @@ void fun() { ff(); } })cpp", - }, - // Type, no other using, namespace. - { - R"cpp( + }, + // Type, no other using, namespace. + { + R"cpp( #include "test.hpp" namespace { void fun() { ::one::t^wo::cc inst; } })cpp", - R"cpp( + R"cpp( #include "test.hpp" namespace {using ::one::two::cc; @@ -131,16 +133,16 @@ void fun() { cc inst; } })cpp", - }, - // Type, no other using, no namespace. - { - R"cpp( + }, + // Type, no other using, no namespace. + { + R"cpp( #include "test.hpp" void fun() { one::two::e^e inst; })cpp", - R"cpp( + R"cpp( #include "test.hpp" using one::two::ee; @@ -148,9 +150,9 @@ using one::two::ee; void fun() { ee inst; })cpp"}, - // Function, other usings. - { - R"cpp( + // Function, other usings. + { + R"cpp( #include "test.hpp" using one::two::cc; @@ -161,7 +163,7 @@ void fun() { one::two::f^f(); } })cpp", - R"cpp( + R"cpp( #include "test.hpp" using one::two::cc; @@ -172,10 +174,10 @@ void fun() { ff(); } })cpp", - }, - // Function, other usings inside namespace. - { - R"cpp( + }, + // Function, other usings inside namespace. + { + R"cpp( #include "test.hpp" using one::two::cc; @@ -188,7 +190,7 @@ void fun() { o^ne::oo(); } })cpp", - R"cpp( + R"cpp( #include "test.hpp" using one::two::cc; @@ -201,9 +203,9 @@ void fun() { oo(); } })cpp"}, - // Using comes after cursor. - { - R"cpp( + // Using comes after cursor. + { + R"cpp( #include "test.hpp" namespace { @@ -215,7 +217,7 @@ void fun() { using one::two::cc; })cpp", - R"cpp( + R"cpp( #include "test.hpp" namespace {using one::two::ff; @@ -228,14 +230,14 @@ void fun() { using one::two::cc; })cpp"}, - // Pointer type. - {R"cpp( + // Pointer type. + {R"cpp( #include "test.hpp" void fun() { one::two::c^c *p; })cpp", - R"cpp( + R"cpp( #include "test.hpp" using one::two::cc; @@ -243,8 +245,8 @@ using one::two::cc; void fun() { cc *p; })cpp"}, - // Namespace declared via macro. - {R"cpp( + // Namespace declared via macro. + {R"cpp( #include "test.hpp" #define NS_BEGIN(name) namespace name { @@ -254,7 +256,7 @@ void fun() { one::two::f^f(); } })cpp", - R"cpp( + R"cpp( #include "test.hpp" #define NS_BEGIN(name) namespace name { @@ -266,15 +268,15 @@ void fun() { ff(); } })cpp"}, - // Inside macro argument. - {R"cpp( + // Inside macro argument. + {R"cpp( #include "test.hpp" #define CALL(name) name() void fun() { CALL(one::t^wo::ff); })cpp", - R"cpp( + R"cpp( #include "test.hpp" #define CALL(name) name() @@ -283,15 +285,15 @@ using one::two::ff; void fun() { CALL(ff); })cpp"}, - // Parent namespace != lexical parent namespace - {R"cpp( + // Parent namespace != lexical parent namespace + {R"cpp( #include "test.hpp" namespace foo { void fun(); } void foo::fun() { one::two::f^f(); })cpp", - R"cpp( + R"cpp( #include "test.hpp" using one::two::ff; @@ -300,8 +302,8 @@ namespace foo { void fun(); } void foo::fun() { ff(); })cpp"}, - // If all other using are fully qualified, add :: - {R"cpp( + // If all other using are fully qualified, add :: + {R"cpp( #include "test.hpp" using ::one::two::cc; @@ -310,7 +312,7 @@ using ::one::two::ee; void fun() { one::two::f^f(); })cpp", - R"cpp( + R"cpp( #include "test.hpp" using ::one::two::cc; @@ -319,8 +321,8 @@ using ::one::two::ff;using ::one::two::ee; void fun() { ff(); })cpp"}, - // Make sure we don't add :: if it's already there - {R"cpp( + // Make sure we don't add :: if it's already there + {R"cpp( #include "test.hpp" using ::one::two::cc; @@ -329,7 +331,7 @@ using ::one::two::ee; void fun() { ::one::two::f^f(); })cpp", - R"cpp( + R"cpp( #include "test.hpp" using ::one::two::cc; @@ -338,8 +340,8 @@ using ::one::two::ff;using ::one::two::ee; void fun() { ff(); })cpp"}, - // If even one using doesn't start with ::, do not add it - {R"cpp( + // If even one using doesn't start with ::, do not add it + {R"cpp( #include "test.hpp" using ::one::two::cc; @@ -348,7 +350,7 @@ using one::two::ee; void fun() { one::two::f^f(); })cpp", - R"cpp( + R"cpp( #include "test.hpp" using ::one::two::cc; @@ -357,14 +359,14 @@ using one::two::ff;using one::two::ee; void fun() { ff(); })cpp"}, - // using alias; insert using for the spelled name. - {R"cpp( + // using alias; insert using for the spelled name. + {R"cpp( #include "test.hpp" void fun() { one::u^u u; })cpp", - R"cpp( + R"cpp( #include "test.hpp" using one::uu; @@ -372,29 +374,29 @@ using one::uu; void fun() { uu u; })cpp"}, - // using namespace. - {R"cpp( + // using namespace. + {R"cpp( #include "test.hpp" using namespace one; namespace { two::c^c C; })cpp", - R"cpp( + R"cpp( #include "test.hpp" using namespace one; namespace {using two::cc; cc C; })cpp"}, - // Type defined in main file, make sure using is after that. - {R"cpp( + // Type defined in main file, make sure using is after that. + {R"cpp( namespace xx { struct yy {}; } x^x::yy X; )cpp", - R"cpp( + R"cpp( namespace xx { struct yy {}; } @@ -403,8 +405,8 @@ using xx::yy; yy X; )cpp"}, - // Type defined in main file via "using", insert after that. - {R"cpp( + // Type defined in main file via "using", insert after that. + {R"cpp( #include "test.hpp" namespace xx { @@ -413,7 +415,7 @@ namespace xx { x^x::yy X; )cpp", - R"cpp( + R"cpp( #include "test.hpp" namespace xx { @@ -424,8 +426,8 @@ using xx::yy; yy X; )cpp"}, - // Using must come after function definition. - {R"cpp( + // Using must come after function definition. + {R"cpp( namespace xx { void yy(); } @@ -434,7 +436,7 @@ void fun() { x^x::yy(); } )cpp", - R"cpp( + R"cpp( namespace xx { void yy(); } @@ -445,28 +447,58 @@ void fun() { yy(); } )cpp"}, - // Existing using with non-namespace part. - {R"cpp( + // Existing using with non-namespace part. + {R"cpp( #include "test.hpp" using one::two::ee::ee_one; one::t^wo::cc c; )cpp", - R"cpp( + R"cpp( #include "test.hpp" using one::two::cc;using one::two::ee::ee_one; cc c; )cpp"}, - // Template (like std::vector). - {R"cpp( + // Template (like std::vector). + {R"cpp( #include "test.hpp" one::v^ec foo; )cpp", - R"cpp( + R"cpp( #include "test.hpp" using one::vec; vec foo; -)cpp"}}; +)cpp"}, + // Typo correction. + {R"cpp( +// error-ok +#include "test.hpp" +c^c C; +)cpp", + R"cpp( +// error-ok +#include "test.hpp" +using one::two::cc; + +cc C; +)cpp"}, + {R"cpp( +// error-ok +#include "test.hpp" +void foo() { + switch(one::two::ee{}) { case two::ee_^one:break; } +} +)cpp", + R"cpp( +// error-ok +#include "test.hpp" +using one::two::ee_one; + +void foo() { + switch(one::two::ee{}) { case ee_one:break; } +} +)cpp"}, + }; llvm::StringMap EditedFiles; for (const auto &Case : Cases) { ExtraFiles["test.hpp"] = R"cpp( @@ -484,6 +516,8 @@ class cc { using uu = two::cc; template struct vec {}; })cpp"; + // Typo correction is disabled in msvc-compatibility mode. + ExtraArgs.push_back("-fno-ms-compatibility"); EXPECT_EQ(apply(Case.TestSource, &EditedFiles), Case.ExpectedSource); } } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index b53516a742e2c..3f79e8e2a187a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -201,6 +201,12 @@ Changes in existing checks :doc:`readability-identifier-naming ` check. +- Updated the Hungarian prefixes for enums in C files to match those used in C++ + files for improved readability, as checked by :doc:`readability-identifier-naming + `. To preserve the previous + behavior of using `i` as the prefix for enum tags, set the `EnumConstantPrefix` + option to `i` instead of using `EnumConstantHungarianPrefix`. + - Fixed a false positive in :doc:`readability-container-size-empty ` check when comparing ``std::array`` objects to default constructed ones. The behavior for this and @@ -223,6 +229,11 @@ Changes in existing checks ` to understand that there is a sequence point between designated initializers. +- Fixed an issue in :doc:`readability-identifier-naming + ` when specifying an empty + string for ``Prefix`` or ``Suffix`` options could result in the style not + being used. + Removed checks ^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp index 7dcbf475e9869..68b6b217a2e01 100644 --- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp @@ -171,6 +171,13 @@ TEST(WalkAST, TemplateNames) { namespace ns {template struct S {}; } using ns::$explicit^S;)cpp", "^S x;"); + testWalk(R"cpp( + namespace ns { + template struct S { S(T);}; + template S(T t) -> S; + } + using ns::$explicit^S;)cpp", + "^S x(123);"); testWalk("template struct $explicit^S {};", R"cpp( template