[PATCH] D39903: [libclang] Allow pretty printing declarations
nik added a comment. Ping... Repository: rC Clang https://reviews.llvm.org/D39903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40481: [libclang] Fix cursors for arguments of Subscript and Call operators
nik added a comment. Ping... https://reviews.llvm.org/D40481 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41537: Optionally add code completion results for arrow instead of dot
yvvan added a comment. ping https://reviews.llvm.org/D41537 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41897: Fixing a crash in Sema.
jkorous-apple created this revision. jkorous-apple added a reviewer: arphaman. The original code is checking for inaccessible base classes but does not expect inheriting from template parameters (or dependent types in general) as these are not modelled by CXXRecord. Issue was at this line since getAsCXXRecord() returned nullptr: bool found = Class->isDerivedFrom(CanonicalBase->getAsCXXRecordDecl(), Paths); https://reviews.llvm.org/D41897 Files: Sema/SemaDeclCXX.cpp SemaCXX/base-class-ambiguity-check.cpp Index: SemaCXX/base-class-ambiguity-check.cpp === --- /dev/null +++ SemaCXX/base-class-ambiguity-check.cpp @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -fsyntax-only %s + +template +class Foo +{ +struct Base : T +{ }; + +struct Derived : Base, T +{ }; +}; \ No newline at end of file Index: Sema/SemaDeclCXX.cpp === --- Sema/SemaDeclCXX.cpp +++ Sema/SemaDeclCXX.cpp @@ -2417,9 +2417,17 @@ // Attach the remaining base class specifiers to the derived class. Class->setBases(Bases.data(), NumGoodBases); + // Check that the only base classes that are duplicate are virtual. for (unsigned idx = 0; idx < NumGoodBases; ++idx) { // Check whether this direct base is inaccessible due to ambiguity. QualType BaseType = Bases[idx]->getType(); + +// Skip all dependent types in templates being used as base specifiers. +// Checks below assume that base specifier is a CXXRecord. +if (BaseType->isDependentType()) { + continue; +} + CanQualType CanonicalBase = Context.getCanonicalType(BaseType) .getUnqualifiedType(); Index: SemaCXX/base-class-ambiguity-check.cpp === --- /dev/null +++ SemaCXX/base-class-ambiguity-check.cpp @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -fsyntax-only %s + +template +class Foo +{ +struct Base : T +{ }; + +struct Derived : Base, T +{ }; +}; \ No newline at end of file Index: Sema/SemaDeclCXX.cpp === --- Sema/SemaDeclCXX.cpp +++ Sema/SemaDeclCXX.cpp @@ -2417,9 +2417,17 @@ // Attach the remaining base class specifiers to the derived class. Class->setBases(Bases.data(), NumGoodBases); + // Check that the only base classes that are duplicate are virtual. for (unsigned idx = 0; idx < NumGoodBases; ++idx) { // Check whether this direct base is inaccessible due to ambiguity. QualType BaseType = Bases[idx]->getType(); + +// Skip all dependent types in templates being used as base specifiers. +// Checks below assume that base specifier is a CXXRecord. +if (BaseType->isDependentType()) { + continue; +} + CanQualType CanonicalBase = Context.getCanonicalType(BaseType) .getUnqualifiedType(); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41668: [clangd] Add static index for the global code completion.
hokein updated this revision to Diff 129245. hokein marked 3 inline comments as done. hokein added a comment. - make static index independent with dynamic index - don't clear results from clang's completion engine as we also intend to merge them into the final results eventually. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41668 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/index/MemIndex.cpp clangd/index/MemIndex.h clangd/tool/ClangdMain.cpp unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -453,12 +453,6 @@ std::unique_ptr simpleIndexFromSymbols( std::vector> Symbols) { - auto I = llvm::make_unique(); - struct Snapshot { -SymbolSlab Slab; -std::vector Pointers; - }; - auto Snap = std::make_shared(); SymbolSlab::Builder Slab; for (const auto &Pair : Symbols) { Symbol Sym; @@ -475,13 +469,7 @@ Sym.SymInfo.Kind = Pair.second; Slab.insert(Sym); } - Snap->Slab = std::move(Slab).build(); - for (auto &Iter : Snap->Slab) -Snap->Pointers.push_back(&Iter); - auto S = std::shared_ptr>(std::move(Snap), -&Snap->Pointers); - I->build(std::move(S)); - return std::move(I); + return MemIndex::build(std::move(Slab).build()); } TEST(CompletionTest, NoIndex) { @@ -496,6 +484,23 @@ EXPECT_THAT(Results.items, Has("No")); } +TEST(CompletionTest, StaticAndDynamicIndex) { + clangd::CodeCompleteOptions Opts; + auto StaticIdx = + simpleIndexFromSymbols({{"ns::XYZ", index::SymbolKind::Class}}); + Opts.StaticIndex = StaticIdx.get(); + auto DynamicIdx = + simpleIndexFromSymbols({{"ns::foo", index::SymbolKind::Function}}); + Opts.Index = DynamicIdx.get(); + + auto Results = completions(R"cpp( + void f() { ::ns::^ } + )cpp", + Opts); + EXPECT_THAT(Results.items, Contains(Labeled("[G]XYZ"))); + EXPECT_THAT(Results.items, Contains(Labeled("foo"))); +} + TEST(CompletionTest, SimpleIndexBased) { clangd::CodeCompleteOptions Opts; auto I = simpleIndexFromSymbols({{"ns::XYZ", index::SymbolKind::Class}, Index: clangd/tool/ClangdMain.cpp === --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -11,6 +11,7 @@ #include "JSONRPCDispatcher.h" #include "Path.h" #include "Trace.h" +#include "index/SymbolYAML.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" @@ -26,7 +27,24 @@ namespace { enum class PCHStorageFlag { Disk, Memory }; + +// Build an in-memory static index for global symbols from a YAML-format file. +// The size of global symbols should be relatively small, so that all symbols +// can be managed in memory. +std::unique_ptr BuildStaticIndex(llvm::StringRef YamlSymbolFile) { + auto Buffer = llvm::MemoryBuffer::getFile(YamlSymbolFile); + if (!Buffer) { +llvm::errs() << "Can't open " << YamlSymbolFile << "\n"; +return nullptr; + } + auto Slab = SymbolFromYAML(Buffer.get()->getBuffer()); + SymbolSlab::Builder SymsBuilder; + for (auto Sym : Slab) +SymsBuilder.insert(Sym); + + return MemIndex::build(std::move(SymsBuilder).build()); } +} // namespace static llvm::cl::opt CompileCommandsDir( "compile-commands-dir", @@ -97,6 +115,15 @@ "use index built from symbols in opened files"), llvm::cl::init(false), llvm::cl::Hidden); +static llvm::cl::opt YamlSymbolFile( +"yaml-symbol-file", +llvm::cl::desc( +"YAML-format global symbol file to build the static index. Clangd will " +"use the static index for global code completion.\n" +"WARNING: This option is experimental only, and will be removed " +"eventually. Don't rely on it."), +llvm::cl::init(""), llvm::cl::Hidden); + int main(int argc, char *argv[]) { llvm::cl::ParseCommandLineOptions(argc, argv, "clangd"); @@ -182,13 +209,16 @@ // Change stdin to binary to not lose \r\n on windows. llvm::sys::ChangeStdinToBinary(); + std::unique_ptr StaticIdx; + if (EnableIndexBasedCompletion && !YamlSymbolFile.empty()) +StaticIdx = BuildStaticIndex(YamlSymbolFile); clangd::CodeCompleteOptions CCOpts; CCOpts.EnableSnippets = EnableSnippets; CCOpts.IncludeIneligibleResults = IncludeIneligibleResults; // Initialize and run ClangdLSPServer. ClangdLSPServer LSPServer(Out, WorkerThreadsCount, StorePreamblesInMemory, CCOpts, ResourceDirRef, CompileCommandsDirPath, -EnableIndexBasedCompletion); +EnableIndexBasedCompletion, StaticIdx.get()); constexpr int
[PATCH] D41901: [clangd] Remove duplicates from code completion
ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added a subscriber: klimek. This patch removes hidden items from code completion. Hidden items are the ones that are hidden by other items (e.g., by other items in the child scopes). This patch addresses a particular problem of a duplicate completion item for the class in the following example: struct Adapter { void method(); }; void Adapter::method() { Adapter^ } We should probably investigate if there are other duplicates in completion and remove them, possibly adding assertions that it never happens. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41901 Files: clangd/CodeComplete.cpp unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -583,6 +583,22 @@ EXPECT_THAT(Results.items, Has("fo", CompletionItemKind::Function)); } +TEST(CompletionTest, NoDuplicates) { + auto Items = completions(R"cpp( +struct Adapter { + void method(); +}; + +void Adapter::method() { + Adapter^ +} + )cpp") + .items; + + // Make sure there are no duplicate entries of 'Adapter'. + EXPECT_THAT(Items, ElementsAre(Named("Adapter"), Named("~Adapter"))); +} + } // namespace } // namespace clangd } // namespace clang Index: clangd/CodeComplete.cpp === --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -294,6 +294,13 @@ std::priority_queue Candidates; for (unsigned I = 0; I < NumResults; ++I) { auto &Result = Results[I]; + // We drop hidden items, as they cannot be found by the lookup after + // inserting the corresponding completion item and only produce noise and + // duplicates in the completion list. However, there is one exception. If + // Result has a Qualifier which is non-informative, we can refer to an + // item by adding that qualifier, so we don't filter out this item. + if (Result.Hidden && (!Result.Qualifier || Result.QualifierIsInformative)) +continue; if (!ClangdOpts.IncludeIneligibleResults && (Result.Availability == CXAvailability_NotAvailable || Result.Availability == CXAvailability_NotAccessible)) Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -583,6 +583,22 @@ EXPECT_THAT(Results.items, Has("fo", CompletionItemKind::Function)); } +TEST(CompletionTest, NoDuplicates) { + auto Items = completions(R"cpp( +struct Adapter { + void method(); +}; + +void Adapter::method() { + Adapter^ +} + )cpp") + .items; + + // Make sure there are no duplicate entries of 'Adapter'. + EXPECT_THAT(Items, ElementsAre(Named("Adapter"), Named("~Adapter"))); +} + } // namespace } // namespace clangd } // namespace clang Index: clangd/CodeComplete.cpp === --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -294,6 +294,13 @@ std::priority_queue Candidates; for (unsigned I = 0; I < NumResults; ++I) { auto &Result = Results[I]; + // We drop hidden items, as they cannot be found by the lookup after + // inserting the corresponding completion item and only produce noise and + // duplicates in the completion list. However, there is one exception. If + // Result has a Qualifier which is non-informative, we can refer to an + // item by adding that qualifier, so we don't filter out this item. + if (Result.Hidden && (!Result.Qualifier || Result.QualifierIsInformative)) +continue; if (!ClangdOpts.IncludeIneligibleResults && (Result.Availability == CXAvailability_NotAvailable || Result.Availability == CXAvailability_NotAccessible)) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41897: Fixing a crash in Sema.
aaron.ballman added reviewers: rsmith, aaron.ballman. aaron.ballman added inline comments. Comment at: Sema/SemaDeclCXX.cpp:2426 +// Skip all dependent types in templates being used as base specifiers. +// Checks below assume that base specifier is a CXXRecord. +if (BaseType->isDependentType()) { that base -> that the base Comment at: Sema/SemaDeclCXX.cpp:2427-2429 +if (BaseType->isDependentType()) { + continue; +} You can elide the braces here. Comment at: SemaCXX/base-class-ambiguity-check.cpp:1 +// RUN: %clang_cc1 -fsyntax-only %s + This run line isn't testing anything. Since you're trying to ensure this doesn't crash, I would put `-verify` on the RUN line and `// expected-no-diagnostics` on the line below. Comment at: SemaCXX/base-class-ambiguity-check.cpp:9 + +struct Derived : Base, T +{ }; I would add a comment around here explaining that this used to crash. Comment at: SemaCXX/base-class-ambiguity-check.cpp:12 +}; \ No newline at end of file Can you add a newline at the end of the file, and then run the file through clang-format to properly format it? https://reviews.llvm.org/D41897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41823: [clangd] Add more filters for collected symbols.
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. LGTM Comment at: clangd/index/SymbolCollector.h:20 -// Collect all symbols from an AST. -// -// Clients (e.g. clangd) can use SymbolCollector together with -// index::indexTopLevelDecls to retrieve all symbols when the source file is -// changed. +/// \brief Collect all symbols from an AST. +/// We should also document what kinds of symbols we are collecting, since this patch has changed the behavior (only collect top-level symbols). Comment at: unittests/clangd/SymbolCollectorTests.cpp:165 +TEST_F(SymbolCollectorTest, IgnoreSymbolsInMainFile) { + const std::string Header = R"( +class Foo {}; nit: although the default value is false, I'd explicitly specify `CollectorOpts.IndexMainFiles = false;` (readers don't need to jump to another file to see the default value). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39903: [libclang] Allow pretty printing declarations
nik updated this revision to Diff 129258. nik added a comment. Rebased only, please review. Repository: rC Clang https://reviews.llvm.org/D39903 Files: include/clang-c/Index.h test/Index/print-display-names.cpp tools/c-index-test/c-index-test.c tools/libclang/CIndex.cpp tools/libclang/libclang.exports Index: tools/libclang/libclang.exports === --- tools/libclang/libclang.exports +++ tools/libclang/libclang.exports @@ -178,6 +178,8 @@ clang_getCursorCompletionString clang_getCursorDefinition clang_getCursorDisplayName +clang_getCursorPrintingPolicy +clang_getCursorPrettyPrinted clang_getCursorExtent clang_getCursorExceptionSpecificationType clang_getCursorKind @@ -359,3 +361,56 @@ clang_EvalResult_getAsDouble clang_EvalResult_getAsStr clang_EvalResult_dispose +clang_PrintingPolicy_dispose +clang_PrintingPolicy_getIndentation +clang_PrintingPolicy_getSuppressSpecifiers +clang_PrintingPolicy_getSuppressTagKeyword +clang_PrintingPolicy_getIncludeTagDefinition +clang_PrintingPolicy_getSuppressScope +clang_PrintingPolicy_getSuppressUnwrittenScope +clang_PrintingPolicy_getSuppressInitializers +clang_PrintingPolicy_getConstantArraySizeAsWritten +clang_PrintingPolicy_getAnonymousTagLocations +clang_PrintingPolicy_getSuppressStrongLifetime +clang_PrintingPolicy_getSuppressLifetimeQualifiers +clang_PrintingPolicy_getSuppressTemplateArgsInCXXConstructors +clang_PrintingPolicy_getBool +clang_PrintingPolicy_getRestrict +clang_PrintingPolicy_getAlignof +clang_PrintingPolicy_getUnderscoreAlignof +clang_PrintingPolicy_getUseVoidForZeroParams +clang_PrintingPolicy_getTerseOutput +clang_PrintingPolicy_getPolishForDeclaration +clang_PrintingPolicy_getHalf +clang_PrintingPolicy_getMSWChar +clang_PrintingPolicy_getIncludeNewlines +clang_PrintingPolicy_getMSVCFormatting +clang_PrintingPolicy_getConstantsAsWritten +clang_PrintingPolicy_getSuppressImplicitBase +clang_PrintingPolicy_getFullyQualifiedName +clang_PrintingPolicy_setIndentation +clang_PrintingPolicy_setSuppressSpecifiers +clang_PrintingPolicy_setSuppressTagKeyword +clang_PrintingPolicy_setIncludeTagDefinition +clang_PrintingPolicy_setSuppressScope +clang_PrintingPolicy_setSuppressUnwrittenScope +clang_PrintingPolicy_setSuppressInitializers +clang_PrintingPolicy_setConstantArraySizeAsWritten +clang_PrintingPolicy_setAnonymousTagLocations +clang_PrintingPolicy_setSuppressStrongLifetime +clang_PrintingPolicy_setSuppressLifetimeQualifiers +clang_PrintingPolicy_setSuppressTemplateArgsInCXXConstructors +clang_PrintingPolicy_setBool +clang_PrintingPolicy_setRestrict +clang_PrintingPolicy_setAlignof +clang_PrintingPolicy_setUnderscoreAlignof +clang_PrintingPolicy_setUseVoidForZeroParams +clang_PrintingPolicy_setTerseOutput +clang_PrintingPolicy_setPolishForDeclaration +clang_PrintingPolicy_setHalf +clang_PrintingPolicy_setMSWChar +clang_PrintingPolicy_setIncludeNewlines +clang_PrintingPolicy_setMSVCFormatting +clang_PrintingPolicy_setConstantsAsWritten +clang_PrintingPolicy_setSuppressImplicitBase +clang_PrintingPolicy_setFullyQualifiedName Index: tools/libclang/CIndex.cpp === --- tools/libclang/CIndex.cpp +++ tools/libclang/CIndex.cpp @@ -4706,6 +4706,107 @@ return cxstring::createSet(Manglings); } +CXPrintingPolicy clang_getCursorPrintingPolicy(CXCursor C) { + if (clang_Cursor_isNull(C)) +return 0; + return new PrintingPolicy(getCursorContext(C).getPrintingPolicy()); +} + +void clang_PrintingPolicy_dispose(CXPrintingPolicy policy) { + if (policy) +delete static_cast(policy); +} + +#define DEFINE_PRINTING_POLICY_GETTER(name)\ + unsigned clang_PrintingPolicy_get##name(CXPrintingPolicy policy) { \ +if (!policy) \ + return 0;\ +return static_cast(policy)->name;\ + } +DEFINE_PRINTING_POLICY_GETTER(Indentation) +DEFINE_PRINTING_POLICY_GETTER(SuppressSpecifiers) +DEFINE_PRINTING_POLICY_GETTER(SuppressTagKeyword) +DEFINE_PRINTING_POLICY_GETTER(IncludeTagDefinition) +DEFINE_PRINTING_POLICY_GETTER(SuppressScope) +DEFINE_PRINTING_POLICY_GETTER(SuppressUnwrittenScope) +DEFINE_PRINTING_POLICY_GETTER(SuppressInitializers) +DEFINE_PRINTING_POLICY_GETTER(ConstantArraySizeAsWritten) +DEFINE_PRINTING_POLICY_GETTER(AnonymousTagLocations) +DEFINE_PRINTING_POLICY_GETTER(SuppressStrongLifetime) +DEFINE_PRINTING_POLICY_GETTER(SuppressLifetimeQualifiers) +DEFINE_PRINTING_POLICY_GETTER(SuppressTemplateArgsInCXXConstructors) +DEFINE_PRINTING_POLICY_GETTER(Bool) +DEFINE_PRINTING_POLICY_GETTER(Restrict) +DEFINE_PRINTING_POLICY_GETTER(Alignof) +DEFINE_PRINTING_POLICY_GETTER(UnderscoreAlignof) +DEFINE_PRINTING_POLICY_GETTER(UseVoidForZeroParams) +DEFINE_PRINTING_POLICY_GETTER(TerseOutput) +DEFINE_PRINTING_POLICY_GETTER(PolishFor
Re: Re: r322112 - [OPENMP] Fix directive kind on stand-alone target data directives, NFC.
These changes are just cosmetic, it does not affect real codegen for target data directives. - Best regards, Alexey Bataev 09.01.2018 15:31, Jonas Hahnfeld via cfe-commits пишет: > Why is this NFC and doesn't change a test? > > Am 2018-01-09 20:59, schrieb Alexey Bataev via cfe-commits: >> Author: abataev >> Date: Tue Jan 9 11:59:25 2018 >> New Revision: 322112 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=322112&view=rev >> Log: >> [OPENMP] Fix directive kind on stand-alone target data directives, NFC. >> >> Modified: >> cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp >> >> Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=322112&r1=322111&r2=322112&view=diff >> >> == >> >> --- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original) >> +++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Tue Jan 9 11:59:25 2018 >> @@ -7662,7 +7662,7 @@ void CGOpenMPRuntime::emitTargetDataStan >> if (D.hasClausesOfKind()) >> CGF.EmitOMPTargetTaskBasedDirective(D, ThenGen, InputInfo); >> else >> - emitInlinedDirective(CGF, OMPD_target_update, ThenGen); >> + emitInlinedDirective(CGF, D.getDirectiveKind(), ThenGen); >> }; >> >> if (IfCond) >> >> >> ___ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > signature.asc Description: OpenPGP digital signature ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41668: [clangd] Add static index for the global code completion.
sammccall accepted this revision. sammccall added a comment. Nice! LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41668 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r322185 - [clangd] Remove duplicates from code completion
Author: ibiryukov Date: Wed Jan 10 05:51:09 2018 New Revision: 322185 URL: http://llvm.org/viewvc/llvm-project?rev=322185&view=rev Log: [clangd] Remove duplicates from code completion Summary: This patch removes hidden items from code completion. Items can be hidden, e.g., by other items in the child scopes. This patch addresses a particular problem of a duplicate completion item for the class in the following example: struct Adapter { void method(); }; void Adapter::method() { Adapter^ } We should probably investigate if there are other duplicates in completion and remove them, possibly adding assertions that it never happens. Reviewers: sammccall Reviewed By: sammccall Subscribers: cfe-commits, klimek Differential Revision: https://reviews.llvm.org/D41901 Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=322185&r1=322184&r2=322185&view=diff == --- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original) +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Jan 10 05:51:09 2018 @@ -294,6 +294,13 @@ public: std::priority_queue Candidates; for (unsigned I = 0; I < NumResults; ++I) { auto &Result = Results[I]; + // We drop hidden items, as they cannot be found by the lookup after + // inserting the corresponding completion item and only produce noise and + // duplicates in the completion list. However, there is one exception. If + // Result has a Qualifier which is non-informative, we can refer to an + // item by adding that qualifier, so we don't filter out this item. + if (Result.Hidden && (!Result.Qualifier || Result.QualifierIsInformative)) +continue; if (!ClangdOpts.IncludeIneligibleResults && (Result.Availability == CXAvailability_NotAvailable || Result.Availability == CXAvailability_NotAccessible)) Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=322185&r1=322184&r2=322185&view=diff == --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Wed Jan 10 05:51:09 2018 @@ -592,6 +592,22 @@ TEST(CompletionTest, ASTIndexMultiFile) Doc("Doooc"), Detail("void"; } +TEST(CompletionTest, NoDuplicates) { + auto Items = completions(R"cpp( +struct Adapter { + void method(); +}; + +void Adapter::method() { + Adapter^ +} + )cpp") + .items; + + // Make sure there are no duplicate entries of 'Adapter'. + EXPECT_THAT(Items, ElementsAre(Named("Adapter"), Named("~Adapter"))); +} + } // namespace } // namespace clangd } // namespace clang ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41901: [clangd] Remove duplicates from code completion
This revision was automatically updated to reflect the committed changes. Closed by commit rL322185: [clangd] Remove duplicates from code completion (authored by ibiryukov, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D41901?vs=129247&id=129264#toc Repository: rL LLVM https://reviews.llvm.org/D41901 Files: clang-tools-extra/trunk/clangd/CodeComplete.cpp clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp === --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp @@ -592,6 +592,22 @@ Doc("Doooc"), Detail("void"; } +TEST(CompletionTest, NoDuplicates) { + auto Items = completions(R"cpp( +struct Adapter { + void method(); +}; + +void Adapter::method() { + Adapter^ +} + )cpp") + .items; + + // Make sure there are no duplicate entries of 'Adapter'. + EXPECT_THAT(Items, ElementsAre(Named("Adapter"), Named("~Adapter"))); +} + } // namespace } // namespace clangd } // namespace clang Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp === --- clang-tools-extra/trunk/clangd/CodeComplete.cpp +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp @@ -294,6 +294,13 @@ std::priority_queue Candidates; for (unsigned I = 0; I < NumResults; ++I) { auto &Result = Results[I]; + // We drop hidden items, as they cannot be found by the lookup after + // inserting the corresponding completion item and only produce noise and + // duplicates in the completion list. However, there is one exception. If + // Result has a Qualifier which is non-informative, we can refer to an + // item by adding that qualifier, so we don't filter out this item. + if (Result.Hidden && (!Result.Qualifier || Result.QualifierIsInformative)) +continue; if (!ClangdOpts.IncludeIneligibleResults && (Result.Availability == CXAvailability_NotAvailable || Result.Availability == CXAvailability_NotAccessible)) Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp === --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp @@ -592,6 +592,22 @@ Doc("Doooc"), Detail("void"; } +TEST(CompletionTest, NoDuplicates) { + auto Items = completions(R"cpp( +struct Adapter { + void method(); +}; + +void Adapter::method() { + Adapter^ +} + )cpp") + .items; + + // Make sure there are no duplicate entries of 'Adapter'. + EXPECT_THAT(Items, ElementsAre(Named("Adapter"), Named("~Adapter"))); +} + } // namespace } // namespace clangd } // namespace clang Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp === --- clang-tools-extra/trunk/clangd/CodeComplete.cpp +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp @@ -294,6 +294,13 @@ std::priority_queue Candidates; for (unsigned I = 0; I < NumResults; ++I) { auto &Result = Results[I]; + // We drop hidden items, as they cannot be found by the lookup after + // inserting the corresponding completion item and only produce noise and + // duplicates in the completion list. However, there is one exception. If + // Result has a Qualifier which is non-informative, we can refer to an + // item by adding that qualifier, so we don't filter out this item. + if (Result.Hidden && (!Result.Qualifier || Result.QualifierIsInformative)) +continue; if (!ClangdOpts.IncludeIneligibleResults && (Result.Availability == CXAvailability_NotAvailable || Result.Availability == CXAvailability_NotAccessible)) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41668: [clangd] Add static index for the global code completion.
hokein updated this revision to Diff 129265. hokein added a comment. - Update comment - Only use Sema results if the scope specifier (`ns::^`) is not a namespace scope specifier. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41668 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/index/MemIndex.cpp clangd/index/MemIndex.h clangd/tool/ClangdMain.cpp unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -453,12 +453,6 @@ std::unique_ptr simpleIndexFromSymbols( std::vector> Symbols) { - auto I = llvm::make_unique(); - struct Snapshot { -SymbolSlab Slab; -std::vector Pointers; - }; - auto Snap = std::make_shared(); SymbolSlab::Builder Slab; for (const auto &Pair : Symbols) { Symbol Sym; @@ -475,13 +469,7 @@ Sym.SymInfo.Kind = Pair.second; Slab.insert(Sym); } - Snap->Slab = std::move(Slab).build(); - for (auto &Iter : Snap->Slab) -Snap->Pointers.push_back(&Iter); - auto S = std::shared_ptr>(std::move(Snap), -&Snap->Pointers); - I->build(std::move(S)); - return std::move(I); + return MemIndex::build(std::move(Slab).build()); } TEST(CompletionTest, NoIndex) { @@ -496,6 +484,23 @@ EXPECT_THAT(Results.items, Has("No")); } +TEST(CompletionTest, StaticAndDynamicIndex) { + clangd::CodeCompleteOptions Opts; + auto StaticIdx = + simpleIndexFromSymbols({{"ns::XYZ", index::SymbolKind::Class}}); + Opts.StaticIndex = StaticIdx.get(); + auto DynamicIdx = + simpleIndexFromSymbols({{"ns::foo", index::SymbolKind::Function}}); + Opts.Index = DynamicIdx.get(); + + auto Results = completions(R"cpp( + void f() { ::ns::^ } + )cpp", + Opts); + EXPECT_THAT(Results.items, Contains(Labeled("[G]XYZ"))); + EXPECT_THAT(Results.items, Contains(Labeled("foo"))); +} + TEST(CompletionTest, SimpleIndexBased) { clangd::CodeCompleteOptions Opts; auto I = simpleIndexFromSymbols({{"ns::XYZ", index::SymbolKind::Class}, Index: clangd/tool/ClangdMain.cpp === --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -11,6 +11,7 @@ #include "JSONRPCDispatcher.h" #include "Path.h" #include "Trace.h" +#include "index/SymbolYAML.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" @@ -26,7 +27,24 @@ namespace { enum class PCHStorageFlag { Disk, Memory }; + +// Build an in-memory static index for global symbols from a YAML-format file. +// The size of global symbols should be relatively small, so that all symbols +// can be managed in memory. +std::unique_ptr BuildStaticIndex(llvm::StringRef YamlSymbolFile) { + auto Buffer = llvm::MemoryBuffer::getFile(YamlSymbolFile); + if (!Buffer) { +llvm::errs() << "Can't open " << YamlSymbolFile << "\n"; +return nullptr; + } + auto Slab = SymbolFromYAML(Buffer.get()->getBuffer()); + SymbolSlab::Builder SymsBuilder; + for (auto Sym : Slab) +SymsBuilder.insert(Sym); + + return MemIndex::build(std::move(SymsBuilder).build()); } +} // namespace static llvm::cl::opt CompileCommandsDir( "compile-commands-dir", @@ -97,6 +115,15 @@ "use index built from symbols in opened files"), llvm::cl::init(false), llvm::cl::Hidden); +static llvm::cl::opt YamlSymbolFile( +"yaml-symbol-file", +llvm::cl::desc( +"YAML-format global symbol file to build the static index. Clangd will " +"use the static index for global code completion.\n" +"WARNING: This option is experimental only, and will be removed " +"eventually. Don't rely on it."), +llvm::cl::init(""), llvm::cl::Hidden); + int main(int argc, char *argv[]) { llvm::cl::ParseCommandLineOptions(argc, argv, "clangd"); @@ -182,13 +209,16 @@ // Change stdin to binary to not lose \r\n on windows. llvm::sys::ChangeStdinToBinary(); + std::unique_ptr StaticIdx; + if (EnableIndexBasedCompletion && !YamlSymbolFile.empty()) +StaticIdx = BuildStaticIndex(YamlSymbolFile); clangd::CodeCompleteOptions CCOpts; CCOpts.EnableSnippets = EnableSnippets; CCOpts.IncludeIneligibleResults = IncludeIneligibleResults; // Initialize and run ClangdLSPServer. ClangdLSPServer LSPServer(Out, WorkerThreadsCount, StorePreamblesInMemory, CCOpts, ResourceDirRef, CompileCommandsDirPath, -EnableIndexBasedCompletion); +EnableIndexBasedCompletion, StaticIdx.get()); constexpr int NoShutdownRequestErrorCode = 1; llvm::set_thread_name("clangd.main"); return LSPServer.run(std:
[PATCH] D41823: [clangd] Add more filters for collected symbols.
ilya-biryukov accepted this revision. ilya-biryukov added a comment. LGTM. I'd add a comment pointing to code in include-fixer we're borrowing, though. (See the relevant comment) Comment at: clangd/index/SymbolCollector.cpp:80 +: decl(unless(isExpansionInMainFile())), +hasDeclContext(anyOf(namespaceDecl(), translationUnitDecl(), +*D, *ASTCtx) ioeric wrote: > ilya-biryukov wrote: > > hokein wrote: > > > These ast matchers seems to be relatively simple, maybe we can use the > > > `Decl` methods to implement them at the moment. Or will we add more > > > complex filters in the future? > > I second this. Also the code would probably be more readable without > > matchers in this particular example. > Yes, I expect to borrow more matchers from include-fixer's find-all-symbols. > Also, although `isExpansionInMainFile` seems easy to implement, I am > inclined to reuse the existing code if possible. Could you add a comment pointing to the code in include fixer, so that everyone reading the code knows where the code comes from? (It's generally useful in case there are bugs and borrowing more code from include-fixer fixes that) Comment at: clangd/index/SymbolCollector.h:28 + struct Options { +bool IndexMainFiles = false; + }; ioeric wrote: > ilya-biryukov wrote: > > hokein wrote: > > > We need documentation for the options. > > Also, would naming it `OnlyMainFiles` make the intent of the option clearer? > Actually, `OnlyMainFiles` is the opposite. We always index header files but > optionally index main files. I've added comment to clarify... The comment looks good, thanks! Comment at: unittests/clangd/SymbolCollectorTests.cpp:107 const std::string Header = R"( -class Foo { - void f(); -}; +class Foo {}; void f1(); ioeric wrote: > ilya-biryukov wrote: > > Why do we remove `void f()`? Maybe assert that it's not in the list of > > symbols instead? > I was trying to limit the scope of each test case (there is a separate test > case `IgnoreClassMembers`). `UnorderedElementsAre` should be able to capture > unexpected symbols. Oh, sorry, I missed that the name of the test changed too. IMO, generally, adding new tests in seems like a better option if the original tests aren't broken. Comment at: unittests/clangd/SymbolCollectorTests.cpp:182 )"; - runSymbolCollector(/*Header=*/"", Main); + runSymbolCollector(Header, /*Main=*/""); EXPECT_THAT( ioeric wrote: > ilya-biryukov wrote: > > Why should we change this test? > By default, main files are not indexed, so I put the code in header. But we still get the symbols from main file in clangd, because we're asking to index main files in `indexAST`... Makes sense. Thanks for clarifying. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41668: [clangd] Add static index for the global code completion.
sammccall added inline comments. Comment at: clangd/CodeComplete.cpp:661 + if (CompletedName.SSInfo) { +// For a namespace scope specifier ("ns::"), we have disabled Sema from +// collecting completion results by setting IncludeNamespaceLevelDecls to I don't think we need this - we plan to avoid overlap by splitting on current file vs other files - i'll put together a patch to make sema do what we want. Until we've got that done, having some duplicate results is OK (this feature is experimental still). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41668 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41668: [clangd] Add static index for the global code completion.
hokein updated this revision to Diff 129272. hokein added a comment. Remove the temporary fix. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41668 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/index/MemIndex.cpp clangd/index/MemIndex.h clangd/tool/ClangdMain.cpp unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -453,12 +453,6 @@ std::unique_ptr simpleIndexFromSymbols( std::vector> Symbols) { - auto I = llvm::make_unique(); - struct Snapshot { -SymbolSlab Slab; -std::vector Pointers; - }; - auto Snap = std::make_shared(); SymbolSlab::Builder Slab; for (const auto &Pair : Symbols) { Symbol Sym; @@ -475,13 +469,7 @@ Sym.SymInfo.Kind = Pair.second; Slab.insert(Sym); } - Snap->Slab = std::move(Slab).build(); - for (auto &Iter : Snap->Slab) -Snap->Pointers.push_back(&Iter); - auto S = std::shared_ptr>(std::move(Snap), -&Snap->Pointers); - I->build(std::move(S)); - return std::move(I); + return MemIndex::build(std::move(Slab).build()); } TEST(CompletionTest, NoIndex) { @@ -496,6 +484,23 @@ EXPECT_THAT(Results.items, Has("No")); } +TEST(CompletionTest, StaticAndDynamicIndex) { + clangd::CodeCompleteOptions Opts; + auto StaticIdx = + simpleIndexFromSymbols({{"ns::XYZ", index::SymbolKind::Class}}); + Opts.StaticIndex = StaticIdx.get(); + auto DynamicIdx = + simpleIndexFromSymbols({{"ns::foo", index::SymbolKind::Function}}); + Opts.Index = DynamicIdx.get(); + + auto Results = completions(R"cpp( + void f() { ::ns::^ } + )cpp", + Opts); + EXPECT_THAT(Results.items, Contains(Labeled("[G]XYZ"))); + EXPECT_THAT(Results.items, Contains(Labeled("foo"))); +} + TEST(CompletionTest, SimpleIndexBased) { clangd::CodeCompleteOptions Opts; auto I = simpleIndexFromSymbols({{"ns::XYZ", index::SymbolKind::Class}, Index: clangd/tool/ClangdMain.cpp === --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -11,6 +11,7 @@ #include "JSONRPCDispatcher.h" #include "Path.h" #include "Trace.h" +#include "index/SymbolYAML.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" @@ -26,7 +27,24 @@ namespace { enum class PCHStorageFlag { Disk, Memory }; + +// Build an in-memory static index for global symbols from a YAML-format file. +// The size of global symbols should be relatively small, so that all symbols +// can be managed in memory. +std::unique_ptr BuildStaticIndex(llvm::StringRef YamlSymbolFile) { + auto Buffer = llvm::MemoryBuffer::getFile(YamlSymbolFile); + if (!Buffer) { +llvm::errs() << "Can't open " << YamlSymbolFile << "\n"; +return nullptr; + } + auto Slab = SymbolFromYAML(Buffer.get()->getBuffer()); + SymbolSlab::Builder SymsBuilder; + for (auto Sym : Slab) +SymsBuilder.insert(Sym); + + return MemIndex::build(std::move(SymsBuilder).build()); } +} // namespace static llvm::cl::opt CompileCommandsDir( "compile-commands-dir", @@ -97,6 +115,15 @@ "use index built from symbols in opened files"), llvm::cl::init(false), llvm::cl::Hidden); +static llvm::cl::opt YamlSymbolFile( +"yaml-symbol-file", +llvm::cl::desc( +"YAML-format global symbol file to build the static index. Clangd will " +"use the static index for global code completion.\n" +"WARNING: This option is experimental only, and will be removed " +"eventually. Don't rely on it."), +llvm::cl::init(""), llvm::cl::Hidden); + int main(int argc, char *argv[]) { llvm::cl::ParseCommandLineOptions(argc, argv, "clangd"); @@ -182,13 +209,16 @@ // Change stdin to binary to not lose \r\n on windows. llvm::sys::ChangeStdinToBinary(); + std::unique_ptr StaticIdx; + if (EnableIndexBasedCompletion && !YamlSymbolFile.empty()) +StaticIdx = BuildStaticIndex(YamlSymbolFile); clangd::CodeCompleteOptions CCOpts; CCOpts.EnableSnippets = EnableSnippets; CCOpts.IncludeIneligibleResults = IncludeIneligibleResults; // Initialize and run ClangdLSPServer. ClangdLSPServer LSPServer(Out, WorkerThreadsCount, StorePreamblesInMemory, CCOpts, ResourceDirRef, CompileCommandsDirPath, -EnableIndexBasedCompletion); +EnableIndexBasedCompletion, StaticIdx.get()); constexpr int NoShutdownRequestErrorCode = 1; llvm::set_thread_name("clangd.main"); return LSPServer.run(std::cin) ? 0 : NoShutdownRequestErrorCode; Index: clangd/index/MemIndex.h ==
[PATCH] D41668: [clangd] Add static index for the global code completion.
hokein marked an inline comment as done. hokein added inline comments. Comment at: clangd/CodeComplete.cpp:661 + if (CompletedName.SSInfo) { +// For a namespace scope specifier ("ns::"), we have disabled Sema from +// collecting completion results by setting IncludeNamespaceLevelDecls to sammccall wrote: > I don't think we need this - we plan to avoid overlap by splitting on current > file vs other files - i'll put together a patch to make sema do what we want. > Until we've got that done, having some duplicate results is OK (this feature > is experimental still). Thanks, that makes sense. Reverted it. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41668 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r322191 - [clangd] Add static index for the global code completion.
Author: hokein Date: Wed Jan 10 06:44:34 2018 New Revision: 322191 URL: http://llvm.org/viewvc/llvm-project?rev=322191&view=rev Log: [clangd] Add static index for the global code completion. Summary: Use the YAML-format symbols (generated by the global-symbol-builder tool) to do the global code completion. It is **experimental** only , but it allows us to experience global code completion on a relatively small project. Tested with LLVM project. Reviewers: sammccall, ioeric Reviewed By: sammccall, ioeric Subscribers: klimek, ilya-biryukov, cfe-commits Differential Revision: https://reviews.llvm.org/D41668 Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp clang-tools-extra/trunk/clangd/ClangdLSPServer.h clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/ClangdServer.h clang-tools-extra/trunk/clangd/CodeComplete.cpp clang-tools-extra/trunk/clangd/CodeComplete.h clang-tools-extra/trunk/clangd/index/MemIndex.cpp clang-tools-extra/trunk/clangd/index/MemIndex.h clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=322191&r1=322190&r2=322191&view=diff == --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Jan 10 06:44:34 2018 @@ -283,10 +283,12 @@ ClangdLSPServer::ClangdLSPServer(JSONOut const clangd::CodeCompleteOptions &CCOpts, llvm::Optional ResourceDir, llvm::Optional CompileCommandsDir, - bool BuildDynamicSymbolIndex) + bool BuildDynamicSymbolIndex, + SymbolIndex *StaticIdx) : Out(Out), CDB(std::move(CompileCommandsDir)), CCOpts(CCOpts), Server(CDB, /*DiagConsumer=*/*this, FSProvider, AsyncThreadsCount, - StorePreamblesInMemory, BuildDynamicSymbolIndex, ResourceDir) {} + StorePreamblesInMemory, BuildDynamicSymbolIndex, StaticIdx, + ResourceDir) {} bool ClangdLSPServer::run(std::istream &In) { assert(!IsDone && "Run was called before"); Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=322191&r1=322190&r2=322191&view=diff == --- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original) +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Wed Jan 10 06:44:34 2018 @@ -22,6 +22,7 @@ namespace clang { namespace clangd { class JSONOutput; +class SymbolIndex; /// This class provides implementation of an LSP server, glueing the JSON /// dispatch and ClangdServer together. @@ -35,7 +36,8 @@ public: const clangd::CodeCompleteOptions &CCOpts, llvm::Optional ResourceDir, llvm::Optional CompileCommandsDir, - bool BuildDynamicSymbolIndex); + bool BuildDynamicSymbolIndex, + SymbolIndex *StaticIdx = nullptr); /// Run LSP server loop, receiving input for it from \p In. \p In must be /// opened in binary mode. Output will be written using Out variable passed to Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=322191&r1=322190&r2=322191&view=diff == --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Wed Jan 10 06:44:34 2018 @@ -134,10 +134,11 @@ ClangdServer::ClangdServer(GlobalCompila FileSystemProvider &FSProvider, unsigned AsyncThreadsCount, bool StorePreamblesInMemory, - bool BuildDynamicSymbolIndex, + bool BuildDynamicSymbolIndex, SymbolIndex *StaticIdx, llvm::Optional ResourceDir) : CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider), FileIdx(BuildDynamicSymbolIndex ? new FileIndex() : nullptr), + StaticIdx(StaticIdx), // Pass a callback into `Units` to extract symbols from a newly parsed // file and rebuild the file index synchronously each time an AST is // parsed. @@ -251,6 +252,8 @@ void ClangdServer::codeComplete( auto CodeCompleteOpts = Opts; if (FileIdx) CodeCompleteOpts.Index = FileIdx.get(); + if (StaticIdx) +CodeCompleteOpts.StaticIndex = StaticIdx;
[PATCH] D41668: [clangd] Add static index for the global code completion.
This revision was automatically updated to reflect the committed changes. hokein marked an inline comment as done. Closed by commit rCTE322191: [clangd] Add static index for the global code completion. (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D41668?vs=129272&id=129274#toc Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41668 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/index/MemIndex.cpp clangd/index/MemIndex.h clangd/tool/ClangdMain.cpp unittests/clangd/CodeCompleteTests.cpp Index: clangd/ClangdLSPServer.h === --- clangd/ClangdLSPServer.h +++ clangd/ClangdLSPServer.h @@ -22,6 +22,7 @@ namespace clangd { class JSONOutput; +class SymbolIndex; /// This class provides implementation of an LSP server, glueing the JSON /// dispatch and ClangdServer together. @@ -35,7 +36,8 @@ const clangd::CodeCompleteOptions &CCOpts, llvm::Optional ResourceDir, llvm::Optional CompileCommandsDir, - bool BuildDynamicSymbolIndex); + bool BuildDynamicSymbolIndex, + SymbolIndex *StaticIdx = nullptr); /// Run LSP server loop, receiving input for it from \p In. \p In must be /// opened in binary mode. Output will be written using Out variable passed to Index: clangd/tool/ClangdMain.cpp === --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -11,6 +11,7 @@ #include "JSONRPCDispatcher.h" #include "Path.h" #include "Trace.h" +#include "index/SymbolYAML.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" @@ -26,7 +27,24 @@ namespace { enum class PCHStorageFlag { Disk, Memory }; + +// Build an in-memory static index for global symbols from a YAML-format file. +// The size of global symbols should be relatively small, so that all symbols +// can be managed in memory. +std::unique_ptr BuildStaticIndex(llvm::StringRef YamlSymbolFile) { + auto Buffer = llvm::MemoryBuffer::getFile(YamlSymbolFile); + if (!Buffer) { +llvm::errs() << "Can't open " << YamlSymbolFile << "\n"; +return nullptr; + } + auto Slab = SymbolFromYAML(Buffer.get()->getBuffer()); + SymbolSlab::Builder SymsBuilder; + for (auto Sym : Slab) +SymsBuilder.insert(Sym); + + return MemIndex::build(std::move(SymsBuilder).build()); } +} // namespace static llvm::cl::opt CompileCommandsDir( "compile-commands-dir", @@ -97,6 +115,15 @@ "use index built from symbols in opened files"), llvm::cl::init(false), llvm::cl::Hidden); +static llvm::cl::opt YamlSymbolFile( +"yaml-symbol-file", +llvm::cl::desc( +"YAML-format global symbol file to build the static index. Clangd will " +"use the static index for global code completion.\n" +"WARNING: This option is experimental only, and will be removed " +"eventually. Don't rely on it."), +llvm::cl::init(""), llvm::cl::Hidden); + int main(int argc, char *argv[]) { llvm::cl::ParseCommandLineOptions(argc, argv, "clangd"); @@ -182,13 +209,16 @@ // Change stdin to binary to not lose \r\n on windows. llvm::sys::ChangeStdinToBinary(); + std::unique_ptr StaticIdx; + if (EnableIndexBasedCompletion && !YamlSymbolFile.empty()) +StaticIdx = BuildStaticIndex(YamlSymbolFile); clangd::CodeCompleteOptions CCOpts; CCOpts.EnableSnippets = EnableSnippets; CCOpts.IncludeIneligibleResults = IncludeIneligibleResults; // Initialize and run ClangdLSPServer. ClangdLSPServer LSPServer(Out, WorkerThreadsCount, StorePreamblesInMemory, CCOpts, ResourceDirRef, CompileCommandsDirPath, -EnableIndexBasedCompletion); +EnableIndexBasedCompletion, StaticIdx.get()); constexpr int NoShutdownRequestErrorCode = 1; llvm::set_thread_name("clangd.main"); return LSPServer.run(std::cin) ? 0 : NoShutdownRequestErrorCode; Index: clangd/ClangdServer.cpp === --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -134,10 +134,11 @@ FileSystemProvider &FSProvider, unsigned AsyncThreadsCount, bool StorePreamblesInMemory, - bool BuildDynamicSymbolIndex, + bool BuildDynamicSymbolIndex, SymbolIndex *StaticIdx, llvm::Optional ResourceDir) : CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider), FileIdx(BuildDynamicSymbolIndex ? new FileIndex() : nullptr), + StaticIdx(StaticIdx), // Pass a callback into `Units`
[PATCH] D41823: [clangd] Add more filters for collected symbols.
ioeric updated this revision to Diff 129275. ioeric marked 2 inline comments as done. ioeric added a comment. - Merge branch 'master' of http://llvm.org/git/clang-tools-extra into collector - Address review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41823 Files: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp clangd/index/FileIndex.cpp clangd/index/SymbolCollector.cpp clangd/index/SymbolCollector.h unittests/clangd/FileIndexTests.cpp unittests/clangd/SymbolCollectorTests.cpp Index: unittests/clangd/SymbolCollectorTests.cpp === --- unittests/clangd/SymbolCollectorTests.cpp +++ unittests/clangd/SymbolCollectorTests.cpp @@ -53,20 +53,22 @@ namespace { class SymbolIndexActionFactory : public tooling::FrontendActionFactory { public: - SymbolIndexActionFactory() = default; + SymbolIndexActionFactory(SymbolCollector::Options COpts) + : COpts(std::move(COpts)) {} clang::FrontendAction *create() override { index::IndexingOptions IndexOpts; IndexOpts.SystemSymbolFilter = index::IndexingOptions::SystemSymbolFilterKind::All; IndexOpts.IndexFunctionLocals = false; -Collector = std::make_shared(); +Collector = std::make_shared(COpts); FrontendAction *Action = index::createIndexingAction(Collector, IndexOpts, nullptr).release(); return Action; } std::shared_ptr Collector; + SymbolCollector::Options COpts; }; class SymbolCollectorTest : public ::testing::Test { @@ -79,7 +81,7 @@ const std::string FileName = "symbol.cc"; const std::string HeaderName = "symbols.h"; -auto Factory = llvm::make_unique(); +auto Factory = llvm::make_unique(CollectorOpts); tooling::ToolInvocation Invocation( {"symbol_collector", "-fsyntax-only", "-std=c++11", FileName}, @@ -100,9 +102,11 @@ protected: SymbolSlab Symbols; + SymbolCollector::Options CollectorOpts; }; -TEST_F(SymbolCollectorTest, CollectSymbol) { +TEST_F(SymbolCollectorTest, CollectSymbols) { + CollectorOpts.IndexMainFiles = true; const std::string Header = R"( class Foo { void f(); @@ -144,7 +148,6 @@ EXPECT_THAT(Symbols, UnorderedElementsAreArray( {QName("Foo"), - QName("Foo::f"), QName("f1"), QName("f2"), QName("KInt"), @@ -158,29 +161,85 @@ QName("foo::baz")})); } -TEST_F(SymbolCollectorTest, SymbolWithDocumentation) { +TEST_F(SymbolCollectorTest, IgnoreSymbolsInMainFile) { + CollectorOpts.IndexMainFiles = false; + const std::string Header = R"( +class Foo {}; +void f1(); +inline void f2() {} + )"; + const std::string Main = R"( +namespace { +void ff() {} // ignore +} +void main_f() {} // ignore +void f1() {} + )"; + runSymbolCollector(Header, Main); + EXPECT_THAT(Symbols, + UnorderedElementsAre(QName("Foo"), QName("f1"), QName("f2"))); +} + +TEST_F(SymbolCollectorTest, IncludeSymbolsInMainFile) { + CollectorOpts.IndexMainFiles = true; + const std::string Header = R"( +class Foo {}; +void f1(); +inline void f2() {} + )"; + const std::string Main = R"( +namespace { +void ff() {} // ignore +} +void main_f() {} +void f1() {} + )"; + runSymbolCollector(Header, Main); + EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("f1"), +QName("f2"), QName("main_f"))); +} + +TEST_F(SymbolCollectorTest, IgnoreClassMembers) { + const std::string Header = R"( +class Foo { + void f() {} + void g(); + static void sf() {} + static void ssf(); + static int x; +}; + )"; const std::string Main = R"( +void Foo::g() {} +void Foo::ssf() {} + )"; + runSymbolCollector(Header, Main); + EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"))); +} + +TEST_F(SymbolCollectorTest, SymbolWithDocumentation) { + const std::string Header = R"( namespace nx { /// Foo comment. int ff(int x, double y) { return 0; } } )"; - runSymbolCollector(/*Header=*/"", Main); + runSymbolCollector(Header, /*Main=*/""); EXPECT_THAT(Symbols, UnorderedElementsAre(QName("nx"), AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Detail("int"), Doc("Foo comment."; } TEST_F(SymbolCollectorTest, PlainAndSnippet) { - const std::string Main = R"( + const std::string Header = R"( namespace nx { void f() {} int ff(int x, double y) { return 0; } } )"; - runSymbolCollector(/*Header=*/"", Main); + runSymbolCollector(Header, /*Main=*/""); EXPECT_THAT( Symbols, UnorderedElementsAre( Index: unittests/clangd/FileIndexTests.cpp ===
[PATCH] D41823: [clangd] Add more filters for collected symbols.
ioeric added inline comments. Comment at: clangd/index/SymbolCollector.cpp:80 +: decl(unless(isExpansionInMainFile())), +hasDeclContext(anyOf(namespaceDecl(), translationUnitDecl(), +*D, *ASTCtx) ilya-biryukov wrote: > ioeric wrote: > > ilya-biryukov wrote: > > > hokein wrote: > > > > These ast matchers seems to be relatively simple, maybe we can use the > > > > `Decl` methods to implement them at the moment. Or will we add more > > > > complex filters in the future? > > > I second this. Also the code would probably be more readable without > > > matchers in this particular example. > > Yes, I expect to borrow more matchers from include-fixer's > > find-all-symbols. Also, although `isExpansionInMainFile` seems easy to > > implement, I am inclined to reuse the existing code if possible. > Could you add a comment pointing to the code in include fixer, so that > everyone reading the code knows where the code comes from? > (It's generally useful in case there are bugs and borrowing more code from > include-fixer fixes that) Actually, I'd say we are borrowing ideas of filtering instead of the actual code from include-fixer, and the code behavior is also quite different, so it's probably not worth adding pointers to another project in the code. (But as a reference for us, the include-fixer code is .../extra/include-fixer/find-all-symbols/FindAllSymbols.cpp:119 :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41823: [clangd] Add more filters for collected symbols.
This revision was automatically updated to reflect the committed changes. Closed by commit rL322193: [clangd] Add more filters for collected symbols. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D41823 Files: clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp clang-tools-extra/trunk/clangd/index/FileIndex.cpp clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp clang-tools-extra/trunk/clangd/index/SymbolCollector.h clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Index: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp === --- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp @@ -166,15 +166,16 @@ EXPECT_THAT(match(M, FuzzyFindRequest()), UnorderedElementsAre()); } -TEST(FileIndexTest, ClassMembers) { +TEST(FileIndexTest, IgnoreClassMembers) { FileIndex M; auto Ctx = Context::empty(); M.update(Ctx, "f1", - build("f1", "class X { static int m1; int m2;};").getPointer()); + build("f1", "class X { static int m1; int m2; static void f(); };") + .getPointer()); FuzzyFindRequest Req; Req.Query = ""; - EXPECT_THAT(match(M, Req), UnorderedElementsAre("X", "X::m1", "X::m2")); + EXPECT_THAT(match(M, Req), UnorderedElementsAre("X")); } } // namespace Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp === --- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp @@ -53,20 +53,22 @@ namespace { class SymbolIndexActionFactory : public tooling::FrontendActionFactory { public: - SymbolIndexActionFactory() = default; + SymbolIndexActionFactory(SymbolCollector::Options COpts) + : COpts(std::move(COpts)) {} clang::FrontendAction *create() override { index::IndexingOptions IndexOpts; IndexOpts.SystemSymbolFilter = index::IndexingOptions::SystemSymbolFilterKind::All; IndexOpts.IndexFunctionLocals = false; -Collector = std::make_shared(); +Collector = std::make_shared(COpts); FrontendAction *Action = index::createIndexingAction(Collector, IndexOpts, nullptr).release(); return Action; } std::shared_ptr Collector; + SymbolCollector::Options COpts; }; class SymbolCollectorTest : public ::testing::Test { @@ -79,7 +81,7 @@ const std::string FileName = "symbol.cc"; const std::string HeaderName = "symbols.h"; -auto Factory = llvm::make_unique(); +auto Factory = llvm::make_unique(CollectorOpts); tooling::ToolInvocation Invocation( {"symbol_collector", "-fsyntax-only", "-std=c++11", FileName}, @@ -100,9 +102,11 @@ protected: SymbolSlab Symbols; + SymbolCollector::Options CollectorOpts; }; -TEST_F(SymbolCollectorTest, CollectSymbol) { +TEST_F(SymbolCollectorTest, CollectSymbols) { + CollectorOpts.IndexMainFiles = true; const std::string Header = R"( class Foo { void f(); @@ -144,7 +148,6 @@ EXPECT_THAT(Symbols, UnorderedElementsAreArray( {QName("Foo"), - QName("Foo::f"), QName("f1"), QName("f2"), QName("KInt"), @@ -158,29 +161,85 @@ QName("foo::baz")})); } -TEST_F(SymbolCollectorTest, SymbolWithDocumentation) { +TEST_F(SymbolCollectorTest, IgnoreSymbolsInMainFile) { + CollectorOpts.IndexMainFiles = false; + const std::string Header = R"( +class Foo {}; +void f1(); +inline void f2() {} + )"; + const std::string Main = R"( +namespace { +void ff() {} // ignore +} +void main_f() {} // ignore +void f1() {} + )"; + runSymbolCollector(Header, Main); + EXPECT_THAT(Symbols, + UnorderedElementsAre(QName("Foo"), QName("f1"), QName("f2"))); +} + +TEST_F(SymbolCollectorTest, IncludeSymbolsInMainFile) { + CollectorOpts.IndexMainFiles = true; + const std::string Header = R"( +class Foo {}; +void f1(); +inline void f2() {} + )"; + const std::string Main = R"( +namespace { +void ff() {} // ignore +} +void main_f() {} +void f1() {} + )"; + runSymbolCollector(Header, Main); + EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("f1"), +QName("f2"), QName("main_f"))); +} + +TEST_F(SymbolCollectorTest, IgnoreClassMembers) { + const std::string Header = R"( +class Foo { + void f() {} + void g(); + static void sf() {} + static void ssf(); + static int x; +}; + )"; const std::string Main = R"( +void Foo::g() {} +void Foo::ss
[clang-tools-extra] r322193 - [clangd] Add more filters for collected symbols.
Author: ioeric Date: Wed Jan 10 06:57:58 2018 New Revision: 322193 URL: http://llvm.org/viewvc/llvm-project?rev=322193&view=rev Log: [clangd] Add more filters for collected symbols. Summary: o We only collect symbols in namespace or translation unit scopes. o Add an option to only collect symbols in included headers. Reviewers: hokein, ilya-biryukov Reviewed By: hokein, ilya-biryukov Subscribers: klimek, cfe-commits Differential Revision: https://reviews.llvm.org/D41823 Modified: clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp clang-tools-extra/trunk/clangd/index/FileIndex.cpp clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp clang-tools-extra/trunk/clangd/index/SymbolCollector.h clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Modified: clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?rev=322193&r1=322192&r2=322193&view=diff == --- clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp (original) +++ clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp Wed Jan 10 06:57:58 2018 @@ -72,8 +72,9 @@ public: IndexOpts.SystemSymbolFilter = index::IndexingOptions::SystemSymbolFilterKind::All; IndexOpts.IndexFunctionLocals = false; -return new WrappedIndexAction(std::make_shared(), - IndexOpts, Ctx); +return new WrappedIndexAction( +std::make_shared(SymbolCollector::Options()), +IndexOpts, Ctx); } tooling::ExecutionContext *Ctx; Modified: clang-tools-extra/trunk/clangd/index/FileIndex.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.cpp?rev=322193&r1=322192&r2=322193&view=diff == --- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original) +++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Wed Jan 10 06:57:58 2018 @@ -19,7 +19,9 @@ namespace { std::unique_ptr indexAST(ASTContext &Ctx, std::shared_ptr PP, llvm::ArrayRef Decls) { - auto Collector = std::make_shared(); + SymbolCollector::Options CollectorOpts; + CollectorOpts.IndexMainFiles = true; + auto Collector = std::make_shared(std::move(CollectorOpts)); Collector->setPreprocessor(std::move(PP)); index::IndexingOptions IndexOpts; IndexOpts.SystemSymbolFilter = Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=322193&r1=322192&r2=322193&view=diff == --- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original) +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Wed Jan 10 06:57:58 2018 @@ -10,6 +10,7 @@ #include "SymbolCollector.h" #include "../CodeCompletionStrings.h" #include "clang/AST/DeclCXX.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Basic/SourceManager.h" #include "clang/Index/IndexSymbol.h" #include "clang/Index/USRGeneration.h" @@ -65,8 +66,39 @@ splitQualifiedName(llvm::StringRef QName return {QName.substr(0, Pos), QName.substr(Pos + 2)}; } +bool shouldFilterDecl(const NamedDecl *ND, ASTContext *ASTCtx, + const SymbolCollector::Options &Opts) { + using namespace clang::ast_matchers; + if (ND->isImplicit()) +return true; + // FIXME: figure out a way to handle internal linkage symbols (e.g. static + // variables, function) defined in the .cc files. Also we skip the symbols + // in anonymous namespace as the qualifier names of these symbols are like + // `foobar`, which need a special handling. + // In real world projects, we have a relatively large set of header files + // that define static variables (like "static const int A = 1;"), we still + // want to collect these symbols, although they cause potential ODR + // violations. + if (ND->isInAnonymousNamespace()) +return true; + + // We only want symbols in namespaces or translation unit scopes (e.g. no + // class members). + if (match(decl(allOf( +Opts.IndexMainFiles ? decl() +: decl(unless(isExpansionInMainFile())), +hasDeclContext(anyOf(namespaceDecl(), translationUnitDecl(), +*ND, *ASTCtx) + .empty()) +return true; + + return false; +} + } // namespace +SymbolCollector::SymbolCollector(Options Opts) : Opts(std::move(Opts)) {} + void SymbolCollector::initialize(ASTContext &Ctx)
[clang-tools-extra] r322194 - Add a missing dependency for r322192
Author: ioeric Date: Wed Jan 10 07:11:26 2018 New Revision: 322194 URL: http://llvm.org/viewvc/llvm-project?rev=322194&view=rev Log: Add a missing dependency for r322192 Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=322194&r1=322193&r2=322194&view=diff == --- clang-tools-extra/trunk/clangd/CMakeLists.txt (original) +++ clang-tools-extra/trunk/clangd/CMakeLists.txt Wed Jan 10 07:11:26 2018 @@ -30,6 +30,7 @@ add_clang_library(clangDaemon LINK_LIBS clangAST + clangASTMatchers clangBasic clangFormat clangFrontend ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons
tbourvon updated this revision to Diff 129278. tbourvon added a comment. Add retrieval of MaximumLineLength from clang-format options, otherwise defaults to 80. Also add unit tests around the limit case for the line length. https://reviews.llvm.org/D37014 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp clang-tidy/readability/UnnecessaryIntermediateVarCheck.cpp clang-tidy/readability/UnnecessaryIntermediateVarCheck.h clang-tidy/utils/LexerUtils.cpp clang-tidy/utils/LexerUtils.h clang-tidy/utils/Matchers.h docs/ReleaseNotes.rst docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/readability-unnecessary-intermediate-var.rst test/clang-tidy/readability-unnecessary-intermediate-var.cpp Index: test/clang-tidy/readability-unnecessary-intermediate-var.cpp === --- /dev/null +++ test/clang-tidy/readability-unnecessary-intermediate-var.cpp @@ -0,0 +1,202 @@ +// RUN: %check_clang_tidy %s readability-unnecessary-intermediate-var %t + +bool f() { + auto test = 1; // Test + // CHECK-FIXES: {{^}} // Test{{$}} + return (test == 1); + // CHECK-FIXES: {{^}} return (1 == 1);{{$}} + // CHECK-MESSAGES: :[[@LINE-4]]:8: warning: unnecessary intermediate variable 'test' [readability-unnecessary-intermediate-var] + // CHECK-MESSAGES: :[[@LINE-3]]:11: note: because it is only used when returning the result of this comparison + // CHECK-MESSAGES: :[[@LINE-6]]:8: note: consider removing the variable declaration + // CHECK-MESSAGES: :[[@LINE-5]]:11: note: and directly using the variable initialization expression here +} + +bool f2() { + auto test1 = 1; // Test1 + // CHECK-FIXES: {{^}} // Test1{{$}} + auto test2 = 2; // Test2 + // CHECK-FIXES: {{^}} // Test2{{$}} + return (test1 == test2); + // CHECK-FIXES: {{^}} return (1 == 2);{{$}} + // CHECK-MESSAGES: :[[@LINE-6]]:8: warning: unnecessary intermediate variable 'test1' [readability-unnecessary-intermediate-var] + // CHECK-MESSAGES: :[[@LINE-5]]:8: warning: and so is 'test2' [readability-unnecessary-intermediate-var] + // CHECK-MESSAGES: :[[@LINE-4]]:11: note: because they are only used when returning the result of this comparison + // CHECK-MESSAGES: :[[@LINE-9]]:8: note: consider removing both this variable declaration + // CHECK-MESSAGES: :[[@LINE-8]]:8: note: and this one + // CHECK-MESSAGES: :[[@LINE-7]]:11: note: and directly using the variable initialization expressions here +} + +bool f3() { + auto test1 = 1; // Test1 + // CHECK-FIXES: {{^}} // Test1{{$}} + auto test2 = 2; // Test2 + return (test1 == 2); + // CHECK-FIXES: {{^}} return (1 == 2);{{$}} + // CHECK-MESSAGES: :[[@LINE-5]]:8: warning: unnecessary intermediate variable 'test1' [readability-unnecessary-intermediate-var] + // CHECK-MESSAGES: :[[@LINE-3]]:11: note: because it is only used when returning the result of this comparison + // CHECK-MESSAGES: :[[@LINE-7]]:8: note: consider removing the variable declaration + // CHECK-MESSAGES: :[[@LINE-5]]:11: note: and directly using the variable initialization expression here +} + +bool f4() { + auto test1 = 1; // Test1 + auto test2 = 2; // Test2 + // CHECK-FIXES: {{^}} // Test2{{$}} + return (test2 == 3); + // CHECK-FIXES: {{^}} return (2 == 3);{{$}} + // CHECK-MESSAGES: :[[@LINE-4]]:8: warning: unnecessary intermediate variable 'test2' [readability-unnecessary-intermediate-var] + // CHECK-MESSAGES: :[[@LINE-3]]:11: note: because it is only used when returning the result of this comparison + // CHECK-MESSAGES: :[[@LINE-6]]:8: note: consider removing the variable declaration + // CHECK-MESSAGES: :[[@LINE-5]]:11: note: and directly using the variable initialization expression here +} + +bool f5() { + auto test1 = 1; // Test1 + // CHECK-FIXES: {{^}} // Test1{{$}} + auto test2 = 2; // Test2 + return (2 == test1); + // CHECK-FIXES: {{^}} return (1 == 2);{{$}} + // CHECK-MESSAGES: :[[@LINE-5]]:8: warning: unnecessary intermediate variable 'test1' [readability-unnecessary-intermediate-var] + // CHECK-MESSAGES: :[[@LINE-3]]:16: note: because it is only used when returning the result of this comparison + // CHECK-MESSAGES: :[[@LINE-7]]:8: note: consider removing the variable declaration + // CHECK-MESSAGES: :[[@LINE-5]]:11: note: and directly using the variable initialization expression here +} + +bool f6() { + auto test1 = 1; // Test1 + auto test2 = 2; // Test2 + // CHECK-FIXES: {{^}} // Test2{{$}} + return (3 == test2); + // CHECK-FIXES: {{^}} return (2 == 3);{{$}} + // CHECK-MESSAGES: :[[@LINE-4]]:8: warning: unnecessary intermediate variable 'test2' [readability-unnecessary-intermediate-var] + // CHECK-MESSAGES: :[[@LINE-3]]:16: note: because it is only used when returning the result of this comparison + // CHECK-MESSAGES: :[[@LINE-6]]:8: note: consider removing the variable declaration + // CHECK-MESSAGES: :[[@LINE-5]]:11: note: and directly using the variable initializati
[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons
tbourvon marked 4 inline comments as done. tbourvon added a comment. (By the way, credits go to @Abpostelnicu for the latest changes regarding MaximumLineLength interop with clang-format options) Comment at: clang-tidy/readability/UnnecessaryIntermediateVarCheck.cpp:376 +// expression wouldn't really benefit readability. Therefore we abort. +if (NewReturnLength > MaximumLineLength) { + return; Abpostelnicu wrote: > lebedev.ri wrote: > > Is there really no way to format the fixes, and *then* check the line > > length? > > ``` > > $ clang-tidy --help > > ... > > -format-style= - > > Style for formatting code around applied > > fixes: > >- 'none' (default) turns off formatting > >- 'file' (literally 'file', not a > > placeholder) > > uses .clang-format file in the closest > > parent > > directory > >- '{ }' specifies options inline, > > e.g. > > -format-style='{BasedOnStyle: llvm, > > IndentWidth: 8}' > >- 'llvm', 'google', 'webkit', 'mozilla' > > See clang-format documentation for the > > up-to-date > > information about formatting styles and > > options. > > This option overrides the 'FormatStyle` > > option in > > .clang-tidy file, if any. > > ... > > ``` > > so `clang-tidy` is at least aware of `clang-format`. > I think this is doable since I see this in the code: > > https://code.woboq.org/llvm/clang-tools-extra/clang-tidy/ClangTidy.cpp.html#199 > > That leads me to think that we can have this before applying the fixes and in > case the fix after re-format has a line that violates our rule it gets > dropped. I'm gonna update the patch with this new addon. Regarding this comment, Andi (@Abpostelnicu) and I have analyzed the situation and we believe that formatting the replacement before checking the line length achieves almost nothing, yet complicates the code a lot. If we format the replacement before applying the fix, then we're //almost// sure that clang-format will split the replacement into multiple lines and that it will never go above the maximum line length (except for extremely rare cases like 80+ characters identifiers). Actually, we believe that splitting the return statement into multiple lines hinders its readability, and therefore that in cases where the fix would exceed the maximum line length, we're better off not applying it, since the main goal of this check **is** readability. clang-format can still be run after the fix is applying, of course. https://reviews.llvm.org/D37014 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41823: [clangd] Add more filters for collected symbols.
ilya-biryukov added inline comments. Comment at: clangd/index/SymbolCollector.cpp:80 +: decl(unless(isExpansionInMainFile())), +hasDeclContext(anyOf(namespaceDecl(), translationUnitDecl(), +*D, *ASTCtx) ioeric wrote: > ilya-biryukov wrote: > > ioeric wrote: > > > ilya-biryukov wrote: > > > > hokein wrote: > > > > > These ast matchers seems to be relatively simple, maybe we can use > > > > > the `Decl` methods to implement them at the moment. Or will we add > > > > > more complex filters in the future? > > > > I second this. Also the code would probably be more readable without > > > > matchers in this particular example. > > > Yes, I expect to borrow more matchers from include-fixer's > > > find-all-symbols. Also, although `isExpansionInMainFile` seems easy to > > > implement, I am inclined to reuse the existing code if possible. > > Could you add a comment pointing to the code in include fixer, so that > > everyone reading the code knows where the code comes from? > > (It's generally useful in case there are bugs and borrowing more code from > > include-fixer fixes that) > Actually, I'd say we are borrowing ideas of filtering instead of the actual > code from include-fixer, and the code behavior is also quite different, so > it's probably not worth adding pointers to another project in the code. > > (But as a reference for us, the include-fixer code is > .../extra/include-fixer/find-all-symbols/FindAllSymbols.cpp:119 :) I thought we were borrowing code after reading this comment: > I expect to borrow more matchers from include-fixer's find-all-symbols. Thanks for the reference, I'd say it's still useful to have a reference in the code: ```// The code is similar to include fixer, see include-fixer/find-all-symbols/FindAllSymbols.cpp for more details.``` But it's up to you, of course. Repository: rL LLVM https://reviews.llvm.org/D41823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r322196 - Fix misspelled macro name - thanks to and...@ispras.ru for the catch
Author: marshall Date: Wed Jan 10 08:25:04 2018 New Revision: 322196 URL: http://llvm.org/viewvc/llvm-project?rev=322196&view=rev Log: Fix misspelled macro name - thanks to and...@ispras.ru for the catch Modified: libcxx/trunk/test/std/algorithms/alg.nonmodifying/alg.search/search.pass.cpp Modified: libcxx/trunk/test/std/algorithms/alg.nonmodifying/alg.search/search.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/algorithms/alg.nonmodifying/alg.search/search.pass.cpp?rev=322196&r1=322195&r2=322196&view=diff == --- libcxx/trunk/test/std/algorithms/alg.nonmodifying/alg.search/search.pass.cpp (original) +++ libcxx/trunk/test/std/algorithms/alg.nonmodifying/alg.search/search.pass.cpp Wed Jan 10 08:25:04 2018 @@ -86,7 +86,7 @@ int main() test, bidirectional_iterator >(); test, random_access_iterator >(); -#if TEST_STD_VERS > 14 +#if TEST_STD_VER > 14 { typedef int * RI; static_assert((std::is_same::value), "" ); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38284: [clang-tidy] Fix google-readability-namespace-comments handling of C++17 nested namespaces
alexfh added a comment. It looks like the latest patch was lost. I'll see whether it still applies cleanly... https://reviews.llvm.org/D38284 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41830: [libc++] Fix PR#35780 - make std::experimental::filesystem::remove and remove_all return false or 0 if the file doesn't exist
TyanNN updated this revision to Diff 129280. TyanNN added a comment. Fix remove and remove_all error resetting. Move test to the appropriate files and fix old tests. https://reviews.llvm.org/D41830 Files: src/experimental/filesystem/operations.cpp test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp Index: test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp === --- test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp +++ test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp @@ -65,14 +65,18 @@ const path testCases[] = { env.make_env_path("dne"), -file_in_bad_dir +//file_in_bad_dir }; const auto BadRet = static_cast(-1); for (auto& p : testCases) { std::error_code ec; -TEST_CHECK(fs::remove_all(p, ec) == BadRet); -TEST_CHECK(ec); -TEST_CHECK(checkThrow(p, ec)); +if (exists(p)) { +TEST_CHECK(fs::remove_all(p, ec) == BadRet); +TEST_CHECK(ec); +TEST_CHECK(checkThrow(p, ec)); +} else { +TEST_CHECK(fs::remove_all(p, ec) == 0); +} } } Index: test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp === --- test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp +++ test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp @@ -64,13 +64,16 @@ "", env.make_env_path("dne"), non_empty_dir, -file_in_bad_dir, +// file_in_bad_dir, // produces "St13exception_ptruncaught_exceptions not yet implemented" }; for (auto& p : testCases) { std::error_code ec; + TEST_CHECK(!fs::remove(p, ec)); -TEST_CHECK(ec); -TEST_CHECK(checkThrow(p, ec)); +if (exists(p)) { +TEST_CHECK(ec); +TEST_CHECK(checkThrow(p, ec)); +} } } Index: src/experimental/filesystem/operations.cpp === --- src/experimental/filesystem/operations.cpp +++ src/experimental/filesystem/operations.cpp @@ -661,8 +661,10 @@ bool __remove(const path& p, std::error_code *ec) { if (ec) ec->clear(); + if (::remove(p.c_str()) == -1) { -set_or_throw(ec, "remove", p); +if (errno != ENOENT) +set_or_throw(ec, "remove", p); return false; } return true; @@ -692,13 +694,18 @@ } // end namespace std::uintmax_t __remove_all(const path& p, std::error_code *ec) { +if (ec) ec->clear(); + std::error_code mec; auto count = remove_all_impl(p, mec); if (mec) { -set_or_throw(mec, ec, "remove_all", p); -return static_cast(-1); +if (mec == errc::no_such_file_or_directory) { +return 0; +} else { +set_or_throw(mec, ec, "remove_all", p); +return static_cast(-1); +} } -if (ec) ec->clear(); return count; } Index: test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp === --- test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp +++ test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp @@ -65,14 +65,18 @@ const path testCases[] = { env.make_env_path("dne"), -file_in_bad_dir +//file_in_bad_dir }; const auto BadRet = static_cast(-1); for (auto& p : testCases) { std::error_code ec; -TEST_CHECK(fs::remove_all(p, ec) == BadRet); -TEST_CHECK(ec); -TEST_CHECK(checkThrow(p, ec)); +if (exists(p)) { +TEST_CHECK(fs::remove_all(p, ec) == BadRet); +TEST_CHECK(ec); +TEST_CHECK(checkThrow(p, ec)); +} else { +TEST_CHECK(fs::remove_all(p, ec) == 0); +} } } Index: test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp === --- test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp +++ test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp @@ -64,13 +64,16 @@ "", env.make_env_path("dne"), non_empty_dir, -file_in_bad_dir, +// file_in_bad_dir, // produces "St13exception_ptruncaught_exceptions not yet implemented" }; for (auto& p : testCases) { std::error_code ec; + TEST_CHECK(!fs::remove(p, ec)); -TEST_CHECK(ec); -TEST_CHECK(checkThrow(p, ec)); +if (exists(p)) { +TEST_CHECK(ec); +TES
[PATCH] D41830: [libc++] Fix PR#35780 - make std::experimental::filesystem::remove and remove_all return false or 0 if the file doesn't exist
TyanNN added inline comments. Comment at: test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp:67 non_empty_dir, -file_in_bad_dir, +// file_in_bad_dir, // produces "St13exception_ptruncaught_exceptions not yet implemented" }; This error is quite strange: it is not inherited from std::exception, one can only really find our the error with std::current_exception, and it happens when you use the path in fs::remove and fs::remove_all. Any idea about this? @mclow.lists https://reviews.llvm.org/D41830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41907: [clangd] Pass Context to onDiagnosticsReady callback
ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added a subscriber: klimek. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41907 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h unittests/clangd/ClangdTests.cpp unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -59,8 +59,10 @@ using ::testing::Not; class IgnoreDiagnostics : public DiagnosticsConsumer { - void onDiagnosticsReady( - PathRef File, Tagged> Diagnostics) override {} + void + onDiagnosticsReady(const Context &Ctx, PathRef File, + Tagged> Diagnostics) override { + } }; // GMock helpers for matching completion items. Index: unittests/clangd/ClangdTests.cpp === --- unittests/clangd/ClangdTests.cpp +++ unittests/clangd/ClangdTests.cpp @@ -49,7 +49,7 @@ class ErrorCheckingDiagConsumer : public DiagnosticsConsumer { public: void - onDiagnosticsReady(PathRef File, + onDiagnosticsReady(const Context &Ctx, PathRef File, Tagged> Diagnostics) override { bool HadError = diagsContainErrors(Diagnostics.Value); @@ -470,7 +470,7 @@ TestDiagConsumer() : Stats(FilesCount, FileStat()) {} void onDiagnosticsReady( -PathRef File, +const Context &Ctx, PathRef File, Tagged> Diagnostics) override { StringRef FileIndexStr = llvm::sys::path::stem(File); ASSERT_TRUE(FileIndexStr.consume_front("Foo")); @@ -758,7 +758,7 @@ : StartSecondReparse(std::move(StartSecondReparse)) {} void onDiagnosticsReady( -PathRef File, +const Context &Ctx, PathRef File, Tagged> Diagnostics) override { std::unique_lock Lock(Mutex, std::try_to_lock_t()); Index: clangd/ClangdServer.h === --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -72,7 +72,7 @@ /// Called by ClangdServer when \p Diagnostics for \p File are ready. virtual void - onDiagnosticsReady(PathRef File, + onDiagnosticsReady(const Context &Ctx, PathRef File, Tagged> Diagnostics) = 0; }; Index: clangd/ClangdServer.cpp === --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -583,7 +583,7 @@ return; LastReportedDiagsVersion = Version; -DiagConsumer.onDiagnosticsReady(FileStr, +DiagConsumer.onDiagnosticsReady(Ctx, FileStr, make_tagged(std::move(*Diags), Tag)); }; Index: clangd/ClangdLSPServer.h === --- clangd/ClangdLSPServer.h +++ clangd/ClangdLSPServer.h @@ -48,7 +48,7 @@ private: // Implement DiagnosticsConsumer. virtual void - onDiagnosticsReady(PathRef File, + onDiagnosticsReady(const Context &Ctx, PathRef File, Tagged> Diagnostics) override; // Implement ProtocolCallbacks. Index: clangd/ClangdLSPServer.cpp === --- clangd/ClangdLSPServer.cpp +++ clangd/ClangdLSPServer.cpp @@ -323,7 +323,8 @@ } void ClangdLSPServer::onDiagnosticsReady( -PathRef File, Tagged> Diagnostics) { +const Context &Ctx, PathRef File, +Tagged> Diagnostics) { json::ary DiagnosticsJSON; DiagnosticToReplacementMap LocalFixIts; // Temporary storage ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38639: [clangd] #include statements support for Open definition
malaperle added inline comments. Comment at: clangd/ClangdUnit.cpp:113 + CppFilePreambleCallbacks(IncludeReferenceMap &IRM) + : IRM(IRM) {} ilya-biryukov wrote: > malaperle wrote: > > ilya-biryukov wrote: > > > ilya-biryukov wrote: > > > > Let's create a new empty map inside this class and have a > > > > `takeIncludeReferences` method (similar to `TopLevelDeclIDs` and > > > > `takeTopLevelDeclIDs`) > > > This comment is not addressed yet, but marked as done. > > As mentioned below, the idea was to have a single map being appended to, > > without having to merge two separate maps. However, I can change the code > > so that two separate maps are built and merged if you prefer. > > > > But I'm not so clear if that's what you'd prefer: > > > > > You copy the map for preamble and then append to it in > > > CppFilePreambleCallbacks? That also LG, we should not have many > > > references there anyway. > > > > It's not meant to have any copy. The idea was to create a single > > IncludeReferenceMap in CppFile::deferRebuild, populate it with both > > preamble and non-preamble include references and std::move it around for > > later use (stored in ParsedAST). > We can't have a single map because AST is rebuilt more often than the > Preamble, so we have two options: > - Store a map for the preamble separately, copy it when we need to rebuild > the AST and append references from the AST to the new instance of the map. > - Store two maps: one contains references only from the Preamble, the other > one from the AST. > > I think both are fine, since the copy of the map will be cheap anyway, as we > only store a list of includes inside the main file. > We can't have a single map because AST is rebuilt more often than the > Preamble, so we have two options: Doh! Indeed. OK so I added the map in PreambleData, this one will say every reparse unless the preamble changes. When the AST is built/rebuilt, the map is copied (or empty) and includes from the AST are appended to it then stored in ParsedAST (option #1?). Comment at: clangd/XRefs.cpp:185 + + if ((unsigned)R.start.line == + SourceMgr.getSpellingLineNumber( ilya-biryukov wrote: > malaperle wrote: > > ilya-biryukov wrote: > > > why do we need to convert to unsigned? To slience compiler warnings? > > Yes, "line" from the protocol is signed, whereas > > getSpellingColumn/lineNumber returns unsigned. I'll extract another var for > > the line number and cast both to int instead to have less casts and make > > the condition smaller. > Can we maybe convert to `clangd::Position` using the helper methods first and > do the comparison of two `clangd::Position`s? > Comparing between `clangd::Position` and clang's line/column numbers is a > common source of off-by-one errors in clangd. offsetToPosition (if that's what you mean) uses a StringRef of the code, which is not handy in this case. I'll add another one "sourceLocToPosition" to convert a SourceLocation to a Position. WDYT? It can be used in a few other places too. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D38639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38639: [clangd] #include statements support for Open definition
malaperle updated this revision to Diff 129292. malaperle added a comment. Store map in PremableData and copy it on reparse. Convert SourceLoc to Position when comparing with include Positions. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D38639 Files: clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/Protocol.h clangd/SourceCode.cpp clangd/SourceCode.h clangd/XRefs.cpp unittests/clangd/ClangdTests.cpp Index: unittests/clangd/ClangdTests.cpp === --- unittests/clangd/ClangdTests.cpp +++ unittests/clangd/ClangdTests.cpp @@ -751,6 +751,74 @@ EXPECT_FALSE(PathResult.hasValue()); } +TEST_F(ClangdVFSTest, CheckDefinitionIncludes) { + MockFSProvider FS; + ErrorCheckingDiagConsumer DiagConsumer; + MockCompilationDatabase CDB; + + ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), + /*StorePreamblesInMemory=*/true); + + auto FooCpp = getVirtualTestFilePath("foo.cpp"); + const auto SourceContents = R"cpp( + #include "foo.h" + #include "invalid.h" + int b = a; + // test + int foo; + #include "foo.h" + )cpp"; + FS.Files[FooCpp] = SourceContents; + auto FooH = getVirtualTestFilePath("foo.h"); + const auto HeaderContents = "int a;"; + + FS.Files[FooCpp] = SourceContents; + FS.Files[FooH] = HeaderContents; + + Server.addDocument(Context::empty(), FooH, HeaderContents); + Server.addDocument(Context::empty(), FooCpp, SourceContents); + + Position P = Position{1, 11}; + + auto ExpectedLocations = Server.findDefinitions(Context::empty(), FooCpp, P); + ASSERT_TRUE(!!ExpectedLocations); + std::vector Locations = ExpectedLocations->Value; + EXPECT_TRUE(!Locations.empty()); + std::string s("file:///"); + std::string check = Locations[0].uri.uri; + check = check.erase(0, s.size() - 1); + check = check.substr(0, check.size()); + ASSERT_EQ(check, FooH); + ASSERT_EQ(Locations[0].range.start.line, 0); + ASSERT_EQ(Locations[0].range.start.character, 0); + ASSERT_EQ(Locations[0].range.end.line, 0); + ASSERT_EQ(Locations[0].range.end.character, 0); + + // Test include in preamble + Position P2 = Position{1, 11}; + + ExpectedLocations = Server.findDefinitions(Context::empty(), FooCpp, P2); + ASSERT_TRUE(!!ExpectedLocations); + Locations = ExpectedLocations->Value; + EXPECT_TRUE(!Locations.empty()); + + // Test invalid include + Position P3 = Position{2, 11}; + + ExpectedLocations = Server.findDefinitions(Context::empty(), FooCpp, P3); + ASSERT_TRUE(!!ExpectedLocations); + Locations = ExpectedLocations->Value; + EXPECT_TRUE(Locations.empty()); + + // Test include outside of Preamble + Position P4 = Position{6, 11}; + + ExpectedLocations = Server.findDefinitions(Context::empty(), FooCpp, P4); + ASSERT_TRUE(!!ExpectedLocations); + Locations = ExpectedLocations->Value; + EXPECT_TRUE(!Locations.empty()); +} + TEST_F(ClangdThreadingTest, NoConcurrentDiagnostics) { class NoConcurrentAccessDiagConsumer : public DiagnosticsConsumer { public: Index: clangd/XRefs.cpp === --- clangd/XRefs.cpp +++ clangd/XRefs.cpp @@ -7,6 +7,7 @@ // //===-===// #include "XRefs.h" +#include "SourceCode.h" #include "clang/Index/IndexDataConsumer.h" #include "clang/Index/IndexingAction.h" namespace clang { @@ -103,12 +104,8 @@ return llvm::None; SourceLocation LocEnd = Lexer::getLocForEndOfToken(ValSourceRange.getEnd(), 0, SourceMgr, LangOpts); - Position Begin; - Begin.line = SourceMgr.getSpellingLineNumber(LocStart) - 1; - Begin.character = SourceMgr.getSpellingColumnNumber(LocStart) - 1; - Position End; - End.line = SourceMgr.getSpellingLineNumber(LocEnd) - 1; - End.character = SourceMgr.getSpellingColumnNumber(LocEnd) - 1; + Position Begin = sourceLocToPosition(SourceMgr, LocStart); + Position End = sourceLocToPosition(SourceMgr, LocEnd); Range R = {Begin, End}; Location L; @@ -142,6 +139,21 @@ indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(), DeclMacrosFinder, IndexOpts); + /// Process targets for paths inside #include directive. + std::vector IncludeTargets; + for (auto &IncludeLoc : AST.getIRM()) { +Range R = IncludeLoc.first; +const SourceManager &SourceMgr = AST.getASTContext().getSourceManager(); +Position Pos = sourceLocToPosition(SourceMgr, SourceLocationBeg); + +if (R.start.line == Pos.line && R.start.character <= Pos.character && +Pos.character <= R.end.character) { + IncludeTargets.push_back(Location{URI::fromFile(IncludeLoc.second), +Range{Position{0, 0}, Position{0, 0}}}); + return IncludeTargets; +} + } + std::vector Decls = DeclMacrosFinder->takeDecls(); std::vector MacroInfos = DeclMacrosFinder->take
[PATCH] D38639: [clangd] #include statements support for Open definition
malaperle updated this revision to Diff 129293. malaperle added a comment. Revert an unintentional white space change. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D38639 Files: clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/Protocol.h clangd/SourceCode.cpp clangd/SourceCode.h clangd/XRefs.cpp unittests/clangd/ClangdTests.cpp Index: unittests/clangd/ClangdTests.cpp === --- unittests/clangd/ClangdTests.cpp +++ unittests/clangd/ClangdTests.cpp @@ -751,6 +751,74 @@ EXPECT_FALSE(PathResult.hasValue()); } +TEST_F(ClangdVFSTest, CheckDefinitionIncludes) { + MockFSProvider FS; + ErrorCheckingDiagConsumer DiagConsumer; + MockCompilationDatabase CDB; + + ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), + /*StorePreamblesInMemory=*/true); + + auto FooCpp = getVirtualTestFilePath("foo.cpp"); + const auto SourceContents = R"cpp( + #include "foo.h" + #include "invalid.h" + int b = a; + // test + int foo; + #include "foo.h" + )cpp"; + FS.Files[FooCpp] = SourceContents; + auto FooH = getVirtualTestFilePath("foo.h"); + const auto HeaderContents = "int a;"; + + FS.Files[FooCpp] = SourceContents; + FS.Files[FooH] = HeaderContents; + + Server.addDocument(Context::empty(), FooH, HeaderContents); + Server.addDocument(Context::empty(), FooCpp, SourceContents); + + Position P = Position{1, 11}; + + auto ExpectedLocations = Server.findDefinitions(Context::empty(), FooCpp, P); + ASSERT_TRUE(!!ExpectedLocations); + std::vector Locations = ExpectedLocations->Value; + EXPECT_TRUE(!Locations.empty()); + std::string s("file:///"); + std::string check = Locations[0].uri.uri; + check = check.erase(0, s.size() - 1); + check = check.substr(0, check.size()); + ASSERT_EQ(check, FooH); + ASSERT_EQ(Locations[0].range.start.line, 0); + ASSERT_EQ(Locations[0].range.start.character, 0); + ASSERT_EQ(Locations[0].range.end.line, 0); + ASSERT_EQ(Locations[0].range.end.character, 0); + + // Test include in preamble + Position P2 = Position{1, 11}; + + ExpectedLocations = Server.findDefinitions(Context::empty(), FooCpp, P2); + ASSERT_TRUE(!!ExpectedLocations); + Locations = ExpectedLocations->Value; + EXPECT_TRUE(!Locations.empty()); + + // Test invalid include + Position P3 = Position{2, 11}; + + ExpectedLocations = Server.findDefinitions(Context::empty(), FooCpp, P3); + ASSERT_TRUE(!!ExpectedLocations); + Locations = ExpectedLocations->Value; + EXPECT_TRUE(Locations.empty()); + + // Test include outside of Preamble + Position P4 = Position{6, 11}; + + ExpectedLocations = Server.findDefinitions(Context::empty(), FooCpp, P4); + ASSERT_TRUE(!!ExpectedLocations); + Locations = ExpectedLocations->Value; + EXPECT_TRUE(!Locations.empty()); +} + TEST_F(ClangdThreadingTest, NoConcurrentDiagnostics) { class NoConcurrentAccessDiagConsumer : public DiagnosticsConsumer { public: Index: clangd/XRefs.cpp === --- clangd/XRefs.cpp +++ clangd/XRefs.cpp @@ -7,6 +7,7 @@ // //===-===// #include "XRefs.h" +#include "SourceCode.h" #include "clang/Index/IndexDataConsumer.h" #include "clang/Index/IndexingAction.h" namespace clang { @@ -103,12 +104,8 @@ return llvm::None; SourceLocation LocEnd = Lexer::getLocForEndOfToken(ValSourceRange.getEnd(), 0, SourceMgr, LangOpts); - Position Begin; - Begin.line = SourceMgr.getSpellingLineNumber(LocStart) - 1; - Begin.character = SourceMgr.getSpellingColumnNumber(LocStart) - 1; - Position End; - End.line = SourceMgr.getSpellingLineNumber(LocEnd) - 1; - End.character = SourceMgr.getSpellingColumnNumber(LocEnd) - 1; + Position Begin = sourceLocToPosition(SourceMgr, LocStart); + Position End = sourceLocToPosition(SourceMgr, LocEnd); Range R = {Begin, End}; Location L; @@ -142,6 +139,21 @@ indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(), DeclMacrosFinder, IndexOpts); + /// Process targets for paths inside #include directive. + std::vector IncludeTargets; + for (auto &IncludeLoc : AST.getIRM()) { +Range R = IncludeLoc.first; +const SourceManager &SourceMgr = AST.getASTContext().getSourceManager(); +Position Pos = sourceLocToPosition(SourceMgr, SourceLocationBeg); + +if (R.start.line == Pos.line && R.start.character <= Pos.character && +Pos.character <= R.end.character) { + IncludeTargets.push_back(Location{URI::fromFile(IncludeLoc.second), +Range{Position{0, 0}, Position{0, 0}}}); + return IncludeTargets; +} + } + std::vector Decls = DeclMacrosFinder->takeDecls(); std::vector MacroInfos = DeclMacrosFinder->takeMacroInfos(); @@ -217,13 +229,8 @@ DocumentHighlight getDocumentHighligh
[clang-tools-extra] r322199 - [clangd] Pass Context to onDiagnosticsReady callback
Author: ibiryukov Date: Wed Jan 10 09:59:27 2018 New Revision: 322199 URL: http://llvm.org/viewvc/llvm-project?rev=322199&view=rev Log: [clangd] Pass Context to onDiagnosticsReady callback Reviewers: sammccall Reviewed By: sammccall Subscribers: klimek, cfe-commits Differential Revision: https://reviews.llvm.org/D41907 Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp clang-tools-extra/trunk/clangd/ClangdLSPServer.h clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/ClangdServer.h clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=322199&r1=322198&r2=322199&view=diff == --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Jan 10 09:59:27 2018 @@ -325,7 +325,8 @@ std::vector ClangdLSPServer::g } void ClangdLSPServer::onDiagnosticsReady( -PathRef File, Tagged> Diagnostics) { +const Context &Ctx, PathRef File, +Tagged> Diagnostics) { json::ary DiagnosticsJSON; DiagnosticToReplacementMap LocalFixIts; // Temporary storage Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=322199&r1=322198&r2=322199&view=diff == --- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original) +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Wed Jan 10 09:59:27 2018 @@ -50,7 +50,7 @@ public: private: // Implement DiagnosticsConsumer. virtual void - onDiagnosticsReady(PathRef File, + onDiagnosticsReady(const Context &Ctx, PathRef File, Tagged> Diagnostics) override; // Implement ProtocolCallbacks. Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=322199&r1=322198&r2=322199&view=diff == --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Wed Jan 10 09:59:27 2018 @@ -586,7 +586,7 @@ std::future ClangdServer::sched return; LastReportedDiagsVersion = Version; -DiagConsumer.onDiagnosticsReady(FileStr, +DiagConsumer.onDiagnosticsReady(Ctx, FileStr, make_tagged(std::move(*Diags), Tag)); }; Modified: clang-tools-extra/trunk/clangd/ClangdServer.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=322199&r1=322198&r2=322199&view=diff == --- clang-tools-extra/trunk/clangd/ClangdServer.h (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.h Wed Jan 10 09:59:27 2018 @@ -72,7 +72,7 @@ public: /// Called by ClangdServer when \p Diagnostics for \p File are ready. virtual void - onDiagnosticsReady(PathRef File, + onDiagnosticsReady(const Context &Ctx, PathRef File, Tagged> Diagnostics) = 0; }; Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp?rev=322199&r1=322198&r2=322199&view=diff == --- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Wed Jan 10 09:59:27 2018 @@ -49,7 +49,7 @@ static bool diagsContainErrors(ArrayRef< class ErrorCheckingDiagConsumer : public DiagnosticsConsumer { public: void - onDiagnosticsReady(PathRef File, + onDiagnosticsReady(const Context &Ctx, PathRef File, Tagged> Diagnostics) override { bool HadError = diagsContainErrors(Diagnostics.Value); @@ -470,7 +470,7 @@ int d; TestDiagConsumer() : Stats(FilesCount, FileStat()) {} void onDiagnosticsReady( -PathRef File, +const Context &Ctx, PathRef File, Tagged> Diagnostics) override { StringRef FileIndexStr = llvm::sys::path::stem(File); ASSERT_TRUE(FileIndexStr.consume_front("Foo")); @@ -758,7 +758,7 @@ TEST_F(ClangdThreadingTest, NoConcurrent : StartSecondReparse(std::move(StartSecondReparse)) {} void onDiagnosticsReady( -PathRef File, +const Context &Ctx, PathRef File, Tagged> Diagnostics) override { std::unique_lock Lock(Mutex, std::try_to_lock_t()); Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp URL: http://llvm
[PATCH] D41907: [clangd] Pass Context to onDiagnosticsReady callback
This revision was automatically updated to reflect the committed changes. Closed by commit rL322199: [clangd] Pass Context to onDiagnosticsReady callback (authored by ibiryukov, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D41907 Files: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp clang-tools-extra/trunk/clangd/ClangdLSPServer.h clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/ClangdServer.h clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Index: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp === --- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp @@ -49,7 +49,7 @@ class ErrorCheckingDiagConsumer : public DiagnosticsConsumer { public: void - onDiagnosticsReady(PathRef File, + onDiagnosticsReady(const Context &Ctx, PathRef File, Tagged> Diagnostics) override { bool HadError = diagsContainErrors(Diagnostics.Value); @@ -470,7 +470,7 @@ TestDiagConsumer() : Stats(FilesCount, FileStat()) {} void onDiagnosticsReady( -PathRef File, +const Context &Ctx, PathRef File, Tagged> Diagnostics) override { StringRef FileIndexStr = llvm::sys::path::stem(File); ASSERT_TRUE(FileIndexStr.consume_front("Foo")); @@ -758,7 +758,7 @@ : StartSecondReparse(std::move(StartSecondReparse)) {} void onDiagnosticsReady( -PathRef File, +const Context &Ctx, PathRef File, Tagged> Diagnostics) override { std::unique_lock Lock(Mutex, std::try_to_lock_t()); Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp === --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp @@ -59,8 +59,10 @@ using ::testing::Not; class IgnoreDiagnostics : public DiagnosticsConsumer { - void onDiagnosticsReady( - PathRef File, Tagged> Diagnostics) override {} + void + onDiagnosticsReady(const Context &Ctx, PathRef File, + Tagged> Diagnostics) override { + } }; // GMock helpers for matching completion items. Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.h === --- clang-tools-extra/trunk/clangd/ClangdLSPServer.h +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h @@ -50,7 +50,7 @@ private: // Implement DiagnosticsConsumer. virtual void - onDiagnosticsReady(PathRef File, + onDiagnosticsReady(const Context &Ctx, PathRef File, Tagged> Diagnostics) override; // Implement ProtocolCallbacks. Index: clang-tools-extra/trunk/clangd/ClangdServer.h === --- clang-tools-extra/trunk/clangd/ClangdServer.h +++ clang-tools-extra/trunk/clangd/ClangdServer.h @@ -72,7 +72,7 @@ /// Called by ClangdServer when \p Diagnostics for \p File are ready. virtual void - onDiagnosticsReady(PathRef File, + onDiagnosticsReady(const Context &Ctx, PathRef File, Tagged> Diagnostics) = 0; }; Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp === --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp @@ -325,7 +325,8 @@ } void ClangdLSPServer::onDiagnosticsReady( -PathRef File, Tagged> Diagnostics) { +const Context &Ctx, PathRef File, +Tagged> Diagnostics) { json::ary DiagnosticsJSON; DiagnosticToReplacementMap LocalFixIts; // Temporary storage Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp === --- clang-tools-extra/trunk/clangd/ClangdServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp @@ -586,7 +586,7 @@ return; LastReportedDiagsVersion = Version; -DiagConsumer.onDiagnosticsReady(FileStr, +DiagConsumer.onDiagnosticsReady(Ctx, FileStr, make_tagged(std::move(*Diags), Tag)); }; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41284: [Concepts] Associated constraints infrastructure.
saar.raz updated this revision to Diff 129297. saar.raz added a comment. - Fixed wrong if that would cause valid instantiated requires clauses to not be accepted. Repository: rC Clang https://reviews.llvm.org/D41284 Files: include/clang/AST/DeclTemplate.h include/clang/AST/RecursiveASTVisitor.h include/clang/Sema/Sema.h lib/AST/DeclTemplate.cpp lib/Sema/SemaTemplate.cpp lib/Sema/SemaTemplateInstantiateDecl.cpp lib/Serialization/ASTReader.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriter.cpp lib/Serialization/ASTWriterDecl.cpp test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/class-template-decl.cpp test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/func-template-decl.cpp test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/var-template-decl.cpp Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/var-template-decl.cpp === --- /dev/null +++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/var-template-decl.cpp @@ -0,0 +1,25 @@ +// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s + +namespace nodiag { + +struct B { +template requires bool(T()) +static int A; +}; + +template requires bool(U()) +int B::A = int(U()); + +} // end namespace nodiag + +namespace diag { + +struct B { +template requires bool(T()) // expected-note{{previous template declaration is here}} +static int A; +}; + +template requires !bool(U()) // expected-error{{associated constraints differ in template redeclaration}} +int B::A = int(U()); + +} // end namespace diag \ No newline at end of file Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/func-template-decl.cpp === --- test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/func-template-decl.cpp +++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/func-template-decl.cpp @@ -1,65 +1,52 @@ -// RUN: %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s +// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s namespace nodiag { template requires bool(T()) -struct A; +int A(); template requires bool(U()) -struct A; +int A(); } // end namespace nodiag namespace diag { template requires true // expected-note{{previous template declaration is here}} -struct A; -template struct A; // expected-error{{associated constraints differ in template redeclaration}} +int A(); +template int A(); // expected-error{{associated constraints differ in template redeclaration}} -template struct B; // expected-note{{previous template declaration is here}} +template int B(); // expected-note{{previous template declaration is here}} template requires true // expected-error{{associated constraints differ in template redeclaration}} -struct B; +int B(); template requires true // expected-note{{previous template declaration is here}} -struct C; +int C(); template requires !0 // expected-error{{associated constraints differ in template redeclaration}} -struct C; +int C(); } // end namespace diag namespace nodiag { struct AA { template requires someFunc(T()) - struct A; + int A(); }; template requires someFunc(T()) -struct AA::A { }; - -struct AAF { - template requires someFunc(T()) - friend struct AA::A; -}; +int AA::A() { return sizeof(T); } } // end namespace nodiag namespace diag { template struct TA { - template class TT> requires TT::happy // expected-note 2{{previous template declaration is here}} - struct A; - - struct AF; + template class TT> requires TT::happy // expected-note{{previous template declaration is here}} + int A(); }; template -template class TT> struct TA::A { }; // expected-error{{associated constraints differ in template redeclaration}} - -template -struct TA::AF { - template class TT> requires TT::happy // expected-error{{associated constraints differ in template redeclaration}} - friend struct TA::A; -}; +template class TT> int TA::A() { return sizeof(TT); } // expected-error{{associated constraints differ in template redeclaration}} } // end namespace diag Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/class-template-decl.cpp === --- test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/class-template-decl.cpp +++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/class-template-decl.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s +// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s namespace nodiag { @@ -33,7 +33,7 @@ struct A; }; -template requires someFunc(T()) +template requires someFunc(U()) struct AA::A { }; struct AAF { @@ -47,18 +47,26 @@ template struct TA { - template class TT> requires TT::happy // expected-note 2{{previous template declaration is here}} + template class TT> requires T
[libcxx] r322201 - libcxx: Stop providing a definition of __GLIBC_PREREQ.
Author: pcc Date: Wed Jan 10 10:16:58 2018 New Revision: 322201 URL: http://llvm.org/viewvc/llvm-project?rev=322201&view=rev Log: libcxx: Stop providing a definition of __GLIBC_PREREQ. An application may determine whether the C standard library is glibc by testing whether __GLIBC_PREREQ is defined. This breaks if libc++ provides its own definition. Instead, define our own macro in our namespace with the desired semantics. Differential Revision: https://reviews.llvm.org/D41892 Modified: libcxx/trunk/include/__config Modified: libcxx/trunk/include/__config URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=322201&r1=322200&r2=322201&view=diff == --- libcxx/trunk/include/__config (original) +++ libcxx/trunk/include/__config Wed Jan 10 10:16:58 2018 @@ -184,9 +184,11 @@ // Need to detect which libc we're using if we're on Linux. #if defined(__linux__) #include -#if !defined(__GLIBC_PREREQ) -#define __GLIBC_PREREQ(a, b) 0 -#endif // !defined(__GLIBC_PREREQ) +#if defined(__GLIBC_PREREQ) +#define _LIBCPP_GLIBC_PREREQ(a, b) __GLIBC_PREREQ(a, b) +#else +#define _LIBCPP_GLIBC_PREREQ(a, b) 0 +#endif // defined(__GLIBC_PREREQ) #endif // defined(__linux__) #ifdef __LITTLE_ENDIAN__ @@ -416,10 +418,10 @@ typedef __char32_t char32_t; #define _LIBCPP_HAS_C11_FEATURES #elif defined(__linux__) #if !defined(_LIBCPP_HAS_MUSL_LIBC) -#if __GLIBC_PREREQ(2, 15) || defined(__BIONIC__) +#if _LIBCPP_GLIBC_PREREQ(2, 15) || defined(__BIONIC__) #define _LIBCPP_HAS_QUICK_EXIT #endif -#if __GLIBC_PREREQ(2, 17) +#if _LIBCPP_GLIBC_PREREQ(2, 17) #define _LIBCPP_HAS_C11_FEATURES #endif #else // defined(_LIBCPP_HAS_MUSL_LIBC) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41423: [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral.
dexonsmith requested changes to this revision. dexonsmith added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Lex/Lexer.cpp:2014-2015 +// getAndAdvanceChar. +if (C == '\\') + C = getAndAdvanceChar(CurPtr, Result); + If `CurPtr` is already equal to `BufferEnd`, why is it safe to call `getAndAdvanceChar`? Is `BufferEnd` dereferenceable? Comment at: clang/lib/Lex/Lexer.cpp:2026 + +if (C == 0) { NulCharacter = CurPtr-1; Should this check still be skipped (in an `else if` of the `C == '\\'` check)? Comment at: clang/unittests/Lex/LexerTest.cpp:477 TEST_F(LexerTest, AvoidPastEndOfStringDereference) { - std::vector LexedTokens = Lex(" // \\\n"); - EXPECT_TRUE(LexedTokens.empty()); + EXPECT_TRUE(Lex(" // \\\n").empty()); + // rdar://problem/35572754 To minimize the diff, please separate this change into an NFC commit ahead of time. Comment at: clang/unittests/Lex/LexerTest.cpp:478 + EXPECT_TRUE(Lex(" // \\\n").empty()); + // rdar://problem/35572754 + EXPECT_TRUE(Lex("#include <").empty()); Usually we don't put rdar/bug numbers in the source file. It would make sense in the commit message though. https://reviews.llvm.org/D41423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41569: Summary:Constraint enforcement and diagnostics
saar.raz updated this revision to Diff 129306. saar.raz added a comment. - Better handling of diagnostics in ConceptSpecializationExprs, correctly written to and read from module files. - Modified Error messages according to some user feedback Updating D41569: Summary: = Constraint enforcement and diagnostics Repository: rC Clang https://reviews.llvm.org/D41569 Files: include/clang/AST/ExprCXX.h include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h include/clang/Sema/TemplateDeduction.h lib/AST/ExprCXX.cpp lib/Sema/SemaOverload.cpp lib/Sema/SemaTemplate.cpp lib/Sema/SemaTemplateDeduction.cpp lib/Sema/SemaTemplateInstantiateDecl.cpp lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/function-templates.cpp test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/non-function-templates.cpp test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/partial-specializations.cpp Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/partial-specializations.cpp === --- /dev/null +++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/partial-specializations.cpp @@ -0,0 +1,28 @@ +// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s + +namespace class_templates +{ + template requires sizeof(T) >= 4 // expected-note {{because 'sizeof(char) >= 4' (1 >= 4) evaluated to false}} + struct is_same { static constexpr bool value = false; }; + + template requires sizeof(T*) >= 4 && sizeof(T) >= 4 + struct is_same { static constexpr bool value = true; }; + + static_assert(!is_same::value); + static_assert(!is_same::value); + static_assert(is_same::value); + static_assert(is_same::value); // expected-error {{constraints not satisfied for class template 'is_same' [with T = char, U = char]}} +} + +namespace variable_templates +{ + template requires sizeof(T) >= 4 + constexpr bool is_same_v = false; + + template requires sizeof(T*) >= 4 && sizeof(T) >= 4 + constexpr bool is_same_v = true; + + static_assert(!is_same_v); + static_assert(!is_same_v); + static_assert(is_same_v); +} \ No newline at end of file Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/non-function-templates.cpp === --- /dev/null +++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/non-function-templates.cpp @@ -0,0 +1,79 @@ +// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s + +template requires sizeof(T) >= 2 // expected-note{{because 'sizeof(char) >= 2' (1 >= 2) evaluated to false}} +struct A { + static constexpr int value = sizeof(T); +}; + +static_assert(A::value == 4); +static_assert(A::value == 1); // expected-error{{constraints not satisfied for class template 'A' [with T = char]}} + +template + requires sizeof(T) != sizeof(U) // expected-note{{because 'sizeof(int) != sizeof(char [4])' (4 != 4) evaluated to false}} + && sizeof(T) >= 4 // expected-note{{because 'sizeof(char) >= 4' (1 >= 4) evaluated to false}} +constexpr int SizeDiff = sizeof(T) > sizeof(U) ? sizeof(T) - sizeof(U) : sizeof(U) - sizeof(T); + +static_assert(SizeDiff == 3); +static_assert(SizeDiff == 0); // expected-error{{constraints not satisfied for variable template 'SizeDiff' [with T = int, U = char [4]]}} +static_assert(SizeDiff == 3); // expected-error{{constraints not satisfied for variable template 'SizeDiff' [with T = char, U = int]}} + +template + requires ((sizeof(Ts) == 4) || ...) // expected-note{{because 'sizeof(char) == 4' (1 == 4) evaluated to false}} expected-note{{'sizeof(long long) == 4' (8 == 4) evaluated to false}} expected-note{{'sizeof(int [20]) == 4' (80 == 4) evaluated to false}} +constexpr auto SumSizes = (sizeof(Ts) + ...); + +static_assert(SumSizes == 13); +static_assert(SumSizes == 89); // expected-error{{constraints not satisfied for variable template 'SumSizes' [with Ts = ]}} + +template +concept IsBig = sizeof(T) > 100; // expected-note{{because 'sizeof(int) > 100' (4 > 100) evaluated to false}} + +template + requires IsBig // expected-note{{'int' does not satisfy 'IsBig'}} +using BigPtr = T*; + +static_assert(sizeof(BigPtr)); // expected-error{{constraints not satisfied for alias template 'BigPtr' [with T = int] + +template requires T::value // expected-note{{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}} +struct S { static constexpr bool value = true; }; + +struct S2 { static constexpr bool value = true; }; + +static_assert(S::value); // expected-error{{constraints not satisfied for class template 'S' [with T = int]}} +static_assert(S::value); + +template +struct AA +{ +template requires sizeof(U) == sizeof(T) // expected-note{{because 'sizeof(int [2])
[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement
NoQ added a comment. In https://reviews.llvm.org/D35109#969712, @baloghadamsoftware wrote: > In https://reviews.llvm.org/D35109#969109, @NoQ wrote: > > > I guess it'd be an `-analyzer-config` flag. You can add it to the > > `AnalyzerOptions` object, which has access to these flags and can be > > accessed from `AnalysisManager`. > > > OK, I can do that. BUt how should I call it? The name should be something the > user also understands, not referring to some internal stuff. Any ideas? I guess a lot of these options already refer to internal stuff. Similarly to off-by-default checkers, such option would not be for users to be thinking whether they want it or not, it is a flag to hide an experimental feature from users completely, while still being able to develop it incrementally, until it's ready to be enabled by default. `-analyzer-config aggressive-relational-comparison-simplification=true` sounds good to me. https://reviews.llvm.org/D35109 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41910: [Concepts] Constrained partial specializations and function overloads.
saar.raz created this revision. saar.raz added reviewers: hubert.reinterpretcast, rsmith, nwilson, changyu, faisalv. Herald added a subscriber: cfe-commits. Added support for constraint satisfaction checking and partial ordering of constraints in constrained partial specialization and function template overloads. Depends on https://reviews.llvm.org/D41569. Repository: rC Clang https://reviews.llvm.org/D41910 Files: include/clang/AST/DeclTemplate.h include/clang/Sema/Sema.h lib/AST/DeclTemplate.cpp lib/Sema/SemaTemplate.cpp lib/Sema/SemaTemplateDeduction.cpp lib/Sema/SemaTemplateInstantiateDecl.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriterDecl.cpp test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/func-template-decl.cpp test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/class-template-partial-specializations.cpp test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/function-templates.cpp test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/var-template-partial-specializations.cpp Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/var-template-partial-specializations.cpp === --- /dev/null +++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/var-template-partial-specializations.cpp @@ -0,0 +1,53 @@ +// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s + +template requires sizeof(T) >= 4 +bool a = false; // expected-note{{template is declared here}} + +template requires sizeof(T) >= 4 && sizeof(T) <= 10 +bool a = true; // expected-error{{variable template partial specialization is not more specialized than the primary template}} + +template +concept C1 = sizeof(T) >= 4; + +template requires C1 +bool b = false; + +template requires C1 && sizeof(T) <= 10 +bool b = true; + +template +concept C2 = sizeof(T) > 1 && sizeof(T) <= 8; + +template +bool c = false; + +template requires C1 +bool c = true; + +template +bool d = false; + +template +bool d = true; // expected-error{{variable template partial specialization does not specialize any template argument; to define the primary template, remove the template argument list}} + +template requires C1 +bool e = false; + +template +bool e = true; // expected-error{{variable template partial specialization does not specialize any template argument; to define the primary template, remove the template argument list}} + +template +constexpr int f = 1; + +template requires C1 && C2 +constexpr int f = 2; + +template requires C1 || C2 +constexpr int f = 3; + +static_assert(f == 2); +static_assert(f == 3); +static_assert(f == 1); + + + Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/function-templates.cpp === --- /dev/null +++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/function-templates.cpp @@ -0,0 +1,54 @@ +// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s + +template requires sizeof(T) >= 4 +bool a() { return false; } // expected-note {{candidate function [with T = unsigned int]}} + +template requires sizeof(T) >= 4 && sizeof(T) <= 10 +bool a() { return true; } // expected-note {{candidate function [with T = unsigned int]}} + +bool av = a(); // expected-error {{call to 'a' is ambiguous}} + +template +concept C1 = sizeof(T) >= 4; + +template requires C1 +constexpr bool b() { return false; } + +template requires C1 && sizeof(T) <= 10 +constexpr bool b() { return true; } + +static_assert(b()); +static_assert(!b()); + +template +concept C2 = sizeof(T) > 1 && sizeof(T) <= 8; + +template +bool c() { return false; } + +template requires C1 +bool c() { return true; } + +template requires C1 +constexpr bool d() { return false; } + +template +constexpr bool d() { return true; } + +static_assert(!d()); + +template +constexpr int e() { return 1; } + +template requires C1 && C2 +constexpr int e() { return 2; } + +template requires C1 || C2 +constexpr int e() { return 3; } + +static_assert(e() == 2); +static_assert(e() == 3); +static_assert(e() == 1); + + + Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/class-template-partial-specializations.cpp === --- /dev/null +++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/class-template-partial-specializations.cpp @@ -0,0 +1,53 @@ +// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s + +template requires sizeof(T) >= 4 +class A{}; // expected-note{{template is declared here}} + +template requires sizeof(T) >= 4 && sizeof(T) <= 10 +class A{}; // expected-error{{class template partial specialization is not more specialized than the primary template}} + +template +concept C1 = sizeof(T) >= 4; + +template requires C1 +class B{}; + +template requires C1 && sizeof(T) <= 10 +class B{}; + +template +concept C2 = sizeof(T) > 1 && sizeof(T) <= 8; + +template +class C{}; + +template requires C1 +clas
[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference
NoQ added a comment. In https://reviews.llvm.org/D35110#969782, @baloghadamsoftware wrote: > Strange, but modifying the tests from `m n` to `m - n > 0` does not help. The statement `if (m - n 0)` does not store a > range for `m - n` in the constraint manager. With the other patch which > automatically changes `m n` to `m - n 0` the range is > stored automatically. I guess we can easily assume how a `SymIntExpr` relates to a number by storing a range on the opaque left-hand-side symbol, no matter how complicated it is, but we cannot assume how a symbol relates to a symbol (there's no obvious range to store). That's just how `assumeSym` currently works. https://reviews.llvm.org/D35110 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41911: [clangd] Include scanner that finds compile commands for headers.
sammccall created this revision. sammccall added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, mgorny, klimek. Not enabled because we need a threadsafe way to change VFS working directories. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41911 Files: clangd/CMakeLists.txt clangd/Compiler.cpp clangd/IncludeScanner.cpp clangd/IncludeScanner.h unittests/clangd/CMakeLists.txt unittests/clangd/IncludeScannerTests.cpp Index: unittests/clangd/IncludeScannerTests.cpp === --- /dev/null +++ unittests/clangd/IncludeScannerTests.cpp @@ -0,0 +1,61 @@ +//===-- IncludeScannerTests.cpp --*- C++ -*---===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "IncludeScanner.h" +#include "TestFS.h" +#include "llvm/ADT/StringMap.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "Logger.h" + +namespace clang { +namespace tooling { +void PrintTo(const llvm::Optional &S, std::ostream *OS) { + if (S) +*OS << llvm::join(S->CommandLine, " "); + else +*OS << ""; +} +} // namespace tooling +namespace clangd { +namespace { + +MATCHER_P2(HasCmd, Args, Filename, "") { + return arg && + arg->CommandLine == Args && + arg->Filename == Filename; +} + +testing::Matcher> +Cmd(std::vector Args) { + return HasCmd(Args, Args.back()); +} + +TEST(IncludeScanner, FindsFiles) { + llvm::StringMap Files; + llvm::StringRef One = getVirtualTestFilePath("one.cc"), + Two = getVirtualTestFilePath("dir/two.h"), + Three = getVirtualTestFilePath("dir/three.h"); + Files[One] = R"cpp(#include "dir/two.h")cpp"; + Files[Two] = R"cpp(#include "three.h")cpp"; + Files[Three] = ""; + auto FS = buildTestFS(Files); + + IncludeScanner Scanner; + Scanner.enqueue({ + {".", One, {"clang", "-DX", One}, ""}, + }, FS); + Scanner.wait(); + EXPECT_THAT(Scanner.lookup(Two), Cmd({"clang", "-DX", "-x", "c++", Two})); + EXPECT_THAT(Scanner.lookup(Three), Cmd({"clang", "-DX", "-x", "c++", Three})); +} + +} // namespace +} // namespace clangd +} // namespace clang Index: unittests/clangd/CMakeLists.txt === --- unittests/clangd/CMakeLists.txt +++ unittests/clangd/CMakeLists.txt @@ -16,6 +16,7 @@ ContextTests.cpp FileIndexTests.cpp FuzzyMatchTests.cpp + IncludeScannerTests.cpp IndexTests.cpp JSONExprTests.cpp TestFS.cpp Index: clangd/IncludeScanner.h === --- /dev/null +++ clangd/IncludeScanner.h @@ -0,0 +1,94 @@ +//===--- IncludeScanner.h - Infer compile commands for headers --*- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===-===// +// Typical compilation databases don't list commands for headers. But headers +// can be compiled with the same flags as the files that include them. +// So when we find a database, we scan through the commands it do +// preprocessor to find #included files that command is valid for. +//===-===// +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INCLUDESCANNER_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INCLUDESCANNER_H +#include "Context.h" +#include "Path.h" +#include "llvm/ADT/IntrusiveRefCntPtr.h" +#include "llvm/ADT/StringMap.h" +#include "llvm/ADT/StringSet.h" +#include +#include +#include +#include +#include +#include + +namespace clang { +class PCHContainerOperations; +namespace vfs { +class FileSystem; +} +namespace tooling { +struct CompileCommand; +} // namespace tooling +namespace clangd { + +// IncludeScanner runs a background thread that scans files for which compile +// commands are known, recording the headers for which that command is valid. +// +// It supports lookup by header name. As scanning doesn't block, it's always +// possible we won't have scanned the right compile command yet. +// This means the compilation database should also have heuristics for headers. +// +// This class is threadsafe. +class IncludeScanner { +public: + IncludeScanner(); + + // The destructor may wait for the current file to finish scanning. + ~IncludeScanner(); + + // If we've scanned some file that #includes Header, return the inferred + // compile command. This will have the same flags, but the filename replaced. + llvm::Optional lookup(PathRef Header) const; + + // Adds compile commands to scan. Files we've already scanned will be ignored. + void enqueue(std::vector Cmds, +
[PATCH] D41816: [analyzer] Model and check unrepresentable left shifts
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Looks great. @dcoughlin: would you approve the warning message text? Maybe actually we could print out the exact numbers that cause the bit shift to overflow, since we do have them when we check. https://reviews.llvm.org/D41816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41912: [Driver] Test for correct '--version' suggestion
modocache created this revision. modocache added reviewers: v.g.vassilev, teemperor, ruiu, jroelofs, yamaguchi. The `llvm::OptTable::findNearest` bug fixed in https://reviews.llvm.org/D41873 manifested itself as the following erroneous message when invoking Clang: clang -version clang-6.0: error: unknown argument '-version', did you mean 'version'? Add a test to catch any future regressions to the now correct behavior, which asks "did you mean '--version'?". Test Plan: `check-clang` Repository: rC Clang https://reviews.llvm.org/D41912 Files: test/Driver/unknown-arg.c Index: test/Driver/unknown-arg.c === --- test/Driver/unknown-arg.c +++ test/Driver/unknown-arg.c @@ -1,6 +1,6 @@ // RUN: not %clang %s -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -### 2>&1 | \ // RUN: FileCheck %s -// RUN: not %clang %s -stdlibs=foo -hell -### 2>&1 | \ +// RUN: not %clang %s -stdlibs=foo -hell -version -### 2>&1 | \ // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN // RUN: %clang_cl -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -### -c -- %s 2>&1 | \ // RUN: FileCheck %s --check-prefix=CL @@ -22,6 +22,7 @@ // CHECK: error: unknown argument: '-funknown-to-clang-option' // DID-YOU-MEAN: error: unknown argument '-stdlibs=foo', did you mean '-stdlib=foo'? // DID-YOU-MEAN: error: unknown argument '-hell', did you mean '-help'? +// DID-YOU-MEAN: error: unknown argument '-version', did you mean '--version'? // CL: warning: unknown argument ignored in clang-cl: '-cake-is-lie' // CL: warning: unknown argument ignored in clang-cl: '-%0' // CL: warning: unknown argument ignored in clang-cl: '-%d' Index: test/Driver/unknown-arg.c === --- test/Driver/unknown-arg.c +++ test/Driver/unknown-arg.c @@ -1,6 +1,6 @@ // RUN: not %clang %s -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -### 2>&1 | \ // RUN: FileCheck %s -// RUN: not %clang %s -stdlibs=foo -hell -### 2>&1 | \ +// RUN: not %clang %s -stdlibs=foo -hell -version -### 2>&1 | \ // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN // RUN: %clang_cl -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -### -c -- %s 2>&1 | \ // RUN: FileCheck %s --check-prefix=CL @@ -22,6 +22,7 @@ // CHECK: error: unknown argument: '-funknown-to-clang-option' // DID-YOU-MEAN: error: unknown argument '-stdlibs=foo', did you mean '-stdlib=foo'? // DID-YOU-MEAN: error: unknown argument '-hell', did you mean '-help'? +// DID-YOU-MEAN: error: unknown argument '-version', did you mean '--version'? // CL: warning: unknown argument ignored in clang-cl: '-cake-is-lie' // CL: warning: unknown argument ignored in clang-cl: '-%0' // CL: warning: unknown argument ignored in clang-cl: '-%d' ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41912: [Driver] Test for correct '--version' suggestion
v.g.vassilev accepted this revision. v.g.vassilev added a comment. This revision is now accepted and ready to land. LGMT! We can rely on post-commit reviews for changes like this one. Repository: rC Clang https://reviews.llvm.org/D41912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37860: [MSan] Enable use-after-dtor instrumentation by default.
morehouse updated this revision to Diff 129318. morehouse added a comment. - Enable use-after-dtor instrumentation by default. - Make sanitize-no-dtor-callback.cpp test fail with UAD instrumentation. - Update test cases to reflect new default. https://reviews.llvm.org/D37860 Files: clang/include/clang/Driver/SanitizerArgs.h clang/test/CodeGenCXX/sanitize-no-dtor-callback.cpp clang/test/Driver/fsanitize.c compiler-rt/test/msan/dtor-member.cc compiler-rt/test/msan/use-after-dtor.cc Index: compiler-rt/test/msan/use-after-dtor.cc === --- compiler-rt/test/msan/use-after-dtor.cc +++ compiler-rt/test/msan/use-after-dtor.cc @@ -1,14 +1,17 @@ // RUN: %clangxx_msan %s -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1 -// RUN: FileCheck %s < %t.out +// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out // RUN: %clangxx_msan %s -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1 -// RUN: FileCheck %s < %t.out +// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out // RUN: %clangxx_msan %s -O2 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1 -// RUN: FileCheck %s < %t.out +// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out // RUN: %clangxx_msan %s -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -fsanitize-memory-track-origins -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1 -// RUN: FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out +// RUN: FileCheck %s --check-prefixes=CHECK-UAD,CHECK-ORIGINS < %t.out + +// RUN: %clangxx_msan %s -fno-sanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t > %t.out 2>&1 +// RUN: FileCheck %s --check-prefix=CHECK-UAD-OFF < %t.out #include #include @@ -32,14 +35,16 @@ Simple *s = new(&buf) Simple(); s->~Simple(); + fprintf(stderr, "\n"); // Need output to parse for CHECK-UAD-OFF case return s->x_; - // CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value - // CHECK: {{#0 0x.* in main.*use-after-dtor.cc:}}[[@LINE-3]] + // CHECK-UAD: WARNING: MemorySanitizer: use-of-uninitialized-value + // CHECK-UAD: {{#0 0x.* in main.*use-after-dtor.cc:}}[[@LINE-3]] // CHECK-ORIGINS: Memory was marked as uninitialized // CHECK-ORIGINS: {{#0 0x.* in __sanitizer_dtor_callback}} // CHECK-ORIGINS: {{#1 0x.* in Simple::~Simple}} - // CHECK: SUMMARY: MemorySanitizer: use-of-uninitialized-value {{.*main}} + // CHECK-UAD: SUMMARY: MemorySanitizer: use-of-uninitialized-value {{.*main}} + // CHECK-UAD-OFF-NOT: SUMMARY: MemorySanitizer: use-of-uninitialized-value } Index: compiler-rt/test/msan/dtor-member.cc === --- compiler-rt/test/msan/dtor-member.cc +++ compiler-rt/test/msan/dtor-member.cc @@ -7,7 +7,7 @@ // RUN: %clangxx_msan %s -O2 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 %run %t >%t.out 2>&1 // RUN: FileCheck %s < %t.out -// RUN: %clangxx_msan %s -fsanitize=memory -o %t && MSAN_OPTIONS=poison_in_dtor=1 %run %t >%t.out 2>&1 +// RUN: %clangxx_msan %s -fsanitize=memory -fno-sanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 %run %t >%t.out 2>&1 // RUN: FileCheck %s --check-prefix=CHECK-NO-FLAG < %t.out // RUN: %clangxx_msan -fsanitize=memory -fsanitize-memory-use-after-dtor %s -o %t && MSAN_OPTIONS=poison_in_dtor=0 %run %t >%t.out 2>&1 Index: clang/test/Driver/fsanitize.c === --- clang/test/Driver/fsanitize.c +++ clang/test/Driver/fsanitize.c @@ -184,11 +184,11 @@ // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fsanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fno-sanitize-memory-use-after-dtor -fsanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR +// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR // CHECK-USE-AFTER-DTOR: -cc1{{.*}}-fsanitize-memory-use-after-dtor // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fno-sanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR-OFF // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fsanitize-memory-use-after-dtor -fno-sanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR-OFF -// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR-OFF // CHECK-USE-AFTER-DTOR-OFF-NOT: -cc1{{.*}}memory-use-after-dtor // RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-field-pa
[PATCH] D37860: [MSan] Enable use-after-dtor instrumentation by default.
morehouse added a comment. PTAL. Patch has been updated. https://reviews.llvm.org/D37860 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41815: [clang-tidy] implement check for goto
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:63 + + The usage of ``goto`` has been discouraged for a long time and is diagnosed + with this check. I think will be good idea to reformulate this statement and its copy in documentation. //diagnosed with this check// is tautological for any check. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41912: [Driver] Test for correct '--version' suggestion
This revision was automatically updated to reflect the committed changes. Closed by commit rC30: [Driver] Test for correct '--version' suggestion (authored by modocache, committed by ). Changed prior to commit: https://reviews.llvm.org/D41912?vs=129314&id=129321#toc Repository: rC Clang https://reviews.llvm.org/D41912 Files: test/Driver/unknown-arg.c Index: test/Driver/unknown-arg.c === --- test/Driver/unknown-arg.c +++ test/Driver/unknown-arg.c @@ -1,6 +1,6 @@ // RUN: not %clang %s -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -### 2>&1 | \ // RUN: FileCheck %s -// RUN: not %clang %s -stdlibs=foo -hell -### 2>&1 | \ +// RUN: not %clang %s -stdlibs=foo -hell -version -### 2>&1 | \ // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN // RUN: %clang_cl -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -### -c -- %s 2>&1 | \ // RUN: FileCheck %s --check-prefix=CL @@ -22,6 +22,7 @@ // CHECK: error: unknown argument: '-funknown-to-clang-option' // DID-YOU-MEAN: error: unknown argument '-stdlibs=foo', did you mean '-stdlib=foo'? // DID-YOU-MEAN: error: unknown argument '-hell', did you mean '-help'? +// DID-YOU-MEAN: error: unknown argument '-version', did you mean '--version'? // CL: warning: unknown argument ignored in clang-cl: '-cake-is-lie' // CL: warning: unknown argument ignored in clang-cl: '-%0' // CL: warning: unknown argument ignored in clang-cl: '-%d' Index: test/Driver/unknown-arg.c === --- test/Driver/unknown-arg.c +++ test/Driver/unknown-arg.c @@ -1,6 +1,6 @@ // RUN: not %clang %s -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -### 2>&1 | \ // RUN: FileCheck %s -// RUN: not %clang %s -stdlibs=foo -hell -### 2>&1 | \ +// RUN: not %clang %s -stdlibs=foo -hell -version -### 2>&1 | \ // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN // RUN: %clang_cl -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -### -c -- %s 2>&1 | \ // RUN: FileCheck %s --check-prefix=CL @@ -22,6 +22,7 @@ // CHECK: error: unknown argument: '-funknown-to-clang-option' // DID-YOU-MEAN: error: unknown argument '-stdlibs=foo', did you mean '-stdlib=foo'? // DID-YOU-MEAN: error: unknown argument '-hell', did you mean '-help'? +// DID-YOU-MEAN: error: unknown argument '-version', did you mean '--version'? // CL: warning: unknown argument ignored in clang-cl: '-cake-is-lie' // CL: warning: unknown argument ignored in clang-cl: '-%0' // CL: warning: unknown argument ignored in clang-cl: '-%d' ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322220 - [Driver] Test for correct '--version' suggestion
Author: modocache Date: Wed Jan 10 12:23:45 2018 New Revision: 30 URL: http://llvm.org/viewvc/llvm-project?rev=30&view=rev Log: [Driver] Test for correct '--version' suggestion Summary: The `llvm::OptTable::findNearest` bug fixed in https://reviews.llvm.org/D41873 manifested itself as the following erroneous message when invoking Clang: ``` clang -version clang-6.0: error: unknown argument '-version', did you mean 'version'? ``` Add a test to catch any future regressions to the now correct behavior, which asks "did you mean '--version'?". Test Plan: `check-clang` Reviewers: v.g.vassilev, teemperor, ruiu, jroelofs, yamaguchi Reviewed By: v.g.vassilev Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D41912 Modified: cfe/trunk/test/Driver/unknown-arg.c Modified: cfe/trunk/test/Driver/unknown-arg.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/unknown-arg.c?rev=30&r1=322219&r2=30&view=diff == --- cfe/trunk/test/Driver/unknown-arg.c (original) +++ cfe/trunk/test/Driver/unknown-arg.c Wed Jan 10 12:23:45 2018 @@ -1,6 +1,6 @@ // RUN: not %clang %s -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -### 2>&1 | \ // RUN: FileCheck %s -// RUN: not %clang %s -stdlibs=foo -hell -### 2>&1 | \ +// RUN: not %clang %s -stdlibs=foo -hell -version -### 2>&1 | \ // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN // RUN: %clang_cl -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -### -c -- %s 2>&1 | \ // RUN: FileCheck %s --check-prefix=CL @@ -22,6 +22,7 @@ // CHECK: error: unknown argument: '-funknown-to-clang-option' // DID-YOU-MEAN: error: unknown argument '-stdlibs=foo', did you mean '-stdlib=foo'? // DID-YOU-MEAN: error: unknown argument '-hell', did you mean '-help'? +// DID-YOU-MEAN: error: unknown argument '-version', did you mean '--version'? // CL: warning: unknown argument ignored in clang-cl: '-cake-is-lie' // CL: warning: unknown argument ignored in clang-cl: '-%0' // CL: warning: unknown argument ignored in clang-cl: '-%d' ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41912: [Driver] Test for correct '--version' suggestion
modocache added a comment. Thanks, will do next time :) Repository: rC Clang https://reviews.llvm.org/D41912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322221 - [MSan] Enable use-after-dtor instrumentation by default.
Author: morehouse Date: Wed Jan 10 12:27:48 2018 New Revision: 31 URL: http://llvm.org/viewvc/llvm-project?rev=31&view=rev Log: [MSan] Enable use-after-dtor instrumentation by default. Summary: Enable the compile-time flag -fsanitize-memory-use-after-dtor by default. Note that the run-time option MSAN_OPTIONS=poison_in_dtor=1 still needs to be enabled for destructors to be poisoned. Reviewers: eugenis, vitalybuka, kcc Reviewed By: eugenis, vitalybuka Subscribers: cfe-commits, llvm-commits Differential Revision: https://reviews.llvm.org/D37860 Modified: cfe/trunk/include/clang/Driver/SanitizerArgs.h cfe/trunk/test/CodeGenCXX/sanitize-no-dtor-callback.cpp cfe/trunk/test/Driver/fsanitize.c Modified: cfe/trunk/include/clang/Driver/SanitizerArgs.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/SanitizerArgs.h?rev=31&r1=30&r2=31&view=diff == --- cfe/trunk/include/clang/Driver/SanitizerArgs.h (original) +++ cfe/trunk/include/clang/Driver/SanitizerArgs.h Wed Jan 10 12:27:48 2018 @@ -30,7 +30,7 @@ class SanitizerArgs { std::vector ExtraDeps; int CoverageFeatures = 0; int MsanTrackOrigins = 0; - bool MsanUseAfterDtor = false; + bool MsanUseAfterDtor = true; bool CfiCrossDso = false; bool CfiICallGeneralizePointers = false; int AsanFieldPadding = 0; Modified: cfe/trunk/test/CodeGenCXX/sanitize-no-dtor-callback.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/sanitize-no-dtor-callback.cpp?rev=31&r1=30&r2=31&view=diff == --- cfe/trunk/test/CodeGenCXX/sanitize-no-dtor-callback.cpp (original) +++ cfe/trunk/test/CodeGenCXX/sanitize-no-dtor-callback.cpp Wed Jan 10 12:27:48 2018 @@ -1,8 +1,9 @@ -// Test without the flag -fsanitize-memory-use-after-dtor, to ensure that +// Test with the flag -fno-sanitize-memory-use-after-dtor, to ensure that // instrumentation is not erroneously inserted -// RUN: %clang_cc1 -fsanitize=memory -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s +// RUN: %clang_cc1 -fsanitize=memory -fno-sanitize-memory-use-after-dtor -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s struct Simple { + int x; ~Simple() {} }; Simple s; @@ -10,6 +11,7 @@ Simple s; // CHECK-NOT: call void @__sanitizer_dtor_callback struct Inlined { + int x; inline ~Inlined() {} }; Inlined i; Modified: cfe/trunk/test/Driver/fsanitize.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize.c?rev=31&r1=30&r2=31&view=diff == --- cfe/trunk/test/Driver/fsanitize.c (original) +++ cfe/trunk/test/Driver/fsanitize.c Wed Jan 10 12:27:48 2018 @@ -184,11 +184,11 @@ // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fsanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fno-sanitize-memory-use-after-dtor -fsanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR +// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR // CHECK-USE-AFTER-DTOR: -cc1{{.*}}-fsanitize-memory-use-after-dtor // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fno-sanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR-OFF // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fsanitize-memory-use-after-dtor -fno-sanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR-OFF -// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR-OFF // CHECK-USE-AFTER-DTOR-OFF-NOT: -cc1{{.*}}memory-use-after-dtor // RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-field-padding=0 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-FIELD-PADDING-0 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37860: [MSan] Enable use-after-dtor instrumentation by default.
This revision was automatically updated to reflect the committed changes. Closed by commit rCRT31: [MSan] Enable use-after-dtor instrumentation by default. (authored by morehouse, committed by ). Herald added a subscriber: Sanitizers. Changed prior to commit: https://reviews.llvm.org/D37860?vs=129318&id=129322#toc Repository: rCRT Compiler Runtime https://reviews.llvm.org/D37860 Files: test/msan/dtor-member.cc test/msan/use-after-dtor.cc Index: test/msan/use-after-dtor.cc === --- test/msan/use-after-dtor.cc +++ test/msan/use-after-dtor.cc @@ -1,14 +1,17 @@ // RUN: %clangxx_msan %s -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1 -// RUN: FileCheck %s < %t.out +// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out // RUN: %clangxx_msan %s -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1 -// RUN: FileCheck %s < %t.out +// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out // RUN: %clangxx_msan %s -O2 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1 -// RUN: FileCheck %s < %t.out +// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out // RUN: %clangxx_msan %s -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -fsanitize-memory-track-origins -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1 -// RUN: FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out +// RUN: FileCheck %s --check-prefixes=CHECK-UAD,CHECK-ORIGINS < %t.out + +// RUN: %clangxx_msan %s -fno-sanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t > %t.out 2>&1 +// RUN: FileCheck %s --check-prefix=CHECK-UAD-OFF < %t.out #include #include @@ -32,14 +35,16 @@ Simple *s = new(&buf) Simple(); s->~Simple(); + fprintf(stderr, "\n"); // Need output to parse for CHECK-UAD-OFF case return s->x_; - // CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value - // CHECK: {{#0 0x.* in main.*use-after-dtor.cc:}}[[@LINE-3]] + // CHECK-UAD: WARNING: MemorySanitizer: use-of-uninitialized-value + // CHECK-UAD: {{#0 0x.* in main.*use-after-dtor.cc:}}[[@LINE-3]] // CHECK-ORIGINS: Memory was marked as uninitialized // CHECK-ORIGINS: {{#0 0x.* in __sanitizer_dtor_callback}} // CHECK-ORIGINS: {{#1 0x.* in Simple::~Simple}} - // CHECK: SUMMARY: MemorySanitizer: use-of-uninitialized-value {{.*main}} + // CHECK-UAD: SUMMARY: MemorySanitizer: use-of-uninitialized-value {{.*main}} + // CHECK-UAD-OFF-NOT: SUMMARY: MemorySanitizer: use-of-uninitialized-value } Index: test/msan/dtor-member.cc === --- test/msan/dtor-member.cc +++ test/msan/dtor-member.cc @@ -7,7 +7,7 @@ // RUN: %clangxx_msan %s -O2 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 %run %t >%t.out 2>&1 // RUN: FileCheck %s < %t.out -// RUN: %clangxx_msan %s -fsanitize=memory -o %t && MSAN_OPTIONS=poison_in_dtor=1 %run %t >%t.out 2>&1 +// RUN: %clangxx_msan %s -fsanitize=memory -fno-sanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 %run %t >%t.out 2>&1 // RUN: FileCheck %s --check-prefix=CHECK-NO-FLAG < %t.out // RUN: %clangxx_msan -fsanitize=memory -fsanitize-memory-use-after-dtor %s -o %t && MSAN_OPTIONS=poison_in_dtor=0 %run %t >%t.out 2>&1 Index: test/msan/use-after-dtor.cc === --- test/msan/use-after-dtor.cc +++ test/msan/use-after-dtor.cc @@ -1,14 +1,17 @@ // RUN: %clangxx_msan %s -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1 -// RUN: FileCheck %s < %t.out +// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out // RUN: %clangxx_msan %s -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1 -// RUN: FileCheck %s < %t.out +// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out // RUN: %clangxx_msan %s -O2 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1 -// RUN: FileCheck %s < %t.out +// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out // RUN: %clangxx_msan %s -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -fsanitize-memory-track-origins -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1 -// RUN: FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out +// RUN: FileCheck %s --check-prefixes=CHECK-UAD,CHECK-ORIGINS < %t.out + +// RUN: %clangxx_msan %s -fno-sanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t > %t.out 2>&1 +// RUN: FileCheck %s --check-prefix=CHECK-UAD-OFF < %t.out #include #include @@ -32,14 +35,16 @@ Simple *s = new(&buf) Simple(); s->~Simple();
[PATCH] D41809: Clang counterpart change for fuzzer FreeBSD support
devnexen updated this revision to Diff 129329. https://reviews.llvm.org/D41809 Files: lib/Driver/ToolChains/FreeBSD.cpp Index: lib/Driver/ToolChains/FreeBSD.cpp === --- lib/Driver/ToolChains/FreeBSD.cpp +++ lib/Driver/ToolChains/FreeBSD.cpp @@ -392,6 +392,8 @@ } if (IsX86 || IsX86_64) { Res |= SanitizerKind::SafeStack; +Res |= SanitizerKind::Fuzzer; +Res |= SanitizerKind::FuzzerNoLink; } return Res; } Index: lib/Driver/ToolChains/FreeBSD.cpp === --- lib/Driver/ToolChains/FreeBSD.cpp +++ lib/Driver/ToolChains/FreeBSD.cpp @@ -392,6 +392,8 @@ } if (IsX86 || IsX86_64) { Res |= SanitizerKind::SafeStack; +Res |= SanitizerKind::Fuzzer; +Res |= SanitizerKind::FuzzerNoLink; } return Res; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40925: Add option -fkeep-static-consts
eandrews added a comment. Thanks for the review Reid. Sorry for the delay in my response. I was OOO. I am not sure if a new attribute is necessary. __ attribute __(used) is already supported in Clang. While this attribute can be used to retain static constants, it would require the user to modify the source which may not always be possible/practical. Its also interesting to note that GCC actually retains unused static constants by default. fno-keep-static-consts is used to remove unused static constants in GCC. https://reviews.llvm.org/D40925 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41918: [libunwind] Set up .arcconfig to point to new Diffusion UNW repository
benhamilton accepted this revision. benhamilton added a comment. This revision is now accepted and ready to land. Too funny. I'll abandon https://reviews.llvm.org/D41917. Repository: rUNW libunwind https://reviews.llvm.org/D41918 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41918: [libunwind] Set up .arcconfig to point to new Diffusion UNW repository
This revision was automatically updated to reflect the committed changes. Closed by commit rL38: [libunwind] Set up .arcconfig to point to new Diffusion UNW repository (authored by phosek, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D41918?vs=129339&id=129344#toc Repository: rL LLVM https://reviews.llvm.org/D41918 Files: libunwind/trunk/.arcconfig Index: libunwind/trunk/.arcconfig === --- libunwind/trunk/.arcconfig +++ libunwind/trunk/.arcconfig @@ -1,4 +1,4 @@ { - "project_id" : "libunwind", + "repository.callsign" : "UNW", "conduit_uri" : "https://reviews.llvm.org/"; } Index: libunwind/trunk/.arcconfig === --- libunwind/trunk/.arcconfig +++ libunwind/trunk/.arcconfig @@ -1,4 +1,4 @@ { - "project_id" : "libunwind", + "repository.callsign" : "UNW", "conduit_uri" : "https://reviews.llvm.org/"; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] r322228 - [libunwind] Set up .arcconfig to point to new Diffusion UNW repository
Author: phosek Date: Wed Jan 10 14:20:03 2018 New Revision: 38 URL: http://llvm.org/viewvc/llvm-project?rev=38&view=rev Log: [libunwind] Set up .arcconfig to point to new Diffusion UNW repository See http://lists.llvm.org/pipermail/cfe-dev/2017-November/056032.html for related discussion and context. Differential Revision: https://reviews.llvm.org/D41918 Modified: libunwind/trunk/.arcconfig Modified: libunwind/trunk/.arcconfig URL: http://llvm.org/viewvc/llvm-project/libunwind/trunk/.arcconfig?rev=38&r1=37&r2=38&view=diff == --- libunwind/trunk/.arcconfig (original) +++ libunwind/trunk/.arcconfig Wed Jan 10 14:20:03 2018 @@ -1,4 +1,4 @@ { - "project_id" : "libunwind", + "repository.callsign" : "UNW", "conduit_uri" : "https://reviews.llvm.org/"; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41039: Add support for attribute "trivial_abi"
ahatanak updated this revision to Diff 129345. ahatanak marked 2 inline comments as done. ahatanak added a comment. Partially revert the changes I made to CodeGenFunction::EmitParmDecl add IRGen test case for exception handling. https://reviews.llvm.org/D41039 Files: include/clang/AST/ASTContext.h include/clang/AST/Decl.h include/clang/AST/DeclCXX.h include/clang/AST/Type.h include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/AST/ASTContext.cpp lib/AST/DeclCXX.cpp lib/AST/Type.cpp lib/CodeGen/CGCall.cpp lib/CodeGen/CGDecl.cpp lib/CodeGen/ItaniumCXXABI.cpp lib/CodeGen/MicrosoftCXXABI.cpp lib/Sema/SemaChecking.cpp lib/Sema/SemaDeclAttr.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaTemplateInstantiate.cpp lib/Sema/SemaType.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriter.cpp lib/Serialization/ASTWriterDecl.cpp test/CodeGenCXX/trivial_abi.cpp test/CodeGenObjCXX/trivial_abi.mm test/Misc/pragma-attribute-supported-attributes-list.test test/SemaObjCXX/attr-trivial-abi.mm Index: test/SemaObjCXX/attr-trivial-abi.mm === --- /dev/null +++ test/SemaObjCXX/attr-trivial-abi.mm @@ -0,0 +1,93 @@ +// RUN: %clang_cc1 -std=c++11 -fobjc-runtime-has-weak -fobjc-weak -fobjc-arc -fsyntax-only -verify %s + +void __attribute__((trivial_abi)) foo(); // expected-warning {{'trivial_abi' attribute only applies to classes}} + +struct [[clang::trivial_abi]] S0 { + int a; +}; + +struct __attribute__((trivial_abi)) S1 { + int a; +}; + +struct __attribute__((trivial_abi)) S2 { // expected-warning {{'trivial_abi' cannot be applied to 'S2'}} + __weak id a; +}; + +struct __attribute__((trivial_abi)) S3 { // expected-warning {{'trivial_abi' cannot be applied to 'S3'}} + virtual void m(); +}; + +struct S4 { + int a; +}; + +struct __attribute__((trivial_abi)) S5 : public virtual S4 { // expected-warning {{'trivial_abi' cannot be applied to 'S5'}} +}; + +struct __attribute__((trivial_abi)) S9 : public S4 { +}; + +struct S6 { + __weak id a; +}; + +struct __attribute__((trivial_abi)) S12 { // expected-warning {{'trivial_abi' cannot be applied to 'S12'}} + __weak id a; +}; + +struct __attribute__((trivial_abi)) S13 { // expected-warning {{'trivial_abi' cannot be applied to 'S13'}} + __weak id a[2]; +}; + +struct __attribute__((trivial_abi)) S7 { // expected-warning {{'trivial_abi' cannot be applied to 'S7'}} + S6 a; +}; + +struct __attribute__((trivial_abi)) S11 { // expected-warning {{'trivial_abi' cannot be applied to 'S11'}} + S6 a[2]; +}; + +struct __attribute__((trivial_abi(1))) S8 { // expected-error {{'trivial_abi' attribute takes no arguments}} + int a; +}; + +// Do not warn when 'trivial_abi' is used to annotate a template class. +template +struct __attribute__((trivial_abi)) S10 { + T p; +}; + +S10 p1; +S10<__weak id> p2; + +template<> +struct __attribute__((trivial_abi)) S10 { // expected-warning {{'trivial_abi' cannot be applied to 'S10'}} + __weak id a; +}; + +template +struct S14 { + T a; + __weak id b; +}; + +template +struct __attribute__((trivial_abi)) S15 : S14 { +}; + +S15 s15; + +template +struct __attribute__((trivial_abi)) S16 { + S14 a; +}; + +S16 s16; + +template +struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}} + __weak id a; +}; + +S17 s17; Index: test/Misc/pragma-attribute-supported-attributes-list.test === --- test/Misc/pragma-attribute-supported-attributes-list.test +++ test/Misc/pragma-attribute-supported-attributes-list.test @@ -2,7 +2,7 @@ // The number of supported attributes should never go down! -// CHECK: #pragma clang attribute supports 66 attributes: +// CHECK: #pragma clang attribute supports 67 attributes: // CHECK-NEXT: AMDGPUFlatWorkGroupSize (SubjectMatchRule_function) // CHECK-NEXT: AMDGPUNumSGPR (SubjectMatchRule_function) // CHECK-NEXT: AMDGPUNumVGPR (SubjectMatchRule_function) @@ -66,6 +66,7 @@ // CHECK-NEXT: TLSModel (SubjectMatchRule_variable_is_thread_local) // CHECK-NEXT: Target (SubjectMatchRule_function) // CHECK-NEXT: TestTypestate (SubjectMatchRule_function_is_member) +// CHECK-NEXT: TrivialABI (SubjectMatchRule_record) // CHECK-NEXT: WarnUnusedResult (SubjectMatchRule_objc_method, SubjectMatchRule_enum, SubjectMatchRule_record, SubjectMatchRule_hasType_functionType) // CHECK-NEXT: XRayInstrument (SubjectMatchRule_function, SubjectMatchRule_objc_method) // CHECK-NEXT: XRayLogArgs (SubjectMatchRule_function, SubjectMatchRule_objc_method) Index: test/CodeGenObjCXX/trivial_abi.mm === --- /dev/null +++ test/CodeGenObjCXX/trivial_abi.mm @@ -0,0 +1,103 @@ +// RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++11 -fobjc-arc -fobjc-weak -fobjc-runtime-has-weak -emit-llvm -o -
[PATCH] D41423: [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral.
vsapsai updated this revision to Diff 129346. vsapsai added a comment. - Remove rdar link from the comment per review. Also rebased on top of master so diff between diffs can be noisy. https://reviews.llvm.org/D41423 Files: clang/lib/Lex/Lexer.cpp clang/unittests/Lex/LexerTest.cpp Index: clang/unittests/Lex/LexerTest.cpp === --- clang/unittests/Lex/LexerTest.cpp +++ clang/unittests/Lex/LexerTest.cpp @@ -474,8 +474,9 @@ } TEST_F(LexerTest, AvoidPastEndOfStringDereference) { - std::vector LexedTokens = Lex(" // \\\n"); - EXPECT_TRUE(LexedTokens.empty()); + EXPECT_TRUE(Lex(" // \\\n").empty()); + EXPECT_TRUE(Lex("#include <").empty()); + EXPECT_TRUE(Lex("#include <\n").empty()); } TEST_F(LexerTest, StringizingRasString) { Index: clang/lib/Lex/Lexer.cpp === --- clang/lib/Lex/Lexer.cpp +++ clang/lib/Lex/Lexer.cpp @@ -2009,18 +2009,21 @@ const char *AfterLessPos = CurPtr; char C = getAndAdvanceChar(CurPtr, Result); while (C != '>') { -// Skip escaped characters. -if (C == '\\' && CurPtr < BufferEnd) { - // Skip the escaped character. - getAndAdvanceChar(CurPtr, Result); -} else if (C == '\n' || C == '\r' || // Newline. - (C == 0 && (CurPtr-1 == BufferEnd || // End of file. - isCodeCompletionPoint(CurPtr-1 { +// Skip escaped characters. Escaped newlines will already be processed by +// getAndAdvanceChar. +if (C == '\\') + C = getAndAdvanceChar(CurPtr, Result); + +if (C == '\n' || C == '\r' || // Newline. +(C == 0 && (CurPtr-1 == BufferEnd || // End of file. +isCodeCompletionPoint(CurPtr-1 { // If the filename is unterminated, then it must just be a lone < // character. Return this as such. FormTokenWithChars(Result, AfterLessPos, tok::less); return true; -} else if (C == 0) { +} + +if (C == 0) { NulCharacter = CurPtr-1; } C = getAndAdvanceChar(CurPtr, Result); Index: clang/unittests/Lex/LexerTest.cpp === --- clang/unittests/Lex/LexerTest.cpp +++ clang/unittests/Lex/LexerTest.cpp @@ -474,8 +474,9 @@ } TEST_F(LexerTest, AvoidPastEndOfStringDereference) { - std::vector LexedTokens = Lex(" // \\\n"); - EXPECT_TRUE(LexedTokens.empty()); + EXPECT_TRUE(Lex(" // \\\n").empty()); + EXPECT_TRUE(Lex("#include <").empty()); + EXPECT_TRUE(Lex("#include <\n").empty()); } TEST_F(LexerTest, StringizingRasString) { Index: clang/lib/Lex/Lexer.cpp === --- clang/lib/Lex/Lexer.cpp +++ clang/lib/Lex/Lexer.cpp @@ -2009,18 +2009,21 @@ const char *AfterLessPos = CurPtr; char C = getAndAdvanceChar(CurPtr, Result); while (C != '>') { -// Skip escaped characters. -if (C == '\\' && CurPtr < BufferEnd) { - // Skip the escaped character. - getAndAdvanceChar(CurPtr, Result); -} else if (C == '\n' || C == '\r' || // Newline. - (C == 0 && (CurPtr-1 == BufferEnd || // End of file. - isCodeCompletionPoint(CurPtr-1 { +// Skip escaped characters. Escaped newlines will already be processed by +// getAndAdvanceChar. +if (C == '\\') + C = getAndAdvanceChar(CurPtr, Result); + +if (C == '\n' || C == '\r' || // Newline. +(C == 0 && (CurPtr-1 == BufferEnd || // End of file. +isCodeCompletionPoint(CurPtr-1 { // If the filename is unterminated, then it must just be a lone < // character. Return this as such. FormTokenWithChars(Result, AfterLessPos, tok::less); return true; -} else if (C == 0) { +} + +if (C == 0) { NulCharacter = CurPtr-1; } C = getAndAdvanceChar(CurPtr, Result); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41039: Add support for attribute "trivial_abi"
ahatanak added inline comments. Comment at: lib/Sema/SemaDecl.cpp:11700 +} + } + ahatanak wrote: > rsmith wrote: > > rjmccall wrote: > > > I think it's correct not to call CheckDestructorAccess and > > > DiagnoseUseOfDecl here, since according to the standard destructor access > > > is always supposed to be checked at the call-site, but please leave a > > > comment explaining that. > > The corresponding code for `areArgsDestroyedLeftToRightInCallee` is in > > `SemaChecking`. This should be done in the same place. > > > > More generally, we have at least three different places where we check > > `CXXABI::areArgsDestroyedLeftToRightInCallee() || > > Type->hasTrivialABIOverride()`. It would make sense to factor that out into > > a `isParamDestroyedInCallee` function (probably on `ASTContext`, since we > > don't have anywhere better for ABI-specific checks at a layering level that > > can talk about types). > I defined function ASTContext::isParamDestroyedInCallee and used it in two > places. I didn't use it in CodeGenFunction::EmitParmDecl because the > destructor cleanup is pushed only when the argument is passed indirectly in > MSVC's case, whereas it is always pushed when the class is > HasTrivialABIOverride. I was misunderstanding the code in CodeGenFunction::EmitParmDecl. When the argument is a class that is passed by value, Arg.isIndirect() returns true even when the argument is passed directly. The code that pushes the destruction cleanup should remain in the if part of the if statement and isParamDestroyedInCallee can be used to determine whether the parameter is destructed in the callee. https://reviews.llvm.org/D41039 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41423: [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral.
vsapsai marked an inline comment as done. vsapsai added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:2014-2015 +// getAndAdvanceChar. +if (C == '\\') + C = getAndAdvanceChar(CurPtr, Result); + dexonsmith wrote: > If `CurPtr` is already equal to `BufferEnd`, why is it safe to call > `getAndAdvanceChar`? Is `BufferEnd` dereferenceable? `BufferEnd` is still dereferancable but we do rely on it storing null character. `CurPtr < BufferEnd` check was added in https://reviews.llvm.org/D9489 to handle `#include <\` (no new line in the end). Before that change the buffer overflow was happening because after `\` we read null character at line 2015 (`CurPtr` becomes `BufferEnd+1`) and then one more character at line 2026. `CurPtr < BufferEnd` makes sure we can read 2 bytes before starting this 2-character sequence. It works fine when each character is 1 byte but fails when it is more. In this case backslash and new line are read as 1 character and `CurPtr < BufferEnd` check is insufficient. In my fix I read the character after backslash and then decide if can read the next one, so it doesn't matter how many bytes are in this character. Comment at: clang/lib/Lex/Lexer.cpp:2026 + +if (C == 0) { NulCharacter = CurPtr-1; dexonsmith wrote: > Should this check still be skipped (in an `else if` of the `C == '\\'` check)? I agree it is behaviour change. `NulCharacter` is used to warn if you have null character in the string and I think it is worth warning even if it is preceded by the backslash. Therefore I think this check shouldn't be skipped after `C == '\\'` check. In practice I don't know how you can create a file with null character in its name and use it in inclusion directive. Comment at: clang/unittests/Lex/LexerTest.cpp:477 TEST_F(LexerTest, AvoidPastEndOfStringDereference) { - std::vector LexedTokens = Lex(" // \\\n"); - EXPECT_TRUE(LexedTokens.empty()); + EXPECT_TRUE(Lex(" // \\\n").empty()); + // rdar://problem/35572754 dexonsmith wrote: > To minimize the diff, please separate this change into an NFC commit ahead of > time. I plan to nominate this fix for inclusion in 6.0.0 release and having one commit will be easier. It is not necessary to include NFC change in 6.0.0 release but it creates discrepancy that increases a chance of merge conflicts. But I don't have strong opinion, just pointing out potential downsides with merging the change to other branches. https://reviews.llvm.org/D41423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322233 - Reland "[Driver] Update default sanitizer blacklist location"
Author: phosek Date: Wed Jan 10 14:59:00 2018 New Revision: 322233 URL: http://llvm.org/viewvc/llvm-project?rev=322233&view=rev Log: Reland "[Driver] Update default sanitizer blacklist location" This is related to moving the sanitizer blacklists to share/ subdirectory. Differential Revision: https://reviews.llvm.org/D41706 Added: cfe/trunk/test/Driver/Inputs/resource_dir/share/asan_blacklist.txt - copied, changed from r31, cfe/trunk/test/Driver/Inputs/resource_dir/asan_blacklist.txt cfe/trunk/test/Driver/Inputs/resource_dir/share/hwasan_blacklist.txt - copied, changed from r31, cfe/trunk/test/Driver/Inputs/resource_dir/hwasan_blacklist.txt cfe/trunk/test/Driver/Inputs/resource_dir/share/ubsan_blacklist.txt - copied, changed from r31, cfe/trunk/test/Driver/Inputs/resource_dir/ubsan_blacklist.txt cfe/trunk/test/Driver/Inputs/resource_dir/share/vtables_blacklist.txt - copied, changed from r31, cfe/trunk/test/Driver/Inputs/resource_dir/vtables_blacklist.txt Removed: cfe/trunk/test/Driver/Inputs/resource_dir/asan_blacklist.txt cfe/trunk/test/Driver/Inputs/resource_dir/hwasan_blacklist.txt cfe/trunk/test/Driver/Inputs/resource_dir/ubsan_blacklist.txt cfe/trunk/test/Driver/Inputs/resource_dir/vtables_blacklist.txt Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=322233&r1=322232&r2=322233&view=diff == --- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original) +++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Wed Jan 10 14:59:00 2018 @@ -112,7 +112,7 @@ static bool getDefaultBlacklist(const Dr if (BlacklistFile) { clang::SmallString<64> Path(D.ResourceDir); -llvm::sys::path::append(Path, BlacklistFile); +llvm::sys::path::append(Path, "share", BlacklistFile); BLPath = Path.str(); return true; } Removed: cfe/trunk/test/Driver/Inputs/resource_dir/asan_blacklist.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir/asan_blacklist.txt?rev=322232&view=auto == (empty) Removed: cfe/trunk/test/Driver/Inputs/resource_dir/hwasan_blacklist.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir/hwasan_blacklist.txt?rev=322232&view=auto == (empty) Copied: cfe/trunk/test/Driver/Inputs/resource_dir/share/asan_blacklist.txt (from r31, cfe/trunk/test/Driver/Inputs/resource_dir/asan_blacklist.txt) URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir/share/asan_blacklist.txt?p2=cfe/trunk/test/Driver/Inputs/resource_dir/share/asan_blacklist.txt&p1=cfe/trunk/test/Driver/Inputs/resource_dir/asan_blacklist.txt&r1=31&r2=322233&rev=322233&view=diff == (empty) Copied: cfe/trunk/test/Driver/Inputs/resource_dir/share/hwasan_blacklist.txt (from r31, cfe/trunk/test/Driver/Inputs/resource_dir/hwasan_blacklist.txt) URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir/share/hwasan_blacklist.txt?p2=cfe/trunk/test/Driver/Inputs/resource_dir/share/hwasan_blacklist.txt&p1=cfe/trunk/test/Driver/Inputs/resource_dir/hwasan_blacklist.txt&r1=31&r2=322233&rev=322233&view=diff == (empty) Copied: cfe/trunk/test/Driver/Inputs/resource_dir/share/ubsan_blacklist.txt (from r31, cfe/trunk/test/Driver/Inputs/resource_dir/ubsan_blacklist.txt) URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir/share/ubsan_blacklist.txt?p2=cfe/trunk/test/Driver/Inputs/resource_dir/share/ubsan_blacklist.txt&p1=cfe/trunk/test/Driver/Inputs/resource_dir/ubsan_blacklist.txt&r1=31&r2=322233&rev=322233&view=diff == (empty) Copied: cfe/trunk/test/Driver/Inputs/resource_dir/share/vtables_blacklist.txt (from r31, cfe/trunk/test/Driver/Inputs/resource_dir/vtables_blacklist.txt) URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir/share/vtables_blacklist.txt?p2=cfe/trunk/test/Driver/Inputs/resource_dir/share/vtables_blacklist.txt&p1=cfe/trunk/test/Driver/Inputs/resource_dir/vtables_blacklist.txt&r1=31&r2=322233&rev=322233&view=diff == (empty) Removed: cfe/trunk/test/Driver/Inputs/resource_dir/ubsan_blacklist.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir/ubsan_blacklist.txt?rev=322232&view=auto ===
r322236 - In C++17, when instantiating an out-of-line definition of an inline static data
Author: rsmith Date: Wed Jan 10 15:08:26 2018 New Revision: 322236 URL: http://llvm.org/viewvc/llvm-project?rev=322236&view=rev Log: In C++17, when instantiating an out-of-line definition of an inline static data member, don't forget to instantiate the initializer too. Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp cfe/trunk/test/CodeGenCXX/cxx1z-inline-variables.cpp cfe/trunk/test/SemaTemplate/cxx17-inline-variables.cpp Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=322236&r1=322235&r2=322236&view=diff == --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Jan 10 15:08:26 2018 @@ -4151,7 +4151,8 @@ void Sema::BuildVariableInstantiation( // it right away if the type contains 'auto'. if ((!isa(NewVar) && !InstantiatingVarTemplate && - !(OldVar->isInline() && OldVar->isThisDeclarationADefinition())) || + !(OldVar->isInline() && OldVar->isThisDeclarationADefinition() && + !NewVar->isThisDeclarationADefinition())) || NewVar->getType()->isUndeducedType()) InstantiateVariableInitializer(NewVar, OldVar, TemplateArgs); Modified: cfe/trunk/test/CodeGenCXX/cxx1z-inline-variables.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx1z-inline-variables.cpp?rev=322236&r1=322235&r2=322236&view=diff == --- cfe/trunk/test/CodeGenCXX/cxx1z-inline-variables.cpp (original) +++ cfe/trunk/test/CodeGenCXX/cxx1z-inline-variables.cpp Wed Jan 10 15:08:26 2018 @@ -58,14 +58,22 @@ template struct X { static int a; static inline int b; static int c; + static const int d; + static int e; }; // CHECK: @_ZN1XIiE1aE = linkonce_odr global i32 10 // CHECK: @_ZN1XIiE1bE = global i32 20 // CHECK-NOT: @_ZN1XIiE1cE +// CHECK: @_ZN1XIiE1dE = linkonce_odr constant i32 40 +// CHECK: @_ZN1XIiE1eE = linkonce_odr global i32 50 template<> inline int X::a = 10; int &use3 = X::a; template<> int X::b = 20; template<> inline int X::c = 30; +template constexpr int X::d = 40; +template inline int X::e = 50; +const int *use_x_int_d = &X::d; +const int *use_x_int_e = &X::e; template struct Y; template<> struct Y { Modified: cfe/trunk/test/SemaTemplate/cxx17-inline-variables.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/cxx17-inline-variables.cpp?rev=322236&r1=322235&r2=322236&view=diff == --- cfe/trunk/test/SemaTemplate/cxx17-inline-variables.cpp (original) +++ cfe/trunk/test/SemaTemplate/cxx17-inline-variables.cpp Wed Jan 10 15:08:26 2018 @@ -16,3 +16,14 @@ namespace CompleteType { constexpr int n = X::value; } + +template struct A { + static const int n; + static const int m; + constexpr int f() { return n; } + constexpr int g() { return n; } +}; +template constexpr int A::n = sizeof(A) + sizeof(T); +template inline constexpr int A::m = sizeof(A) + sizeof(T); +static_assert(A().f() == 5); +static_assert(A().g() == 5); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41921: [Parse] Forward brace locations to TypeConstructExpr
vsk created this revision. vsk added reviewers: rsmith, aaron.ballman, faisalv. When parsing C++ type construction expressions with list initialization, forward the locations of the braces to Sema. Without these locations, the code coverage pass crashes on the given test case, because the pass relies on getLocEnd() returning a valid location. Here is what this patch does in more detail: - Forwards init-list brace locations to Sema (ParseExprCXX), - Builds an InitializationKind with these locations (SemaExprCXX), and - Uses these locations for constructor initialization (SemaInit). The remaining changes fall out of introducing a new overload for creating direct-list InitializationKinds. Testing: check-clang, and a stage2 coverage-enabled build of clang with asserts enabled. https://reviews.llvm.org/D41921 Files: include/clang/AST/ExprCXX.h include/clang/Sema/Initialization.h include/clang/Sema/Sema.h lib/CodeGen/CoverageMappingGen.cpp lib/Parse/ParseExprCXX.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaExprCXX.cpp lib/Sema/SemaInit.cpp lib/Sema/TreeTransform.h test/CoverageMapping/classtemplate.cpp test/SemaCXX/sourceranges.cpp Index: test/SemaCXX/sourceranges.cpp === --- test/SemaCXX/sourceranges.cpp +++ test/SemaCXX/sourceranges.cpp @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -triple i686-mingw32 -ast-dump %s | FileCheck %s +// RUN: %clang_cc1 -triple i686-mingw32 -std=c++1z -ast-dump %s | FileCheck %s -check-prefix=CHECK-1Z template class P { @@ -12,7 +13,7 @@ typedef int C; } -// CHECK: VarDecl {{0x[0-9a-fA-F]+}} col:15 ImplicitConstrArray 'foo::A [2]' +// CHECK: VarDecl {{0x[0-9a-fA-F]+}} col:15 ImplicitConstrArray 'foo::A [2]' static foo::A ImplicitConstrArray[2]; int main() { @@ -50,3 +51,89 @@ D d = D(12); // CHECK: CXXConstructExpr {{0x[0-9a-fA-F]+}} 'D' 'void (int){{( __attribute__\(\(thiscall\)\))?}}' } + +void abort() __attribute__((noreturn)); + +namespace std { +typedef decltype(sizeof(int)) size_t; + +template struct initializer_list { + const E *p; + size_t n; + initializer_list(const E *p, size_t n) : p(p), n(n) {} +}; + +template struct pair { + F f; + S s; + pair(const F &f, const S &s) : f(f), s(s) {} +}; + +struct string { + const char *str; + string() { abort(); } + string(const char *S) : str(S) {} + ~string() { abort(); } +}; + +template +struct map { + using T = pair; + map(initializer_list i, const string &s = string()) {} + ~map() { abort(); } +}; + +}; // namespace std + +#if __cplusplus >= 201703L +// CHECK-1Z: FunctionDecl {{.*}} construct_with_init_list +std::map construct_with_init_list() { + // CHECK-1Z-NEXT: CompoundStmt + // CHECK-1Z-NEXT: ReturnStmt {{.*}} {{0, 0}}; +} + +// CHECK-1Z: NamespaceDecl {{.*}} in_class_init +namespace in_class_init { + struct A {}; + + // CHECK-1Z: CXXRecordDecl {{.*}} struct B definition + struct B { +// CHECK-1Z: FieldDecl {{.*}} a 'in_class_init::A' +// CHECK-1Z-NEXT: InitListExpr {{.*}} class Test { @@ -44,11 +45,51 @@ void unmangleable(UninstantiatedClassWithTraits x) {} }; +void abort() __attribute__((noreturn)); + +namespace std { +typedef decltype(sizeof(int)) size_t; + +template struct initializer_list { + const E *p; + size_t n; + initializer_list(const E *p, size_t n) : p(p), n(n) {} +}; + +template struct pair { + F f; + S s; + pair(const F &f, const S &s) : f(f), s(s) {} +}; + +struct string { + const char *str; + string() { abort(); } + string(const char *S) : str(S) {} + ~string() { abort(); } +}; + +template +struct map { + using T = pair; + map(initializer_list i, const string &s = string()) {} + ~map() { abort(); } +}; + +}; // namespace std + +// CHECK-INIT-LIST-LABEL: _Z5Test4v: +std::map Test4() { // CHECK-INIT-LIST: File 0, [[@LINE]]:28 -> [[@LINE+3]]:2 = #0 + abort(); + return std::map{{0, 0}}; // CHECK-INIT-LIST-NEXT: [[@LINE]]:3 -> [[@LINE]]:36 = 0 +} + int main() { Test t; t.set(Test::A, 5.5); t.set(Test::T, 5.6); t.set(Test::G, 5.7); t.set(Test::C, 5.8); + Test4(); return 0; } Index: lib/Sema/TreeTransform.h === --- lib/Sema/TreeTransform.h +++ lib/Sema/TreeTransform.h @@ -2591,10 +2591,11 @@ ExprResult RebuildCXXFunctionalCastExpr(TypeSourceInfo *TInfo, SourceLocation LParenLoc, Expr *Sub, - SourceLocation RParenLoc) { + SourceLocation RParenLoc, + bool ListInitialization) { return getSema().BuildCXXTypeConstructExpr(TInfo, LParenLoc, - MultiExprArg(&Sub, 1), - RParenLoc); + MultiExprArg(&Sub, 1), RParenL
[PATCH] D41423: [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral.
dexonsmith added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:2026 + +if (C == 0) { NulCharacter = CurPtr-1; vsapsai wrote: > dexonsmith wrote: > > Should this check still be skipped (in an `else if` of the `C == '\\'` > > check)? > I agree it is behaviour change. `NulCharacter` is used to warn if you have > null character in the string and I think it is worth warning even if it is > preceded by the backslash. Therefore I think this check shouldn't be skipped > after `C == '\\'` check. In practice I don't know how you can create a file > with null character in its name and use it in inclusion directive. Can you add a test for the behaviour change then? Comment at: clang/unittests/Lex/LexerTest.cpp:477 TEST_F(LexerTest, AvoidPastEndOfStringDereference) { - std::vector LexedTokens = Lex(" // \\\n"); - EXPECT_TRUE(LexedTokens.empty()); + EXPECT_TRUE(Lex(" // \\\n").empty()); + // rdar://problem/35572754 vsapsai wrote: > dexonsmith wrote: > > To minimize the diff, please separate this change into an NFC commit ahead > > of time. > I plan to nominate this fix for inclusion in 6.0.0 release and having one > commit will be easier. It is not necessary to include NFC change in 6.0.0 > release but it creates discrepancy that increases a chance of merge conflicts. > > But I don't have strong opinion, just pointing out potential downsides with > merging the change to other branches. I think it's worth separating the NFC changes from behaviour changes, even if it means having to cherry-pick extra patches. https://reviews.llvm.org/D41423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41923: [WebAssembly] Remove `-allow-undefined-file wasm.syms` from linker args
sbc100 created this revision. Herald added subscribers: cfe-commits, sunfish, aheejin, jgravelle-google, dschuff, jfb. See: https://github.com/WebAssembly/tool-conventions/issues/35 Repository: rC Clang https://reviews.llvm.org/D41923 Files: lib/Driver/ToolChains/WebAssembly.cpp Index: lib/Driver/ToolChains/WebAssembly.cpp === --- lib/Driver/ToolChains/WebAssembly.cpp +++ lib/Driver/ToolChains/WebAssembly.cpp @@ -62,8 +62,6 @@ if (Args.hasArg(options::OPT_pthread)) CmdArgs.push_back("-lpthread"); -CmdArgs.push_back("-allow-undefined-file"); -CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("wasm.syms"))); CmdArgs.push_back("-lc"); AddRunTimeLibs(ToolChain, ToolChain.getDriver(), CmdArgs, Args); } Index: lib/Driver/ToolChains/WebAssembly.cpp === --- lib/Driver/ToolChains/WebAssembly.cpp +++ lib/Driver/ToolChains/WebAssembly.cpp @@ -62,8 +62,6 @@ if (Args.hasArg(options::OPT_pthread)) CmdArgs.push_back("-lpthread"); -CmdArgs.push_back("-allow-undefined-file"); -CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("wasm.syms"))); CmdArgs.push_back("-lc"); AddRunTimeLibs(ToolChain, ToolChain.getDriver(), CmdArgs, Args); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41923: [WebAssembly] Remove `-allow-undefined-file wasm.syms` from linker args
sbc100 updated this revision to Diff 129352. sbc100 added a comment. update tests Repository: rC Clang https://reviews.llvm.org/D41923 Files: lib/Driver/ToolChains/WebAssembly.cpp test/Driver/wasm-toolchain.c Index: test/Driver/wasm-toolchain.c === --- test/Driver/wasm-toolchain.c +++ test/Driver/wasm-toolchain.c @@ -27,10 +27,10 @@ // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK %s // LINK: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]" -// LINK: lld{{.*}}" "-flavor" "wasm" "-L/foo/lib" "crt1.o" "[[temp]]" "-allow-undefined-file" "wasm.syms" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out" +// LINK: lld{{.*}}" "-flavor" "wasm" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out" // A basic C link command-line with optimization. // RUN: %clang -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK_OPT %s // LINK_OPT: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]" -// LINK_OPT: lld{{.*}}" "-flavor" "wasm" "-L/foo/lib" "crt1.o" "[[temp]]" "-allow-undefined-file" "wasm.syms" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out" +// LINK_OPT: lld{{.*}}" "-flavor" "wasm" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out" Index: lib/Driver/ToolChains/WebAssembly.cpp === --- lib/Driver/ToolChains/WebAssembly.cpp +++ lib/Driver/ToolChains/WebAssembly.cpp @@ -62,8 +62,6 @@ if (Args.hasArg(options::OPT_pthread)) CmdArgs.push_back("-lpthread"); -CmdArgs.push_back("-allow-undefined-file"); -CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("wasm.syms"))); CmdArgs.push_back("-lc"); AddRunTimeLibs(ToolChain, ToolChain.getDriver(), CmdArgs, Args); } Index: test/Driver/wasm-toolchain.c === --- test/Driver/wasm-toolchain.c +++ test/Driver/wasm-toolchain.c @@ -27,10 +27,10 @@ // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK %s // LINK: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]" -// LINK: lld{{.*}}" "-flavor" "wasm" "-L/foo/lib" "crt1.o" "[[temp]]" "-allow-undefined-file" "wasm.syms" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out" +// LINK: lld{{.*}}" "-flavor" "wasm" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out" // A basic C link command-line with optimization. // RUN: %clang -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK_OPT %s // LINK_OPT: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]" -// LINK_OPT: lld{{.*}}" "-flavor" "wasm" "-L/foo/lib" "crt1.o" "[[temp]]" "-allow-undefined-file" "wasm.syms" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out" +// LINK_OPT: lld{{.*}}" "-flavor" "wasm" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out" Index: lib/Driver/ToolChains/WebAssembly.cpp === --- lib/Driver/ToolChains/WebAssembly.cpp +++ lib/Driver/ToolChains/WebAssembly.cpp @@ -62,8 +62,6 @@ if (Args.hasArg(options::OPT_pthread)) CmdArgs.push_back("-lpthread"); -CmdArgs.push_back("-allow-undefined-file"); -CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("wasm.syms"))); CmdArgs.push_back("-lc"); AddRunTimeLibs(ToolChain, ToolChain.getDriver(), CmdArgs, Args); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322240 - [Lex] Inline a variable in test in preparation for more similar tests. NFC.
Author: vsapsai Date: Wed Jan 10 15:37:29 2018 New Revision: 322240 URL: http://llvm.org/viewvc/llvm-project?rev=322240&view=rev Log: [Lex] Inline a variable in test in preparation for more similar tests. NFC. Modified: cfe/trunk/unittests/Lex/LexerTest.cpp Modified: cfe/trunk/unittests/Lex/LexerTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/LexerTest.cpp?rev=322240&r1=322239&r2=322240&view=diff == --- cfe/trunk/unittests/Lex/LexerTest.cpp (original) +++ cfe/trunk/unittests/Lex/LexerTest.cpp Wed Jan 10 15:37:29 2018 @@ -474,8 +474,7 @@ TEST_F(LexerTest, GetBeginningOfTokenWit } TEST_F(LexerTest, AvoidPastEndOfStringDereference) { - std::vector LexedTokens = Lex(" // \\\n"); - EXPECT_TRUE(LexedTokens.empty()); + EXPECT_TRUE(Lex(" // \\\n").empty()); } TEST_F(LexerTest, StringizingRasString) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40560: [analyzer] Get construction into `operator new` running in simple cases.
NoQ updated this revision to Diff 129362. NoQ added a comment. Rename getters and setters for the new state trait (i.e. "push" -> "set", etc., because we no longer have a stack). Also make them static, so that it was potentially possible to access them from elsewhere, eg. from `CallEvent`, if deemed necessary. https://reviews.llvm.org/D40560 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/ExprEngine.cpp lib/StaticAnalyzer/Core/ExprEngineCXX.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp test/Analysis/inline.cpp test/Analysis/new-ctor-conservative.cpp test/Analysis/new-ctor-inlined.cpp test/Analysis/new-ctor-recursive.cpp Index: test/Analysis/new-ctor-recursive.cpp === --- /dev/null +++ test/Analysis/new-ctor-recursive.cpp @@ -0,0 +1,118 @@ +// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s + +void clang_analyzer_eval(bool); +void clang_analyzer_dump(int); + +typedef __typeof__(sizeof(int)) size_t; + +void *conjure(); +void exit(int); + +struct S; + +S *global_s; + +// Recursive operator kinda placement new. +void *operator new(size_t size, S *place); + +enum class ConstructionKind : char { + Garbage, + Recursive +}; + +struct S { +public: + int x; + S(): x(1) {} + S(int y): x(y) {} + + S(ConstructionKind k) { +switch (k) { +case ConstructionKind::Recursive: { // Call one more operator new 'r'ecursively. + S *s = new (nullptr) S(5); + x = s->x + 1; + global_s = s; + return; +} +case ConstructionKind::Garbage: { + // Leaves garbage in 'x'. +} +} + } + ~S() {} +}; + +// Do not try this at home! +void *operator new(size_t size, S *place) { + if (!place) +return new S(); + return place; +} + +void testThatCharConstructorIndeedYieldsGarbage() { + S *s = new S(ConstructionKind::Garbage); + clang_analyzer_eval(s->x == 0); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(s->x == 1); // expected-warning{{UNKNOWN}} + // FIXME: This should warn, but MallocChecker doesn't default-bind regions + // returned by standard operator new to garbage. + s->x += 1; // no-warning + delete s; +} + + +void testChainedOperatorNew() { + S *s; + // * Evaluate standard new. + // * Evaluate constructor S(3). + // * Bind value for standard new. + // * Evaluate our custom new. + // * Evaluate constructor S(Garbage). + // * Bind value for our custom new. + s = new (new S(3)) S(ConstructionKind::Garbage); + clang_analyzer_eval(s->x == 3); // expected-warning{{TRUE}} + // expected-warning@+9{{Potential leak of memory pointed to by 's'}} + + // * Evaluate standard new. + // * Evaluate constructor S(Garbage). + // * Bind value for standard new. + // * Evaluate our custom new. + // * Evaluate constructor S(4). + // * Bind value for our custom new. + s = new (new S(ConstructionKind::Garbage)) S(4); + clang_analyzer_eval(s->x == 4); // expected-warning{{TRUE}} + delete s; + + // -> Enter our custom new (nullptr). + // * Evaluate standard new. + // * Inline constructor S(). + // * Bind value for standard new. + // <- Exit our custom new (nullptr). + // * Evaluate constructor S(Garbage). + // * Bind value for our custom new. + s = new (nullptr) S(ConstructionKind::Garbage); + clang_analyzer_eval(s->x == 1); // expected-warning{{TRUE}} + delete s; + + // -> Enter our custom new (nullptr). + // * Evaluate standard new. + // * Inline constructor S(). + // * Bind value for standard new. + // <- Exit our custom new (nullptr). + // -> Enter constructor S(Recursive). + // -> Enter our custom new (nullptr). + // * Evaluate standard new. + // * Inline constructor S(). + // * Bind value for standard new. + // <- Exit our custom new (nullptr). + // * Evaluate constructor S(5). + // * Bind value for our custom new (nullptr). + // * Assign that value to global_s. + // <- Exit constructor S(Recursive). + // * Bind value for our custom new (nullptr). + global_s = nullptr; + s = new (nullptr) S(ConstructionKind::Recursive); + clang_analyzer_eval(global_s); // expected-warning{{TRUE}} + clang_analyzer_eval(global_s->x == 5); // expected-warning{{TRUE}} + clang_analyzer_eval(s->x == 6); // expected-warning{{TRUE}} + delete s; +} Index: test/Analysis/new-ctor-inlined.cpp === --- /dev/null +++ test/Analysis/new-ctor-inlined.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s + +void clang_analyzer_eval(bool); + +typedef __typeof__(sizeof(int)) size_t; + +void *conjure(); +void exit(int); + +void *operator new(size_t size) throw() { + void *x = conjure(); + if (x
[PATCH] D41830: [libc++] Fix PR#35780 - make std::experimental::filesystem::remove and remove_all return false or 0 if the file doesn't exist
mclow.lists added a comment. I like the fact that you've removed the extra test that you've added. However, I think that modifying the tests as you've done is more work than is needed. I *suspect* that all you need to do is to move a couple of test cases from the "calls which should fail" to the "calls that should succeed" list (and adjust for the fact that the calls return false even though they succeed) For `remove`, that would be `path("dne")` and `path("")` . Just FYI - I believe that 'dne' is Eric's shorthand for "does not exist". Something like this: TEST_CASE(basic_remove_test) { scoped_test_env env; const path dne = env.make_env_path("dne"); const path link = env.create_symlink(dne, "link"); const path nested_link = env.make_env_path("nested_link"); create_symlink(link, nested_link); const path testCases1[] = { env.create_file("file", 42), env.create_dir("empty_dir"), nested_link, link }; for (auto& p : testCases1) { std::error_code ec = std::make_error_code(std::errc::address_in_use); TEST_CHECK(remove(p, ec)); TEST_CHECK(!ec); TEST_CHECK(!exists(symlink_status(p))); } // This is https://bugs.llvm.org/show_bug.cgi?id=35780 const path testCases2[] = { env.make_env_path("dne"), "" }; for (auto& p : testCases2) { std::error_code ec = std::make_error_code(std::errc::address_in_use); TEST_CHECK(!remove(p, ec)); TEST_CHECK(!ec); TEST_CHECK(!exists(symlink_status(p))); } } https://reviews.llvm.org/D41830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41809: Clang counterpart change for fuzzer FreeBSD support
devnexen updated this revision to Diff 129364. https://reviews.llvm.org/D41809 Files: lib/Driver/ToolChains/FreeBSD.cpp Index: lib/Driver/ToolChains/FreeBSD.cpp === --- lib/Driver/ToolChains/FreeBSD.cpp +++ lib/Driver/ToolChains/FreeBSD.cpp @@ -392,6 +392,8 @@ } if (IsX86 || IsX86_64) { Res |= SanitizerKind::SafeStack; +Res |= SanitizerKind::Fuzzer; +Res |= SanitizerKind::FuzzerNoLink; } return Res; } Index: lib/Driver/ToolChains/FreeBSD.cpp === --- lib/Driver/ToolChains/FreeBSD.cpp +++ lib/Driver/ToolChains/FreeBSD.cpp @@ -392,6 +392,8 @@ } if (IsX86 || IsX86_64) { Res |= SanitizerKind::SafeStack; +Res |= SanitizerKind::Fuzzer; +Res |= SanitizerKind::FuzzerNoLink; } return Res; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41250: [analyzer] Model implied cast around operator new().
NoQ updated this revision to Diff 129365. NoQ added a comment. Rebase. Add a FIXME to bring back the cast in the conservative case. https://reviews.llvm.org/D41250 Files: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp test/Analysis/new-ctor-conservative.cpp test/Analysis/new-ctor-inlined.cpp Index: test/Analysis/new-ctor-inlined.cpp === --- test/Analysis/new-ctor-inlined.cpp +++ test/Analysis/new-ctor-inlined.cpp @@ -38,3 +38,18 @@ Sp *p = new Sp(new Sp(0)); clang_analyzer_eval(p->p->p == 0); // expected-warning{{TRUE}} } + +void checkNewPOD() { + int *i = new int; + clang_analyzer_eval(*i == 0); // expected-warning{{UNKNOWN}} + int *j = new int(); + clang_analyzer_eval(*j == 0); // expected-warning{{TRUE}} + int *k = new int(5); + clang_analyzer_eval(*k == 5); // expected-warning{{TRUE}} +} + +void checkTrivialCopy() { + S s; + S *t = new S(s); // no-crash + clang_analyzer_eval(t->x == 1); // expected-warning{{TRUE}} +} Index: test/Analysis/new-ctor-conservative.cpp === --- test/Analysis/new-ctor-conservative.cpp +++ test/Analysis/new-ctor-conservative.cpp @@ -12,3 +12,18 @@ S *s = new S; clang_analyzer_eval(s->x == 1); // expected-warning{{TRUE}} } + +void checkNewPOD() { + int *i = new int; + clang_analyzer_eval(*i == 0); // expected-warning{{UNKNOWN}} + int *j = new int(); + clang_analyzer_eval(*j == 0); // expected-warning{{TRUE}} + int *k = new int(5); + clang_analyzer_eval(*k == 5); // expected-warning{{TRUE}} +} + +void checkNewArray() { + S *s = new S[10]; + // FIXME: Should be true once we inline array constructors. + clang_analyzer_eval(s[0].x == 1); // expected-warning{{UNKNOWN}} +} Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp === --- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -277,12 +277,19 @@ state = state->BindExpr(CCE, callerCtx, ThisV); } -if (const CXXNewExpr *CNE = dyn_cast(CE)) { +if (const auto *CNE = dyn_cast(CE)) { // We are currently evaluating a CXXNewAllocator CFGElement. It takes a // while to reach the actual CXXNewExpr element from here, so keep the // region for later use. - state = setCXXNewAllocatorValue(state, CNE, calleeCtx->getParent(), - state->getSVal(CE, callerCtx)); + // Additionally cast the return value of the inlined operator new + // (which is of type 'void *') to the correct object type. + SVal AllocV = state->getSVal(CNE, callerCtx); + AllocV = svalBuilder.evalCast( + AllocV, CNE->getType(), + getContext().getPointerType(getContext().VoidTy)); + + state = + setCXXNewAllocatorValue(state, CNE, calleeCtx->getParent(), AllocV); } } Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp === --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -119,9 +119,17 @@ if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) { // TODO: Detect when the allocator returns a null pointer. // Constructor shall not be called in this case. - if (const MemRegion *MR = - getCXXNewAllocatorValue(State, CNE, LCtx).getAsRegion()) + if (const SubRegion *MR = dyn_cast_or_null( + getCXXNewAllocatorValue(State, CNE, LCtx).getAsRegion())) { +if (CNE->isArray()) { + // TODO: This code exists only to trigger the suppression for + // array constructors. In fact, we need to call the constructor + // for every allocated element, not just the first one! + return getStoreManager().GetElementZeroRegion( + MR, CNE->getType()->getPointeeType()); +} return MR; + } } } else if (auto *DS = dyn_cast(StmtElem->getStmt())) { if (const auto *Var = dyn_cast(DS->getSingleDecl())) { @@ -473,12 +481,22 @@ StmtNodeBuilder CallBldr(DstPreCall, DstPostCall, *currBldrCtx); for (auto I : DstPreCall) defaultEvalCall(CallBldr, I, *Call); + // If the call is inlined, DstPostCall will be empty and we bail out now. // Store return value of operator new() for future use, until the actual // CXXNewExpr gets processed. ExplodedNodeSet DstPostValue; StmtNodeBuilder ValueBldr(DstPostCall, DstPostValue, *currBldrCtx); for (auto I : DstPostCall) { +// FIXME: Because CNE serves as the "call site" for the allocator (due to +// lack of a better expression in the AST), the conjured return value symbol +// is going to be of the same type (C++ object pointer type). Technical
[PATCH] D41228: [ObjC] Enable __strong pointers in structs under ARC
ahatanak updated this revision to Diff 129359. ahatanak marked an inline comment as done. ahatanak added a comment. In EmitCallArg and EmitParmDecl, use the existing code for Microsoft C++ ABI to determine whether the argument should be destructed in the callee. Also, add a test case that checks the destructor for a non-trivial C struct is called on the EH path. https://reviews.llvm.org/D41228 Files: docs/LanguageExtensions.rst include/clang/AST/ASTContext.h include/clang/AST/Decl.h include/clang/AST/Type.h include/clang/Basic/DiagnosticSemaKinds.td lib/AST/ASTContext.cpp lib/AST/Decl.cpp lib/AST/Type.cpp lib/CodeGen/CGBlocks.cpp lib/CodeGen/CGCall.cpp lib/CodeGen/CGDecl.cpp lib/CodeGen/CGDeclCXX.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGNonTrivialStruct.cpp lib/CodeGen/CMakeLists.txt lib/CodeGen/CodeGenFunction.h lib/Lex/PPMacroExpansion.cpp lib/Sema/JumpDiagnostics.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaExpr.cpp test/ARCMT/checking.m test/CodeGenObjC/nontrivial-c-struct-exception.m test/CodeGenObjC/strong-in-c-struct.m test/Lexer/has_feature_objc_arc.m test/SemaObjC/arc-decls.m test/SemaObjC/arc-system-header.m test/SemaObjC/strong-in-c-struct.m Index: test/SemaObjC/strong-in-c-struct.m === --- /dev/null +++ test/SemaObjC/strong-in-c-struct.m @@ -0,0 +1,56 @@ +// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -fblocks -fobjc-runtime=ios-11.0 -fsyntax-only -verify %s + +typedef struct { + id a; +} Strong; + +void callee_variadic(const char *, ...); + +void test_variadic(void) { + Strong t; + callee_variadic("s", t); // expected-error {{cannot pass non-trivial C object of type 'Strong' by value to variadic function}} +} + +void test_jump0(int cond) { + switch (cond) { + case 0: +; +Strong x; // expected-note {{jump bypasses initialization of variable of non-trivial C struct type}} +break; + case 1: // expected-error {{cannot jump from switch statement to this case label}} +x.a = 0; +break; + } +} + +void test_jump1(void) { + static void *ips[] = { &&L0 }; +L0: // expected-note {{possible target of indirect goto}} + ; + Strong x; // expected-note {{jump exits scope of variable with non-trivial destructor}} + goto *ips; // expected-error {{cannot jump}} +} + +typedef void (^BlockTy)(void); +void func(BlockTy); +void func2(Strong); + +void test_block_scope0(int cond) { + Strong x; // expected-note {{jump enters lifetime of block which captures a C struct that is non-trivial to destroy}} + switch (cond) { + case 0: +func(^{ func2(x); }); +break; + default: // expected-error {{cannot jump from switch statement to this case label}} +break; + } +} + +void test_block_scope1(void) { + static void *ips[] = { &&L0 }; +L0: // expected-note {{possible target of indirect goto}} + ; + Strong x; // expected-note {{jump exits scope of variable with non-trivial destructor}} expected-note {{jump exits lifetime of block which captures a C struct that is non-trivial to destroy}} + func(^{ func2(x); }); + goto *ips; // expected-error {{cannot jump}} +} Index: test/SemaObjC/arc-system-header.m === --- test/SemaObjC/arc-system-header.m +++ test/SemaObjC/arc-system-header.m @@ -23,8 +23,7 @@ } void test5(struct Test5 *p) { - p->field = 0; // expected-error {{'field' is unavailable in ARC}} -// expected-note@arc-system-header.h:25 {{field has non-trivial ownership qualification}} + p->field = 0; } id test6() { @@ -49,8 +48,7 @@ extern void doSomething(Test9 arg); void test9() { -Test9 foo2 = {0, 0}; // expected-error {{'field' is unavailable in ARC}} - // expected-note@arc-system-header.h:56 {{field has non-trivial ownership qualification}} +Test9 foo2 = {0, 0}; doSomething(foo2); } #endif Index: test/SemaObjC/arc-decls.m === --- test/SemaObjC/arc-decls.m +++ test/SemaObjC/arc-decls.m @@ -3,7 +3,7 @@ // rdar://8843524 struct A { -id x; // expected-error {{ARC forbids Objective-C objects in struct}} +id x; }; union u { @@ -13,7 +13,7 @@ @interface I { struct A a; struct B { -id y[10][20]; // expected-error {{ARC forbids Objective-C objects in struct}} +id y[10][20]; id z; } b; @@ -23,7 +23,7 @@ // rdar://10260525 struct r10260525 { - id (^block) (); // expected-error {{ARC forbids blocks in struct}} + id (^block) (); }; struct S { Index: test/Lexer/has_feature_objc_arc.m === --- test/Lexer/has_feature_objc_arc.m +++ test/Lexer/has_feature_objc_arc.m @@ -13,8 +13,16 @@ void no_objc_arc_weak_feature(); #endif +#if __has_feature(objc_arc_fields) +void has_objc_arc_fields(); +#else +void no_objc_arc_fields(); +#endif + // CHECK-ARC: v
[PATCH] D41792: [AArch64] Add ARMv8.2-A FP16 scalar intrinsics
az updated this revision to Diff 129360. https://reviews.llvm.org/D41792 Files: clang/include/clang/Basic/BuiltinsNEON.def clang/include/clang/Basic/CMakeLists.txt clang/include/clang/Basic/arm_fp16.td clang/include/clang/Basic/arm_neon.td clang/include/clang/Basic/arm_neon_incl.td clang/lib/Basic/Targets/AArch64.cpp clang/lib/CodeGen/CGBuiltin.cpp clang/lib/Headers/CMakeLists.txt clang/lib/Headers/module.modulemap clang/lib/Sema/SemaChecking.cpp clang/test/CodeGen/aarch64-v8.2a-fp16-intrinsics.c clang/utils/TableGen/NeonEmitter.cpp clang/utils/TableGen/TableGen.cpp clang/utils/TableGen/TableGenBackends.h llvm/include/llvm/IR/IntrinsicsAArch64.td Index: llvm/include/llvm/IR/IntrinsicsAArch64.td === --- llvm/include/llvm/IR/IntrinsicsAArch64.td +++ llvm/include/llvm/IR/IntrinsicsAArch64.td @@ -146,6 +146,9 @@ class AdvSIMD_CvtFPToFx_Intrinsic : Intrinsic<[llvm_anyint_ty], [llvm_anyfloat_ty, llvm_i32_ty], [IntrNoMem]>; + + class AdvSIMD_1Arg_Intrinsic +: Intrinsic<[llvm_any_ty], [LLVMMatchType<0>], [IntrNoMem]>; } // Arithmetic ops @@ -244,7 +247,7 @@ // Vector Max def int_aarch64_neon_smax : AdvSIMD_2VectorArg_Intrinsic; def int_aarch64_neon_umax : AdvSIMD_2VectorArg_Intrinsic; - def int_aarch64_neon_fmax : AdvSIMD_2VectorArg_Intrinsic; + def int_aarch64_neon_fmax : AdvSIMD_2FloatArg_Intrinsic; def int_aarch64_neon_fmaxnmp : AdvSIMD_2VectorArg_Intrinsic; // Vector Max Across Lanes @@ -256,7 +259,7 @@ // Vector Min def int_aarch64_neon_smin : AdvSIMD_2VectorArg_Intrinsic; def int_aarch64_neon_umin : AdvSIMD_2VectorArg_Intrinsic; - def int_aarch64_neon_fmin : AdvSIMD_2VectorArg_Intrinsic; + def int_aarch64_neon_fmin : AdvSIMD_2FloatArg_Intrinsic; def int_aarch64_neon_fminnmp : AdvSIMD_2VectorArg_Intrinsic; // Vector Min/Max Number @@ -354,7 +357,7 @@ def int_aarch64_neon_sqxtun : AdvSIMD_1VectorArg_Narrow_Intrinsic; // Vector Absolute Value - def int_aarch64_neon_abs : AdvSIMD_1IntArg_Intrinsic; + def int_aarch64_neon_abs : AdvSIMD_1Arg_Intrinsic; // Vector Saturating Absolute Value def int_aarch64_neon_sqabs : AdvSIMD_1IntArg_Intrinsic; Index: clang/utils/TableGen/TableGenBackends.h === --- clang/utils/TableGen/TableGenBackends.h +++ clang/utils/TableGen/TableGenBackends.h @@ -65,6 +65,7 @@ void EmitClangCommentCommandList(RecordKeeper &Records, raw_ostream &OS); void EmitNeon(RecordKeeper &Records, raw_ostream &OS); +void EmitFP16(RecordKeeper &Records, raw_ostream &OS); void EmitNeonSema(RecordKeeper &Records, raw_ostream &OS); void EmitNeonTest(RecordKeeper &Records, raw_ostream &OS); void EmitNeon2(RecordKeeper &Records, raw_ostream &OS); Index: clang/utils/TableGen/TableGen.cpp === --- clang/utils/TableGen/TableGen.cpp +++ clang/utils/TableGen/TableGen.cpp @@ -52,6 +52,7 @@ GenClangCommentCommandInfo, GenClangCommentCommandList, GenArmNeon, + GenArmFP16, GenArmNeonSema, GenArmNeonTest, GenAttrDocs, @@ -139,6 +140,7 @@ "Generate list of commands that are used in " "documentation comments"), clEnumValN(GenArmNeon, "gen-arm-neon", "Generate arm_neon.h for clang"), +clEnumValN(GenArmFP16, "gen-arm-fp16", "Generate arm_fp16.h for clang"), clEnumValN(GenArmNeonSema, "gen-arm-neon-sema", "Generate ARM NEON sema support for clang"), clEnumValN(GenArmNeonTest, "gen-arm-neon-test", @@ -250,6 +252,9 @@ case GenArmNeon: EmitNeon(Records, OS); break; + case GenArmFP16: +EmitFP16(Records, OS); +break; case GenArmNeonSema: EmitNeonSema(Records, OS); break; Index: clang/utils/TableGen/NeonEmitter.cpp === --- clang/utils/TableGen/NeonEmitter.cpp +++ clang/utils/TableGen/NeonEmitter.cpp @@ -552,7 +552,11 @@ // run - Emit arm_neon.h.inc void run(raw_ostream &o); + // runFP16 - Emit arm_fp16.h.inc + void runFP16(raw_ostream &o); + // runHeader - Emit all the __builtin prototypes used in arm_neon.h + // and arm_fp16.h void runHeader(raw_ostream &o); // runTests - Emit tests for all the Neon intrinsics. @@ -852,6 +856,35 @@ NumVectors = 0; Float = true; break; + case 'Y': +Bitwidth = ElementBitwidth = 16; +NumVectors = 0; +Float = true; +break; + case 'I': +Bitwidth = ElementBitwidth = 32; +NumVectors = 0; +Float = false; +Signed = true; +break; + case 'L': +Bitwidth = ElementBitwidth = 64; +NumVectors = 0; +Float = false; +Signed = true; +break; + case 'U': +Bitwidth = ElementBitwidth = 32; +NumVectors = 0; +Float = false; +Signed = false; +break; + case 'O': +Bitwidth = ElementBi
r322242 - Revert "[Driver] Update default sanitizer blacklist location"
Author: phosek Date: Wed Jan 10 16:12:02 2018 New Revision: 322242 URL: http://llvm.org/viewvc/llvm-project?rev=322242&view=rev Log: Revert "[Driver] Update default sanitizer blacklist location" This reverts commit r322233: this is breaking dfsan tests. Added: cfe/trunk/test/Driver/Inputs/resource_dir/asan_blacklist.txt - copied, changed from r322240, cfe/trunk/test/Driver/Inputs/resource_dir/share/asan_blacklist.txt cfe/trunk/test/Driver/Inputs/resource_dir/hwasan_blacklist.txt - copied, changed from r322240, cfe/trunk/test/Driver/Inputs/resource_dir/share/hwasan_blacklist.txt cfe/trunk/test/Driver/Inputs/resource_dir/ubsan_blacklist.txt - copied, changed from r322240, cfe/trunk/test/Driver/Inputs/resource_dir/share/ubsan_blacklist.txt cfe/trunk/test/Driver/Inputs/resource_dir/vtables_blacklist.txt - copied, changed from r322240, cfe/trunk/test/Driver/Inputs/resource_dir/share/vtables_blacklist.txt Removed: cfe/trunk/test/Driver/Inputs/resource_dir/share/asan_blacklist.txt cfe/trunk/test/Driver/Inputs/resource_dir/share/hwasan_blacklist.txt cfe/trunk/test/Driver/Inputs/resource_dir/share/ubsan_blacklist.txt cfe/trunk/test/Driver/Inputs/resource_dir/share/vtables_blacklist.txt Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=322242&r1=322241&r2=322242&view=diff == --- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original) +++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Wed Jan 10 16:12:02 2018 @@ -112,7 +112,7 @@ static bool getDefaultBlacklist(const Dr if (BlacklistFile) { clang::SmallString<64> Path(D.ResourceDir); -llvm::sys::path::append(Path, "share", BlacklistFile); +llvm::sys::path::append(Path, BlacklistFile); BLPath = Path.str(); return true; } Copied: cfe/trunk/test/Driver/Inputs/resource_dir/asan_blacklist.txt (from r322240, cfe/trunk/test/Driver/Inputs/resource_dir/share/asan_blacklist.txt) URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir/asan_blacklist.txt?p2=cfe/trunk/test/Driver/Inputs/resource_dir/asan_blacklist.txt&p1=cfe/trunk/test/Driver/Inputs/resource_dir/share/asan_blacklist.txt&r1=322240&r2=322242&rev=322242&view=diff == (empty) Copied: cfe/trunk/test/Driver/Inputs/resource_dir/hwasan_blacklist.txt (from r322240, cfe/trunk/test/Driver/Inputs/resource_dir/share/hwasan_blacklist.txt) URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir/hwasan_blacklist.txt?p2=cfe/trunk/test/Driver/Inputs/resource_dir/hwasan_blacklist.txt&p1=cfe/trunk/test/Driver/Inputs/resource_dir/share/hwasan_blacklist.txt&r1=322240&r2=322242&rev=322242&view=diff == (empty) Removed: cfe/trunk/test/Driver/Inputs/resource_dir/share/asan_blacklist.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir/share/asan_blacklist.txt?rev=322241&view=auto == (empty) Removed: cfe/trunk/test/Driver/Inputs/resource_dir/share/hwasan_blacklist.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir/share/hwasan_blacklist.txt?rev=322241&view=auto == (empty) Removed: cfe/trunk/test/Driver/Inputs/resource_dir/share/ubsan_blacklist.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir/share/ubsan_blacklist.txt?rev=322241&view=auto == (empty) Removed: cfe/trunk/test/Driver/Inputs/resource_dir/share/vtables_blacklist.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir/share/vtables_blacklist.txt?rev=322241&view=auto == (empty) Copied: cfe/trunk/test/Driver/Inputs/resource_dir/ubsan_blacklist.txt (from r322240, cfe/trunk/test/Driver/Inputs/resource_dir/share/ubsan_blacklist.txt) URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir/ubsan_blacklist.txt?p2=cfe/trunk/test/Driver/Inputs/resource_dir/ubsan_blacklist.txt&p1=cfe/trunk/test/Driver/Inputs/resource_dir/share/ubsan_blacklist.txt&r1=322240&r2=322242&rev=322242&view=diff == (empty) Copied: cfe/trunk/test/Driver/Inputs/resource_dir/vtables_blacklist.txt (from r322240, cfe/trunk/test/Driver/Inputs/resource_dir/share/vtables_blacklist.txt) URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs
LLVM buildmaster will be updated and restarted tonight
Hello everyone, LLVM buildmaster will be updated and restarted after 6 PM Pacific time today. Thanks Galina ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41266: [analyzer] With c++-allocator-inlining, fix memory space for operator new pointers.
NoQ updated this revision to Diff 129367. NoQ added a comment. Rebase (fix minor conflict). https://reviews.llvm.org/D41266 Files: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp test/Analysis/NewDelete-checker-test.cpp test/Analysis/new-ctor-null.cpp Index: test/Analysis/new-ctor-null.cpp === --- /dev/null +++ test/Analysis/new-ctor-null.cpp @@ -0,0 +1,23 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s + +void clang_analyzer_eval(bool); + +typedef __typeof__(sizeof(int)) size_t; + +void *operator new(size_t size) throw() { + return nullptr; +} +void *operator new[](size_t size) throw() { + return nullptr; +} + +struct S { + int x; + S() : x(1) {} + ~S() {} +}; + +void testArrays() { + S *s = new S[10]; // no-crash + s[0].x = 2; // expected-warning{{Dereference of null pointer}} +} Index: test/Analysis/NewDelete-checker-test.cpp === --- test/Analysis/NewDelete-checker-test.cpp +++ test/Analysis/NewDelete-checker-test.cpp @@ -1,5 +1,7 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -fblocks -verify %s // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDeleteLeaks -DLEAKS -std=c++11 -fblocks -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -fblocks -analyzer-config c++-allocator-inlining=true -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDeleteLeaks -DLEAKS -std=c++11 -fblocks -analyzer-config c++-allocator-inlining=true -verify %s #include "Inputs/system-header-simulator-cxx.h" typedef __typeof__(sizeof(int)) size_t; Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp === --- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -569,7 +569,19 @@ QualType ResultTy = Call.getResultType(); SValBuilder &SVB = getSValBuilder(); unsigned Count = currBldrCtx->blockCount(); - SVal R = SVB.conjureSymbolVal(nullptr, E, LCtx, ResultTy, Count); + + // See if we need to conjure a heap pointer instead of + // a regular unknown pointer. + bool IsHeapPointer = false; + if (const auto *CNE = dyn_cast(E)) +if (CNE->getOperatorNew()->isReplaceableGlobalAllocationFunction()) { + // FIXME: Delegate this to evalCall in MallocChecker? + IsHeapPointer = true; +} + + SVal R = IsHeapPointer + ? SVB.getConjuredHeapSymbolVal(E, LCtx, Count) + : SVB.conjureSymbolVal(nullptr, E, LCtx, ResultTy, Count); return State->BindExpr(E, LCtx, R); } Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp === --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -479,8 +479,10 @@ ExplodedNodeSet DstPostCall; StmtNodeBuilder CallBldr(DstPreCall, DstPostCall, *currBldrCtx); - for (auto I : DstPreCall) + for (auto I : DstPreCall) { +// FIXME: Provide evalCall for checkers? defaultEvalCall(CallBldr, I, *Call); + } // If the call is inlined, DstPostCall will be empty and we bail out now. // Store return value of operator new() for future use, until the actual @@ -507,7 +509,6 @@ *Call, *this); } - void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred, ExplodedNodeSet &Dst) { // FIXME: Much of this should eventually migrate to CXXAllocatorCall. @@ -520,18 +521,8 @@ SVal symVal = UnknownVal(); FunctionDecl *FD = CNE->getOperatorNew(); - bool IsStandardGlobalOpNewFunction = false; - if (FD && !isa(FD) && !FD->isVariadic()) { -if (FD->getNumParams() == 2) { - QualType T = FD->getParamDecl(1)->getType(); - if (const IdentifierInfo *II = T.getBaseTypeIdentifier()) -// NoThrow placement new behaves as a standard new. -IsStandardGlobalOpNewFunction = II->getName().equals("nothrow_t"); -} -else - // Placement forms are considered non-standard. - IsStandardGlobalOpNewFunction = (FD->getNumParams() == 1); - } + bool IsStandardGlobalOpNewFunction = + FD->isReplaceableGlobalAllocationFunction(); ProgramStateRef State = Pred->getState(); @@ -587,9 +578,8 @@ if (CNE->isArray()) { // FIXME: allocating an array requires simulating the constructors. // For now, just return a symbolicated region. -if (!AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) { - const SubRegion *NewReg = - symVal.castAs().getRegionAs(); +if (const SubRegion *NewReg = +dyn_cast_or_null(symVal.getAsRegion())) { QualType ObjTy = CNE->getType(
[PATCH] D41406: [analyzer] Add a new checker callback, check::NewAllocator.
NoQ updated this revision to Diff 129369. NoQ added a comment. Rebase (getter rename). https://reviews.llvm.org/D41406 Files: include/clang/StaticAnalyzer/Core/Checker.h include/clang/StaticAnalyzer/Core/CheckerManager.h lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp lib/StaticAnalyzer/Checkers/MallocChecker.cpp lib/StaticAnalyzer/Core/CheckerManager.cpp lib/StaticAnalyzer/Core/ExprEngineCXX.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp test/Analysis/NewDelete-custom.cpp test/Analysis/new-ctor-malloc.cpp Index: test/Analysis/new-ctor-malloc.cpp === --- /dev/null +++ test/Analysis/new-ctor-malloc.cpp @@ -0,0 +1,18 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection,unix.Malloc -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s + +void clang_analyzer_eval(bool); + +typedef __typeof__(sizeof(int)) size_t; + +void *malloc(size_t size); + +void *operator new(size_t size) throw() { + void *x = malloc(size); + if (!x) +return nullptr; + return x; +} + +void checkNewAndConstructorInlining() { + int *s = new int; +} // expected-warning {{Potential leak of memory pointed to by 's'}} Index: test/Analysis/NewDelete-custom.cpp === --- test/Analysis/NewDelete-custom.cpp +++ test/Analysis/NewDelete-custom.cpp @@ -1,8 +1,10 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,unix.Malloc -std=c++11 -fblocks -verify %s -// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc -std=c++11 -DLEAKS -fblocks -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc -std=c++11 -DLEAKS=1 -fblocks -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,unix.Malloc -std=c++11 -analyzer-config c++-allocator-inlining=true -DALLOCATOR_INLINING=1 -fblocks -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc -std=c++11 -analyzer-config c++-allocator-inlining=true -DLEAKS=1 -DALLOCATOR_INLINING=1 -fblocks -verify %s #include "Inputs/system-header-simulator-cxx.h" -#ifndef LEAKS +#if !LEAKS // expected-no-diagnostics #endif @@ -26,7 +28,7 @@ C *c3 = ::new C; } -#ifdef LEAKS +#if LEAKS && !ALLOCATOR_INLINING // expected-warning@-2{{Potential leak of memory pointed to by 'c3'}} #endif @@ -37,7 +39,7 @@ void testNewExprArray() { int *p = new int[0]; } -#ifdef LEAKS +#if LEAKS && !ALLOCATOR_INLINING // expected-warning@-2{{Potential leak of memory pointed to by 'p'}} #endif @@ -50,30 +52,30 @@ void testNewExpr() { int *p = new int; } -#ifdef LEAKS +#if LEAKS && !ALLOCATOR_INLINING // expected-warning@-2{{Potential leak of memory pointed to by 'p'}} #endif //- Custom NoThrow placement operators void testOpNewNoThrow() { void *p = operator new(0, std::nothrow); } -#ifdef LEAKS +#if LEAKS // expected-warning@-2{{Potential leak of memory pointed to by 'p'}} #endif void testNewExprNoThrow() { int *p = new(std::nothrow) int; } -#ifdef LEAKS +#if LEAKS // expected-warning@-2{{Potential leak of memory pointed to by 'p'}} #endif //- Custom placement operators void testOpNewPlacement() { void *p = operator new(0, 0.1); // no warn -} +} void testNewExprPlacement() { int *p = new(0.1) int; // no warn Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp === --- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -346,10 +346,23 @@ CallEventRef<> UpdatedCall = Call.cloneWithState(CEEState); ExplodedNodeSet DstPostCall; -getCheckerManager().runCheckersForPostCall(DstPostCall, CEENode, - *UpdatedCall, *this, - /*WasInlined=*/true); - +if (const CXXNewExpr *CNE = dyn_cast_or_null(CE)) { + ExplodedNodeSet DstPostPostCallCallback; + getCheckerManager().runCheckersForPostCall(DstPostPostCallCallback, + CEENode, *UpdatedCall, *this, + /*WasInlined=*/true); + for (auto I : DstPostPostCallCallback) { +getCheckerManager().runCheckersForNewAllocator( +CNE, getCXXNewAllocatorValue(I->getState(), CNE, + calleeCtx->getParent()), +DstPostCall, I, *this, +/*WasInlined=*/true); + } +} else { + getCheckerManager().runCheckersForPostCall(DstPostCall, CEENode, + *UpdatedCall, *this, + /*WasInlined=*/true); +} ExplodedNodeSet Ds
[PATCH] D41923: [WebAssembly] Remove `-allow-undefined-file wasm.syms` from linker args
dschuff accepted this revision. dschuff added a comment. This revision is now accepted and ready to land. The code LGTM. If there are objections to the scheme overall they can go in tool-conventions. Repository: rC Clang https://reviews.llvm.org/D41923 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41311: [CodeGen] Fix crash when a function taking transparent union is redeclared.
vsapsai added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2321 + ArgI.getCoerceToType() == + ConvertType(isPromoted ? Ty : Arg->getType()) && ArgI.getDirectOffset() == 0) { ahatanak wrote: > Maybe a comment explaining why different types are passed depending on the > value of isPromoted? My reasoning was that for promoted arguments `emitArgumentDemotion` is responsible for bridging gaps between `Ty` and `Arg->getType()`, so it should be enough for `ArgI.getCoerceToType()` and `Ty` to match. But this explanation is woefully unsuitable for the comment. After some more thinking I start doubting if some other s/Ty/Arg->getType()/ changes are entirely correct. Comment at: clang/lib/CodeGen/CGCall.cpp:2469 if (isPromoted) V = emitArgumentDemotion(*this, Arg, V); ArgVals.push_back(ParamValue::forDirect(V)); For example, here. If `V` is of type `Arg->getType()`, will `emitArgumentDemotion` do anything? Because there you have ``` if (value->getType() == varType) return value; ``` I'll keep digging and if meanwhile anybody has comments on this situation, I'll be glad to hear them. https://reviews.llvm.org/D41311 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41830: [libc++] Fix PR#35780 - make std::experimental::filesystem::remove and remove_all return false or 0 if the file doesn't exist
mclow.lists added inline comments. Comment at: test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp:67 non_empty_dir, -file_in_bad_dir, +// file_in_bad_dir, // produces "St13exception_ptruncaught_exceptions not yet implemented" }; TyanNN wrote: > This error is quite strange: it is not inherited from std::exception, one can > only really find our the error with std::current_exception, and it happens > when you use the path in fs::remove and fs::remove_all. Any idea about this? > @mclow.lists I'm not seeing this error on my system (nor are any of the bots reporting it AFAIK). That looks like "`uncaught_exceptions` is not yet implemented", and that comes from the ABI library. What are you using for an ABI library? Is it libc++abi? https://reviews.llvm.org/D41830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41820: [coroutines] Pass coro func args to promise ctor
GorNishanov requested changes to this revision. GorNishanov added a comment. This revision now requires changes to proceed. Thank you for doing this change! Comment at: lib/Sema/SemaCoroutine.cpp:475 +// Create a static_cast\(expr). +static Expr *castForMoving(Sema &S, Expr *E, QualType T = QualType()) { I would keep this block of functions in their original place. (Or move them here as a separate NFC) commit. Easier to review what was changed and what was simply moved. Comment at: lib/Sema/SemaCoroutine.cpp:570 +auto RefExpr = ExprEmpty(); +auto Moves = ScopeInfo->CoroutineParameterMoves; +if (Moves.find(PD) != Moves.end()) { This creates a copy of the CoroutineParameterMoves map in every iteration of the loop. It seems like an intent is just to create a shorter alias "Moves" to it to refer later. I suggest: 1) Make Moves a reference to the map: `auto &Moves = ...` 2) Move it out of the loop Comment at: lib/Sema/SemaCoroutine.cpp:572 +if (Moves.find(PD) != Moves.end()) { + // If a reference to the function parameter exists in the coroutine + // frame, use that reference. Instead of doing lookup twice, once, using `find`, another, using `operator[]`, you can just say: ``` auto EntryIter = Moves.find(PD); if (EntryIter != Moves.end()) { auto *VD = cast(cast(EntryIter->second)->getSingleDecl()); ... ``` Comment at: lib/Sema/SemaCoroutine.cpp:574 + // frame, use that reference. + auto *VD = cast(cast(Moves[PD])->getSingleDecl()); + RefExpr = BuildDeclRefExpr(VD, VD->getType(), ExprValueKind::VK_LValue, This VD hides outer VD referring to the promise. I would rename one of them to some other name. Comment at: lib/Sema/SemaCoroutine.cpp:619 FD->addDecl(VD); assert(!VD->isInvalidDecl()); return VD; I believe this assert needs to be removed. We can get an invalid decl if we failed to find an appropriate constructor. (Couple of negative tests in SemaCXX/coroutines.cpp would help to flush those cases out) Comment at: test/CodeGenCoroutines/coro-alloc.cpp:196 } + +struct promise_matching_constructor {}; I would move this test to coro-params.cpp, as it is closer to parameter moves than to allocations. I would also add a negative test or two to SemaCXX/coroutines.cpp to verify that we emit sane errors when something goes wrong with promise constructor and parameter copies. Repository: rC Clang https://reviews.llvm.org/D41820 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41792: [AArch64] Add ARMv8.2-A FP16 scalar intrinsics
az marked 8 inline comments as done. az added inline comments. Comment at: clang/include/clang/Basic/arm_fp16.td:58 +class IInst : Inst {} + +// ARMv8.2-A FP16 intrinsics. SjoerdMeijer wrote: > There's a little bit of duplication here: the definitions above are the same > as in arm_neon.td. Would it be easy to share this, with e.g. an include? The duplication is small compared to the overall infrastructure/data structure needed to automatically generate the intrinsics. There are 3 ways to do this: 1) copy only the needed data structure in arm_fp16.td (this is what was done in original review) 2) put all data structure in a newly created file and include it in arm_neon.td and arm_fp16.td (done here). 3) put only the duplication in a new file and include it. I did not go for this one given that we create a new file for the only purpose of avoiding a small duplication but I am fine of going with 3 too. Note that some of the duplicated structure in the original arm_fp16.td was a stripped down version of the copied one. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4102 NEONMAP1(vuqadds_s32, aarch64_neon_suqadd, Add1ArgType), + // FP16 scalar intrinisics go here. + NEONMAP1(vabdh_f16, aarch64_sisd_fabd, Add1ArgType), SjoerdMeijer wrote: > Looks like a few intrinsic descriptions are missing here. For example, the > first 2-operand intrinsic vaddh_f16 is missing, but there are also more. Is > this intentional, or might they have slipped through the cracks (or am I > missing something)? I agree that this is confusing. For the intrinsics listed in this table, code generation happens in a generic way based on the info in the table. The ones not listed in this table are addressed in a more specific way below in a the function called EmitAArch64BuiltinExpr. While I do not like how few things were implemented in generating the intrinsics, I am in general following the approach taken for arm_neon instead of introducing a new approach. Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:149 [IntrNoMem]>; + + class AdvSIMD_1Arg_Intrinsic SjoerdMeijer wrote: > This and the other changes in this file are changes to LLVM. Do we need these > changes for this patch? It doesn't look like it. Some tests in aarch64-v8.2a-fp16-intrinsics.c will fail for me without these changes. In clang/lib/CodeGen/BackendUtil.cpp, there is code there that includes llvm files and header files. It fails there if I do not fix IntrinsicAArch64.td. If you know of a better way to test differently without the need for llvm, then let me know. For example, if I remove the option flag -S from testing (i.e from aarch64-v8.2a-fp16-intrinsics.c), then there is no need to llvm but I won't be able to compare results. https://reviews.llvm.org/D41792 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Buildbot numbers for the week of 12/24/2017 - 12/30/2017
Hello everyone, Below are some buildbot numbers for the week of 12/24/2017 - 12/30/2017. Please see the same data in attached csv files: The longest time each builder was red during the week; "Status change ratio" by active builder (percent of builds that changed the builder status from greed to red or from red to green); Count of commits by project; Number of completed builds, failed builds and average build time for successful builds per active builder; Average waiting time for a revision to get build result per active builder (response time). Thanks Galina The longest time each builder was red during the week: buildername | was_red ---+-- clang-x86-windows-msvc2015| 103:36:40 clang-cmake-armv7-a15-selfhost-neon | 95:16:57 lldb-windows7-android | 93:57:11 clang-cmake-thumbv7-a15-full-sh | 92:32:18 clang-cmake-armv7-a15-selfhost| 55:29:46 libcxx-libcxxabi-libunwind-aarch64-linux-noexceptions | 14:56:55 libcxx-libcxxabi-libunwind-aarch64-linux | 08:58:28 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast| 07:20:04 clang-cmake-armv7-a15-full| 04:29:44 clang-lld-x86_64-2stage | 04:16:14 clang-with-lto-ubuntu | 04:14:12 clang-cmake-thumbv7-a15 | 03:53:43 clang-cmake-armv7-a15 | 03:53:27 sanitizer-x86_64-linux-fuzzer | 03:52:18 clang-hexagon-elf | 03:42:51 clang-cmake-aarch64-full | 03:12:54 clang-with-thin-lto-ubuntu| 02:46:44 sanitizer-ppc64le-linux | 02:22:52 clang-ppc64le-linux-lnt | 02:09:42 lldb-amd64-ninja-netbsd8 | 02:08:35 clang-ppc64le-linux-multistage| 01:53:50 clang-cuda-build | 01:52:26 lldb-x86_64-ubuntu-14.04-buildserver | 01:51:14 llvm-clang-x86_64-expensive-checks-win| 01:50:37 reverse-iteration | 01:48:03 sanitizer-x86_64-linux-fast | 01:47:45 sanitizer-x86_64-linux-bootstrap-msan | 01:46:53 clang-x86_64-linux-selfhost-modules-2 | 01:45:11 clang-ppc64le-linux | 01:43:18 clang-ppc64be-linux | 01:42:10 clang-atom-d525-fedora-rel| 01:41:27 clang-ppc64be-linux-lnt | 01:41:22 clang-bpf-build | 01:39:43 sanitizer-x86_64-linux-bootstrap-ubsan| 01:36:30 lldb-x86_64-ubuntu-14.04-cmake| 01:33:59 sanitizer-x86_64-linux-bootstrap | 01:33:08 clang-s390x-linux-lnt | 01:31:42 clang-x86_64-linux-selfhost-modules | 01:31:11 clang-cmake-x86_64-avx2-linux | 01:30:19 clang-ppc64be-linux-multistage| 01:21:14 lldb-x86-windows-msvc2015 | 01:17:07 lldb-amd64-ninja-freebsd11| 01:16:12 clang-s390x-linux | 01:12:29 libcxx-libcxxabi-libunwind-x86_64-linux-debian| 01:01:21 clang-cmake-aarch64-lld | 00:59:42 lldb-x86_64-ubuntu-14.04-android | 00:57:04 sanitizer-x86_64-linux| 00:53:59 clang-cmake-aarch64-global-isel | 00:51:29 clang-cmake-aarch64-quick | 00:51:21 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast | 00:47:55 lld-x86_64-win7 | 00:23:39 lld-x86_64-freebsd| 00:23:27 lld-x86_64-darwin13 | 00:23:27 clang-cmake-x86_64-sde-avx512-linux | 00:17:57 (54 rows) "Status change ratio" by active builder (percent of builds that changed the builder status from greed to red or from red to green): buildername | builds | changes | status_change_ratio -+-- --+-+ libcxx-libcxxabi-libunwind-aarch64-linux| 10 | 4 |40.0 libcxx-libcxxabi-libunwind-aarch64-linux-noexceptions | 10 | 4 |40.0 clang-with-lto-ubuntu | 64 | 11 |17.2 libcxx-libcxxabi-libunwind-x86_64-l
Buildbot numbers for the last week of 12/31/2017 - 1/06/2018
Hello everyone, Below are some buildbot numbers for the last week of 12/31/2017 - 1/06/2018. Please see the same data in attached csv files: The longest time each builder was red during the week; "Status change ratio" by active builder (percent of builds that changed the builder status from greed to red or from red to green); Count of commits by project; Number of completed builds, failed builds and average build time for successful builds per active builder; Average waiting time for a revision to get build result per active builder (response time). Thanks Galina The longest time each builder was red during the week: buildername | was_red ---+- clang-x86_64-linux-selfhost-modules | 47:47:01 clang-x86_64-linux-selfhost-modules-2 | 47:08:52 clang-lld-x86_64-2stage | 27:15:29 clang-with-lto-ubuntu | 24:59:35 clang-with-thin-lto-ubuntu| 24:39:00 llvm-clang-x86_64-expensive-checks-win| 20:58:27 llvm-hexagon-elf | 15:56:13 clang-cmake-aarch64-quick | 15:19:04 clang-cmake-aarch64-global-isel | 15:18:43 clang-cmake-armv7-a15 | 15:11:29 clang-hexagon-elf | 15:01:57 clang-cmake-thumbv7-a15 | 14:55:18 clang-cmake-aarch64-full | 13:14:53 clang-cmake-thumbv7-a15-full-sh | 12:19:28 sanitizer-x86_64-linux-android| 12:08:47 openmp-clang-x86_64-linux-debian | 10:38:17 clang-cmake-armv7-a15-selfhost| 10:09:03 clang-cmake-armv7-a15-selfhost-neon | 09:56:47 libcxx-libcxxabi-libunwind-x86_64-linux-debian| 08:35:35 clang-s390x-linux-multistage | 08:23:50 clang-cmake-armv7-a15-full| 08:01:52 llvm-mips-linux | 08:01:37 clang-s390x-linux | 06:35:19 clang-s390x-linux-lnt | 06:33:46 clang-x86-windows-msvc2015| 06:31:54 sanitizer-x86_64-linux-bootstrap | 06:26:10 sanitizer-x86_64-linux-fast | 05:46:00 clang-ppc64le-linux-lnt | 05:38:56 clang-ppc64be-linux-multistage| 05:35:37 lldb-windows7-android | 05:34:33 clang-ppc64be-linux-lnt | 04:25:16 libcxx-libcxxabi-libunwind-aarch64-linux-noexceptions | 04:24:56 lldb-amd64-ninja-netbsd8 | 04:21:56 clang-ppc64be-linux | 04:19:57 clang-ppc64le-linux | 04:16:53 clang-ppc64le-linux-multistage| 03:50:03 clang-bpf-build | 03:31:05 ubuntu-gcc7.1-werror | 03:23:19 clang-cmake-aarch64-lld | 03:05:36 libcxx-libcxxabi-x86_64-linux-ubuntu-asan | 02:50:12 lldb-x86_64-ubuntu-14.04-cmake| 02:40:21 sanitizer-x86_64-linux| 02:31:45 clang-sphinx-docs | 02:24:37 clang-atom-d525-fedora-rel| 02:14:29 sanitizer-ppc64be-linux | 02:01:17 sanitizer-x86_64-linux-bootstrap-ubsan| 01:59:51 clang-cuda-build | 01:47:58 clang-native-arm-lnt | 01:43:53 clang-cmake-x86_64-sde-avx512-linux | 01:38:09 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast| 01:37:16 sanitizer-x86_64-linux-bootstrap-msan | 01:35:56 clang-x86_64-linux-abi-test | 01:32:39 clang-cmake-x86_64-avx2-linux | 01:29:50 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast | 01:23:10 libcxx-libcxxabi-libunwind-arm-linux-noexceptions | 01:22:32 clang-x86_64-debian-fast | 01:20:12 sanitizer-x86_64-linux-fuzzer | 01:16:27 reverse-iteration | 01:15:48 clang-cmake-x86_64-avx2-linux-perf| 01:13:52 lld-x86_64-darwin13 | 01:10:02 polly-amd64-linux | 01:04:01 perf-x86_64-penryn-O3-polly-parallel-fast | 01:03:10 lldb-x86_64-darwin-13.4 | 01:00:51 perf-x86_64-penryn-O3-polly-unprofitable | 00:50:02 lldb-x86_6
[PATCH] D41929: [Fuchsia] Disable LLDB from the toolchain distribution
phosek created this revision. phosek added reviewers: mcgrathr, jakehehrlich. Herald added subscribers: cfe-commits, mgorny. This is currently not being used so disable it to reduce toolchain size. Repository: rC Clang https://reviews.llvm.org/D41929 Files: cmake/caches/Fuchsia-stage2.cmake Index: cmake/caches/Fuchsia-stage2.cmake === --- cmake/caches/Fuchsia-stage2.cmake +++ cmake/caches/Fuchsia-stage2.cmake @@ -19,10 +19,6 @@ set(CLANG_DEFAULT_LINKER lld CACHE STRING "") endif() -if(APPLE) - set(LLDB_CODESIGN_IDENTITY "" CACHE STRING "") -endif() - set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "") set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "") set(CMAKE_C_FLAGS_RELWITHDEBINFO "-O3 -gline-tables-only -DNDEBUG" CACHE STRING "") @@ -83,8 +79,6 @@ set(LLVM_DISTRIBUTION_COMPONENTS clang lld - lldb - liblldb LTO clang-format clang-headers Index: cmake/caches/Fuchsia-stage2.cmake === --- cmake/caches/Fuchsia-stage2.cmake +++ cmake/caches/Fuchsia-stage2.cmake @@ -19,10 +19,6 @@ set(CLANG_DEFAULT_LINKER lld CACHE STRING "") endif() -if(APPLE) - set(LLDB_CODESIGN_IDENTITY "" CACHE STRING "") -endif() - set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "") set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "") set(CMAKE_C_FLAGS_RELWITHDEBINFO "-O3 -gline-tables-only -DNDEBUG" CACHE STRING "") @@ -83,8 +79,6 @@ set(LLVM_DISTRIBUTION_COMPONENTS clang lld - lldb - liblldb LTO clang-format clang-headers ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41930: [Fuchsia] Use llvm-objcopy as objcopy on Linux
phosek created this revision. phosek added reviewers: mcgrathr, jakehehrlich. Herald added subscribers: cfe-commits, mgorny. llvm-objcopy already supports all the necessary functionality for ELF. Repository: rC Clang https://reviews.llvm.org/D41930 Files: cmake/caches/Fuchsia-stage2.cmake Index: cmake/caches/Fuchsia-stage2.cmake === --- cmake/caches/Fuchsia-stage2.cmake +++ cmake/caches/Fuchsia-stage2.cmake @@ -17,6 +17,7 @@ if(NOT APPLE) set(LLVM_ENABLE_LLD ON CACHE BOOL "") set(CLANG_DEFAULT_LINKER lld CACHE STRING "") + set(CLANG_DEFAULT_OBJCOPY llvm-objcopy CACHE STRING "") endif() if(APPLE) Index: cmake/caches/Fuchsia-stage2.cmake === --- cmake/caches/Fuchsia-stage2.cmake +++ cmake/caches/Fuchsia-stage2.cmake @@ -17,6 +17,7 @@ if(NOT APPLE) set(LLVM_ENABLE_LLD ON CACHE BOOL "") set(CLANG_DEFAULT_LINKER lld CACHE STRING "") + set(CLANG_DEFAULT_OBJCOPY llvm-objcopy CACHE STRING "") endif() if(APPLE) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41930: [Fuchsia] Use llvm-objcopy as objcopy on Linux
emaste added a comment. The title is more correctly "non-Apple hosts"? I.e., building on FreeBSD will also use llvm-objcopy. Repository: rC Clang https://reviews.llvm.org/D41930 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322244 - [X86][Sema] Remove constant range checks on on builtins that take a char.
Author: ctopper Date: Wed Jan 10 17:37:57 2018 New Revision: 322244 URL: http://llvm.org/viewvc/llvm-project?rev=322244&view=rev Log: [X86][Sema] Remove constant range checks on on builtins that take a char. The constant is already reduced to 8-bits by the time we get here and the checks were just ensuring that it was 8 bits. Thus I don't think there's anyway for them to fail. Modified: cfe/trunk/lib/Sema/SemaChecking.cpp Modified: cfe/trunk/lib/Sema/SemaChecking.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=322244&r1=322243&r2=322244&view=diff == --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) +++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Jan 10 17:37:57 2018 @@ -2361,13 +2361,6 @@ bool Sema::CheckX86BuiltinFunctionCall(u case X86::BI__builtin_ia32_cmpss_mask: i = 2; l = 0; u = 31; break; - case X86::BI__builtin_ia32_xabort: -i = 0; l = -128; u = 255; -break; - case X86::BI__builtin_ia32_pshufw: - case X86::BI__builtin_ia32_aeskeygenassist128: -i = 1; l = -128; u = 255; -break; case X86::BI__builtin_ia32_vcvtps2ph: case X86::BI__builtin_ia32_vcvtps2ph_mask: case X86::BI__builtin_ia32_vcvtps2ph256: @@ -2405,27 +2398,6 @@ bool Sema::CheckX86BuiltinFunctionCall(u case X86::BI__builtin_ia32_fpcla_mask: i = 1; l = 0; u = 255; break; - case X86::BI__builtin_ia32_palignr: - case X86::BI__builtin_ia32_insertps128: - case X86::BI__builtin_ia32_dpps: - case X86::BI__builtin_ia32_dppd: - case X86::BI__builtin_ia32_dpps256: - case X86::BI__builtin_ia32_mpsadbw128: - case X86::BI__builtin_ia32_mpsadbw256: - case X86::BI__builtin_ia32_pcmpistrm128: - case X86::BI__builtin_ia32_pcmpistri128: - case X86::BI__builtin_ia32_pcmpistria128: - case X86::BI__builtin_ia32_pcmpistric128: - case X86::BI__builtin_ia32_pcmpistrio128: - case X86::BI__builtin_ia32_pcmpistris128: - case X86::BI__builtin_ia32_pcmpistriz128: - case X86::BI__builtin_ia32_pclmulqdq128: - case X86::BI__builtin_ia32_vperm2f128_pd256: - case X86::BI__builtin_ia32_vperm2f128_ps256: - case X86::BI__builtin_ia32_vperm2f128_si256: - case X86::BI__builtin_ia32_permti256: -i = 2; l = -128; u = 255; -break; case X86::BI__builtin_ia32_palignr128: case X86::BI__builtin_ia32_palignr256: case X86::BI__builtin_ia32_palignr512_mask: @@ -2480,15 +2452,6 @@ bool Sema::CheckX86BuiltinFunctionCall(u case X86::BI__builtin_ia32_scatterpfqps: i = 4; l = 2; u = 3; break; - case X86::BI__builtin_ia32_pcmpestrm128: - case X86::BI__builtin_ia32_pcmpestri128: - case X86::BI__builtin_ia32_pcmpestria128: - case X86::BI__builtin_ia32_pcmpestric128: - case X86::BI__builtin_ia32_pcmpestrio128: - case X86::BI__builtin_ia32_pcmpestris128: - case X86::BI__builtin_ia32_pcmpestriz128: -i = 4; l = -128; u = 255; -break; case X86::BI__builtin_ia32_rndscalesd_round_mask: case X86::BI__builtin_ia32_rndscaless_round_mask: i = 4; l = 0; u = 255; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322245 - [X86] Make -mavx512f imply -mfma and -mf16c in the frontend like it does in the backend.
Author: ctopper Date: Wed Jan 10 17:37:59 2018 New Revision: 322245 URL: http://llvm.org/viewvc/llvm-project?rev=322245&view=rev Log: [X86] Make -mavx512f imply -mfma and -mf16c in the frontend like it does in the backend. Similarly, make -mno-fma and -mno-f16c imply -mno-avx512f. Withou this "-mno-sse -mavx512f" ends up with avx512f being enabled in the frontend but disabled in the backend. Modified: cfe/trunk/lib/Basic/Targets/X86.cpp Modified: cfe/trunk/lib/Basic/Targets/X86.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=322245&r1=322244&r2=322245&view=diff == --- cfe/trunk/lib/Basic/Targets/X86.cpp (original) +++ cfe/trunk/lib/Basic/Targets/X86.cpp Wed Jan 10 17:37:59 2018 @@ -430,7 +430,7 @@ void X86TargetInfo::setSSELevel(llvm::St if (Enabled) { switch (Level) { case AVX512F: - Features["avx512f"] = true; + Features["avx512f"] = Features["fma"] = Features["f16c"] = true; LLVM_FALLTHROUGH; case AVX2: Features["avx2"] = true; @@ -644,6 +644,8 @@ void X86TargetInfo::setFeatureEnabledImp } else if (Name == "fma") { if (Enabled) setSSELevel(Features, AVX, Enabled); +else + setSSELevel(Features, AVX512F, Enabled); } else if (Name == "fma4") { setXOPLevel(Features, FMA4, Enabled); } else if (Name == "xop") { @@ -653,6 +655,8 @@ void X86TargetInfo::setFeatureEnabledImp } else if (Name == "f16c") { if (Enabled) setSSELevel(Features, AVX, Enabled); +else + setSSELevel(Features, AVX512F, Enabled); } else if (Name == "sha") { if (Enabled) setSSELevel(Features, SSE2, Enabled); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322247 - [X86][Sema] Range check the constant argument for the vpshld/vpshrd builtins to ensure it fits in 8-bits.
Author: ctopper Date: Wed Jan 10 17:38:02 2018 New Revision: 322247 URL: http://llvm.org/viewvc/llvm-project?rev=322247&view=rev Log: [X86][Sema] Range check the constant argument for the vpshld/vpshrd builtins to ensure it fits in 8-bits. Modified: cfe/trunk/lib/Sema/SemaChecking.cpp cfe/trunk/test/Sema/builtins-x86.c Modified: cfe/trunk/lib/Sema/SemaChecking.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=322247&r1=322246&r2=322247&view=diff == --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) +++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Jan 10 17:38:02 2018 @@ -2410,6 +2410,24 @@ bool Sema::CheckX86BuiltinFunctionCall(u case X86::BI__builtin_ia32_dbpsadbw128_mask: case X86::BI__builtin_ia32_dbpsadbw256_mask: case X86::BI__builtin_ia32_dbpsadbw512_mask: + case X86::BI__builtin_ia32_vpshldd128_mask: + case X86::BI__builtin_ia32_vpshldd256_mask: + case X86::BI__builtin_ia32_vpshldd512_mask: + case X86::BI__builtin_ia32_vpshldq128_mask: + case X86::BI__builtin_ia32_vpshldq256_mask: + case X86::BI__builtin_ia32_vpshldq512_mask: + case X86::BI__builtin_ia32_vpshldw128_mask: + case X86::BI__builtin_ia32_vpshldw256_mask: + case X86::BI__builtin_ia32_vpshldw512_mask: + case X86::BI__builtin_ia32_vpshrdd128_mask: + case X86::BI__builtin_ia32_vpshrdd256_mask: + case X86::BI__builtin_ia32_vpshrdd512_mask: + case X86::BI__builtin_ia32_vpshrdq128_mask: + case X86::BI__builtin_ia32_vpshrdq256_mask: + case X86::BI__builtin_ia32_vpshrdq512_mask: + case X86::BI__builtin_ia32_vpshrdw128_mask: + case X86::BI__builtin_ia32_vpshrdw256_mask: + case X86::BI__builtin_ia32_vpshrdw512_mask: i = 2; l = 0; u = 255; break; case X86::BI__builtin_ia32_fixupimmpd512_mask: Modified: cfe/trunk/test/Sema/builtins-x86.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtins-x86.c?rev=322247&r1=322246&r2=322247&view=diff == --- cfe/trunk/test/Sema/builtins-x86.c (original) +++ cfe/trunk/test/Sema/builtins-x86.c Wed Jan 10 17:38:02 2018 @@ -4,12 +4,17 @@ typedef long long __m128i __attribute__( typedef float __m128 __attribute__((__vector_size__(16))); typedef double __m128d __attribute__((__vector_size__(16))); +typedef long long __m256i __attribute__((__vector_size__(32))); +typedef float __m256 __attribute__((__vector_size__(32))); +typedef double __m256d __attribute__((__vector_size__(32))); + typedef long long __m512i __attribute__((__vector_size__(64))); typedef float __m512 __attribute__((__vector_size__(64))); typedef double __m512d __attribute__((__vector_size__(64))); typedef unsigned char __mmask8; typedef unsigned short __mmask16; +typedef unsigned int __mmask32; __m128 test__builtin_ia32_cmpps(__m128 __a, __m128 __b) { __builtin_ia32_cmpps(__a, __b, 32); // expected-error {{argument should be a value from 0 to 31}} @@ -83,3 +88,74 @@ __m512 _mm512_mask_prefetch_i32gather_ps return __builtin_ia32_gatherpfdps(mask, index, addr, 1, 1); // expected-error {{argument should be a value from 2 to 3}} } +__m512i test_mm512_mask_shldi_epi64(__m512i __S, __mmask8 __U, __m512i __A, __m512i __B) { + return __builtin_ia32_vpshldq512_mask(__A, __B, 1024, __S, __U); // expected-error {{argument should be a value from 0 to 255}} +} + +__m512i test_mm512_mask_shldi_epi32(__m512i __S, __mmask16 __U, __m512i __A, __m512i __B) { + return __builtin_ia32_vpshldd512_mask(__A, __B, 1024, __S, __U); // expected-error {{argument should be a value from 0 to 255}} +} + +__m512i test_mm512_mask_shldi_epi16(__m512i __S, __mmask32 __U, __m512i __A, __m512i __B) { + return __builtin_ia32_vpshldw512_mask(__A, __B, 1024, __S, __U); // expected-error {{argument should be a value from 0 to 255}} +} + +__m512i test_mm512_mask_shrdi_epi64(__m512i __S, __mmask8 __U, __m512i __A, __m512i __B) { + return __builtin_ia32_vpshrdq512_mask(__A, __B, 1024, __S, __U); // expected-error {{argument should be a value from 0 to 255}} +} + +__m512i test_mm512_mask_shrdi_epi32(__m512i __S, __mmask16 __U, __m512i __A, __m512i __B) { + return __builtin_ia32_vpshrdd512_mask(__A, __B, 1024, __S, __U); // expected-error {{argument should be a value from 0 to 255}} +} + +__m512i test_mm512_mask_shrdi_epi16(__m512i __S, __mmask32 __U, __m512i __A, __m512i __B) { + return __builtin_ia32_vpshrdw512_mask(__A, __B, 1024, __S, __U); // expected-error {{argument should be a value from 0 to 255}} +} + +__m256i test_mm256_mask_shldi_epi64(__m256i __S, __mmask8 __U, __m256i __A, __m256i __B) { + return __builtin_ia32_vpshldq256_mask(__A, __B, 1024, __S, __U); // expected-error {{argument should be a value from 0 to 255}} +} + +__m128i test_mm128_mask_shldi_epi64(__m128i __S, __mmask8 __U, __m128i __A, __m128i __B) { + return __builtin_ia32_vpshldq128_mask(__A, __B, 1024, __S, __U); // expected-er
r322246 - [X86] Fix vpshrd builtins to require an ICE for their constant argument to match vpshld.
Author: ctopper Date: Wed Jan 10 17:38:00 2018 New Revision: 322246 URL: http://llvm.org/viewvc/llvm-project?rev=322246&view=rev Log: [X86] Fix vpshrd builtins to require an ICE for their constant argument to match vpshld. Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=322246&r1=322245&r2=322246&view=diff == --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original) +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Wed Jan 10 17:38:00 2018 @@ -1357,15 +1357,15 @@ TARGET_BUILTIN(__builtin_ia32_vpshrdvw12 TARGET_BUILTIN(__builtin_ia32_vpshrdvw256_maskz, "V16sV16sV16sV16sUs", "", "avx512vl,avx512vbmi2") TARGET_BUILTIN(__builtin_ia32_vpshrdvw512_maskz, "V32sV32sV32sV32sUi", "", "avx512vbmi2") -TARGET_BUILTIN(__builtin_ia32_vpshrdd128_mask, "V4iV4iV4iiV4iUc", "", "avx512vl,avx512vbmi2") -TARGET_BUILTIN(__builtin_ia32_vpshrdd256_mask, "V8iV8iV8iiV8iUc", "", "avx512vl,avx512vbmi2") -TARGET_BUILTIN(__builtin_ia32_vpshrdd512_mask, "V16iV16iV16iiV16iUs", "", "avx512vbmi2") -TARGET_BUILTIN(__builtin_ia32_vpshrdq128_mask, "V2LLiV2LLiV2LLiiV2LLiUc", "", "avx512vl,avx512vbmi2") -TARGET_BUILTIN(__builtin_ia32_vpshrdq256_mask, "V4LLiV4LLiV4LLiiV4LLiUc", "", "avx512vl,avx512vbmi2") -TARGET_BUILTIN(__builtin_ia32_vpshrdq512_mask, "V8LLiV8LLiV8LLiiV8LLiUc", "", "avx512vbmi2") -TARGET_BUILTIN(__builtin_ia32_vpshrdw128_mask, "V8sV8sV8siV8sUc", "", "avx512vl,avx512vbmi2") -TARGET_BUILTIN(__builtin_ia32_vpshrdw256_mask, "V16sV16sV16siV16sUs", "", "avx512vl,avx512vbmi2") -TARGET_BUILTIN(__builtin_ia32_vpshrdw512_mask, "V32sV32sV32siV32sUi", "", "avx512vbmi2") +TARGET_BUILTIN(__builtin_ia32_vpshrdd128_mask, "V4iV4iV4iIiV4iUc", "", "avx512vl,avx512vbmi2") +TARGET_BUILTIN(__builtin_ia32_vpshrdd256_mask, "V8iV8iV8iIiV8iUc", "", "avx512vl,avx512vbmi2") +TARGET_BUILTIN(__builtin_ia32_vpshrdd512_mask, "V16iV16iV16iIiV16iUs", "", "avx512vbmi2") +TARGET_BUILTIN(__builtin_ia32_vpshrdq128_mask, "V2LLiV2LLiV2LLiIiV2LLiUc", "", "avx512vl,avx512vbmi2") +TARGET_BUILTIN(__builtin_ia32_vpshrdq256_mask, "V4LLiV4LLiV4LLiIiV4LLiUc", "", "avx512vl,avx512vbmi2") +TARGET_BUILTIN(__builtin_ia32_vpshrdq512_mask, "V8LLiV8LLiV8LLiIiV8LLiUc", "", "avx512vbmi2") +TARGET_BUILTIN(__builtin_ia32_vpshrdw128_mask, "V8sV8sV8sIiV8sUc", "", "avx512vl,avx512vbmi2") +TARGET_BUILTIN(__builtin_ia32_vpshrdw256_mask, "V16sV16sV16sIiV16sUs", "", "avx512vl,avx512vbmi2") +TARGET_BUILTIN(__builtin_ia32_vpshrdw512_mask, "V32sV32sV32sIiV32sUi", "", "avx512vbmi2") TARGET_BUILTIN(__builtin_ia32_pmovswb512_mask, "V32cV32sV32cUi", "", "avx512bw") TARGET_BUILTIN(__builtin_ia32_pmovuswb512_mask, "V32cV32sV32cUi", "", "avx512bw") ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41423: [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral.
vsapsai updated this revision to Diff 129379. vsapsai added a comment. - Add a test for null character in string/character literals. https://reviews.llvm.org/D41423 Files: clang/lib/Lex/Lexer.cpp clang/test/Lexer/null-character-in-literal.c clang/unittests/Lex/LexerTest.cpp Index: clang/unittests/Lex/LexerTest.cpp === --- clang/unittests/Lex/LexerTest.cpp +++ clang/unittests/Lex/LexerTest.cpp @@ -475,6 +475,8 @@ TEST_F(LexerTest, AvoidPastEndOfStringDereference) { EXPECT_TRUE(Lex(" // \\\n").empty()); + EXPECT_TRUE(Lex("#include <").empty()); + EXPECT_TRUE(Lex("#include <\n").empty()); } TEST_F(LexerTest, StringizingRasString) { Index: clang/lib/Lex/Lexer.cpp === --- clang/lib/Lex/Lexer.cpp +++ clang/lib/Lex/Lexer.cpp @@ -2009,18 +2009,21 @@ const char *AfterLessPos = CurPtr; char C = getAndAdvanceChar(CurPtr, Result); while (C != '>') { -// Skip escaped characters. -if (C == '\\' && CurPtr < BufferEnd) { - // Skip the escaped character. - getAndAdvanceChar(CurPtr, Result); -} else if (C == '\n' || C == '\r' || // Newline. - (C == 0 && (CurPtr-1 == BufferEnd || // End of file. - isCodeCompletionPoint(CurPtr-1 { +// Skip escaped characters. Escaped newlines will already be processed by +// getAndAdvanceChar. +if (C == '\\') + C = getAndAdvanceChar(CurPtr, Result); + +if (C == '\n' || C == '\r' || // Newline. +(C == 0 && (CurPtr-1 == BufferEnd || // End of file. +isCodeCompletionPoint(CurPtr-1 { // If the filename is unterminated, then it must just be a lone < // character. Return this as such. FormTokenWithChars(Result, AfterLessPos, tok::less); return true; -} else if (C == 0) { +} + +if (C == 0) { NulCharacter = CurPtr-1; } C = getAndAdvanceChar(CurPtr, Result); Index: clang/unittests/Lex/LexerTest.cpp === --- clang/unittests/Lex/LexerTest.cpp +++ clang/unittests/Lex/LexerTest.cpp @@ -475,6 +475,8 @@ TEST_F(LexerTest, AvoidPastEndOfStringDereference) { EXPECT_TRUE(Lex(" // \\\n").empty()); + EXPECT_TRUE(Lex("#include <").empty()); + EXPECT_TRUE(Lex("#include <\n").empty()); } TEST_F(LexerTest, StringizingRasString) { Index: clang/lib/Lex/Lexer.cpp === --- clang/lib/Lex/Lexer.cpp +++ clang/lib/Lex/Lexer.cpp @@ -2009,18 +2009,21 @@ const char *AfterLessPos = CurPtr; char C = getAndAdvanceChar(CurPtr, Result); while (C != '>') { -// Skip escaped characters. -if (C == '\\' && CurPtr < BufferEnd) { - // Skip the escaped character. - getAndAdvanceChar(CurPtr, Result); -} else if (C == '\n' || C == '\r' || // Newline. - (C == 0 && (CurPtr-1 == BufferEnd || // End of file. - isCodeCompletionPoint(CurPtr-1 { +// Skip escaped characters. Escaped newlines will already be processed by +// getAndAdvanceChar. +if (C == '\\') + C = getAndAdvanceChar(CurPtr, Result); + +if (C == '\n' || C == '\r' || // Newline. +(C == 0 && (CurPtr-1 == BufferEnd || // End of file. +isCodeCompletionPoint(CurPtr-1 { // If the filename is unterminated, then it must just be a lone < // character. Return this as such. FormTokenWithChars(Result, AfterLessPos, tok::less); return true; -} else if (C == 0) { +} + +if (C == 0) { NulCharacter = CurPtr-1; } C = getAndAdvanceChar(CurPtr, Result); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41423: [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral.
vsapsai marked 2 inline comments as done. vsapsai added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:2026 + +if (C == 0) { NulCharacter = CurPtr-1; dexonsmith wrote: > vsapsai wrote: > > dexonsmith wrote: > > > Should this check still be skipped (in an `else if` of the `C == '\\'` > > > check)? > > I agree it is behaviour change. `NulCharacter` is used to warn if you have > > null character in the string and I think it is worth warning even if it is > > preceded by the backslash. Therefore I think this check shouldn't be > > skipped after `C == '\\'` check. In practice I don't know how you can > > create a file with null character in its name and use it in inclusion > > directive. > Can you add a test for the behaviour change then? Added clang/test/Lexer/null-character-in-literal.c but struggling with Phabricator to display it as text file, not as binary. Guess those null characters can be confusing for some tools. Comment at: clang/unittests/Lex/LexerTest.cpp:477 TEST_F(LexerTest, AvoidPastEndOfStringDereference) { - std::vector LexedTokens = Lex(" // \\\n"); - EXPECT_TRUE(LexedTokens.empty()); + EXPECT_TRUE(Lex(" // \\\n").empty()); + // rdar://problem/35572754 dexonsmith wrote: > vsapsai wrote: > > dexonsmith wrote: > > > To minimize the diff, please separate this change into an NFC commit > > > ahead of time. > > I plan to nominate this fix for inclusion in 6.0.0 release and having one > > commit will be easier. It is not necessary to include NFC change in 6.0.0 > > release but it creates discrepancy that increases a chance of merge > > conflicts. > > > > But I don't have strong opinion, just pointing out potential downsides with > > merging the change to other branches. > I think it's worth separating the NFC changes from behaviour changes, even if > it means having to cherry-pick extra patches. OK, done. https://reviews.llvm.org/D41423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41423: [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral.
vsapsai marked an inline comment as done. vsapsai added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:2026 + +if (C == 0) { NulCharacter = CurPtr-1; vsapsai wrote: > dexonsmith wrote: > > vsapsai wrote: > > > dexonsmith wrote: > > > > Should this check still be skipped (in an `else if` of the `C == '\\'` > > > > check)? > > > I agree it is behaviour change. `NulCharacter` is used to warn if you > > > have null character in the string and I think it is worth warning even if > > > it is preceded by the backslash. Therefore I think this check shouldn't > > > be skipped after `C == '\\'` check. In practice I don't know how you can > > > create a file with null character in its name and use it in inclusion > > > directive. > > Can you add a test for the behaviour change then? > Added clang/test/Lexer/null-character-in-literal.c but struggling with > Phabricator to display it as text file, not as binary. Guess those null > characters can be confusing for some tools. Added the test file at https://gist.github.com/vsapsai/d61a8a7d06356a18db26658acc0c4b25 https://reviews.llvm.org/D41423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits