[PATCH] D41005: Reuse preamble even if an unsaved file does not exist
ilya-biryukov added a comment. In https://reviews.llvm.org/D41005#949550, @cameron314 wrote: > It's been a while since I was in this code, but I seem to recall the file > needed to exist on disk in order to uniquely identify it (via inode). Does > this break the up-to-date check? When the file is missing from the disk, but was remapped, preamble can not be reused. (Because we're always looking at uniqueids). If the remapped file did not exist on disk originally when building the preamble, we should allow the preamble to be reused if it's still remapped, but not created on disk. Repository: rC Clang https://reviews.llvm.org/D41005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40715: [analyser] different.LabelInsideSwitch checker implementation
a.sidorin added a comment. Hello Alexey, Thank you for the update. The code looks much cleaner now. Comment at: lib/StaticAnalyzer/Checkers/LabelInsideSwitchChecker.cpp:115 + +namespace clang { + namespace ento { alexey.knyshev wrote: > a.sidorin wrote: > > You can write just `void > > ento::registerLabelInsideSwitchChecker(CheckerManager &Mgr) { > Actually I can't, get error: > > ``` > /llvm/tools/clang/lib/StaticAnalyzer/Checkers/LabelInsideSwitchChecker.cpp:103:68: > error: 'void > clang::ento::registerLabelInsideSwitchChecker(clang::ento::CheckerManager&)' > should have been declared inside 'clang::ento' > void ento::registerLabelInsideSwitchChecker(CheckerManager &Mgr) { > > ``` This looks strange. I can remember that I have met this issue but I cannot remember how I resolved it. Comment at: lib/StaticAnalyzer/Checkers/LabelInsideSwitchChecker.cpp:1 +//== LabelInsideSwitchChecker.cpp - Lable inside switch checker -*- C++ -*--==// +// Check for label inside switch Comment at: lib/StaticAnalyzer/Checkers/LabelInsideSwitchChecker.cpp:11 +// This defines LabelInsideSwitchChecker, which looks for typos in switch +// cases like missprinting 'defualt', 'cas' or other accidental insertion +// of labeled statement. misprinting Comment at: lib/StaticAnalyzer/Checkers/LabelInsideSwitchChecker.cpp:19 +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" + We don't use CheckerContext in the code below. Looks like this include is redundant or should be replaced by more relevant include files. Comment at: lib/StaticAnalyzer/Checkers/LabelInsideSwitchChecker.cpp:87 +BugReporter &BR) const { + auto LabelStmt = stmt(hasDescendant(switchStmt( + eachOf(has(compoundStmt(forEach(labelStmt().bind("label", alexey.knyshev wrote: > Looks like I have to use `forEachDescendant` instead of `hasDescendant`. > Please, comment! 1. Yes, `hasDescendant()` will give you only single match. `forEachDescendant()` will continue matching after match found and that is what we should do here. 2. Maybe we can just use stmt(forEachDescendant(switchStmt(forEachDescendant(labelStmt()? We don't distinguish "label" and "label_in_case" anyway. Also, current matcher will ignore deeply-nested switches: ``` switch (x) } case 1: { {{ label: }} // ignored } } ``` Is that intentional? Repository: rC Clang https://reviews.llvm.org/D40715 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40705: [Parser] Diagnose storage classes in template parameter declarations
miyuki added a comment. Ping https://reviews.llvm.org/D40705 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40707: [libcxx] Fix basic_stringbuf constructor
miyuki added a comment. Ping https://reviews.llvm.org/D40707 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
ilya-biryukov updated this revision to Diff 126315. ilya-biryukov added a comment. - Made ContextData a private member of Context. - Added clone method. - Added forgotten header guard in TypedValueMap.h - Pass Key<> by const-ref instead of by-ref. - Pass Context by-const-ref instead of by-ref. - Made derive() const. - Added docs. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 Files: clangd/CMakeLists.txt clangd/Context.cpp clangd/Context.h clangd/TypedValueMap.h unittests/clangd/CMakeLists.txt unittests/clangd/ContextTests.cpp Index: unittests/clangd/ContextTests.cpp === --- /dev/null +++ unittests/clangd/ContextTests.cpp @@ -0,0 +1,71 @@ +//===-- ContextTests.cpp - Context tests *- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "Context.h" + +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { + +TEST(TypedValueMapTests, Simple) { + Key IntParam; + Key ExtraIntParam; + + clangd::TypedValueMap Ctx; + + ASSERT_TRUE(Ctx.emplace(IntParam, 10)); + ASSERT_TRUE(Ctx.emplace(ExtraIntParam, 20)); + + EXPECT_EQ(*Ctx.get(IntParam), 10); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); + + ASSERT_FALSE(Ctx.emplace(IntParam, 30)); + + ASSERT_TRUE(Ctx.remove(IntParam)); + EXPECT_EQ(Ctx.get(IntParam), nullptr); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); + + ASSERT_TRUE(Ctx.emplace(IntParam, 30)); + EXPECT_EQ(*Ctx.get(IntParam), 30); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); +} + +TEST(TypedValueMapTests, MoveOps) { + Key> Param; + + clangd::TypedValueMap Ctx; + Ctx.emplace(Param, llvm::make_unique(10)); + EXPECT_EQ(**Ctx.get(Param), 10); + + clangd::TypedValueMap NewCtx = std::move(Ctx); + EXPECT_EQ(**NewCtx.get(Param), 10); +} + +TEST(ContextTests, Builders) { + Key ParentParam; + Key ParentAndChildParam; + Key ChildParam; + + Context ParentCtx = + buildCtx().add(ParentParam, 10).add(ParentAndChildParam, 20); + Context ChildCtx = + ParentCtx.derive().add(ParentAndChildParam, 30).add(ChildParam, 40); + + EXPECT_EQ(*ParentCtx.get(ParentParam), 10); + EXPECT_EQ(*ParentCtx.get(ParentAndChildParam), 20); + EXPECT_EQ(ParentCtx.get(ChildParam), nullptr); + + EXPECT_EQ(*ChildCtx.get(ParentParam), 10); + EXPECT_EQ(*ChildCtx.get(ParentAndChildParam), 30); + EXPECT_EQ(*ChildCtx.get(ChildParam), 40); +} + +} // namespace clangd +} // namespace clang Index: unittests/clangd/CMakeLists.txt === --- unittests/clangd/CMakeLists.txt +++ unittests/clangd/CMakeLists.txt @@ -11,6 +11,7 @@ add_extra_unittest(ClangdTests ClangdTests.cpp CodeCompleteTests.cpp + ContextTests.cpp FuzzyMatchTests.cpp JSONExprTests.cpp TestFS.cpp Index: clangd/TypedValueMap.h === --- /dev/null +++ clangd/TypedValueMap.h @@ -0,0 +1,100 @@ +//===--- TypedValueMap.h - Type-safe heterogenous key-value map -*- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// Type-safe heterogenous map. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TYPEDVALUEMAP_H_ +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TYPEDVALUEMAP_H_ + +#include "llvm/ADT/DenseMap.h" +#include + +namespace clang { +namespace clangd { + +/// Used as identity for map values. Non-movable and non-copyable. Address of +/// this object is used internally to as keys in a map. +template class Key { +public: + static_assert(!std::is_reference::value, +"Reference arguments to Key<> are not allowed"); + + Key() = default; + + Key(Key const &) = delete; + Key &operator=(Key const &) = delete; + Key(Key &&) = delete; + Key &operator=(Key &&) = delete; +}; + +/// A type-safe map from Key to T. +class TypedValueMap { +public: + TypedValueMap() = default; + TypedValueMap(const TypedValueMap &) = delete; + TypedValueMap(TypedValueMap &&) = default; + + template Type *get(const Key &Key) const { +auto It = Map.find(&Key); +if (It == Map.end()) + return nullptr; +return static_cast(It->second->getValuePtr()); + } + + template + bool emplace(const Key &Key, Args &&... As) { +bool Added = +Map.try_emplace(&Key, +llvm::make_unique< +TypedAnyStorage::type>>( +std::forward(As)...)) +.second; +return Added; + } + + te
[PATCH] D40486: [clangd] Implemented logging using Context
ilya-biryukov updated this revision to Diff 126316. ilya-biryukov added a comment. - Pass Context by const-ref instead of by-ref. - Don't expose global logger, it is only used in log method now. - Renamed logImpl to log. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40486 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/ClangdUnitStore.cpp clangd/ClangdUnitStore.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/GlobalCompilationDatabase.cpp clangd/GlobalCompilationDatabase.h clangd/JSONRPCDispatcher.cpp clangd/JSONRPCDispatcher.h clangd/Logger.cpp clangd/Logger.h clangd/Protocol.h clangd/ProtocolHandlers.cpp clangd/ProtocolHandlers.h clangd/tool/ClangdMain.cpp unittests/clangd/ClangdTests.cpp unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -8,6 +8,7 @@ //===--===// #include "ClangdServer.h" #include "Compiler.h" +#include "Context.h" #include "Protocol.h" #include "TestFS.h" #include "gtest/gtest.h" @@ -74,8 +75,7 @@ MockCompilationDatabase CDB; ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, - EmptyLogger::getInstance()); + /*StorePreamblesInMemory=*/true); auto FooCpp = getVirtualTestFilePath("foo.cpp"); const auto SourceContents = R"cpp( @@ -101,29 +101,33 @@ // No need to sync reparses here as there are no asserts on diagnostics (or // other async operations). - Server.addDocument(FooCpp, SourceContents); + Server.addDocument(FooCpp, SourceContents, buildCtx()); { auto CodeCompletionResults1 = -Server.codeComplete(FooCpp, CompletePos, CCOpts, None).get().Value; +Server.codeComplete(FooCpp, CompletePos, CCOpts, buildCtx(), None) +.get() +.first.Value; EXPECT_TRUE(ContainsItem(CodeCompletionResults1, "aba")); EXPECT_FALSE(ContainsItem(CodeCompletionResults1, "cbc")); } { auto CodeCompletionResultsOverriden = Server -.codeComplete(FooCpp, CompletePos, CCOpts, +.codeComplete(FooCpp, CompletePos, CCOpts, buildCtx(), StringRef(OverridenSourceContents)) .get() -.Value; +.first.Value; EXPECT_TRUE(ContainsItem(CodeCompletionResultsOverriden, "cbc")); EXPECT_FALSE(ContainsItem(CodeCompletionResultsOverriden, "aba")); } { auto CodeCompletionResults2 = -Server.codeComplete(FooCpp, CompletePos, CCOpts, None).get().Value; +Server.codeComplete(FooCpp, CompletePos, CCOpts, buildCtx(), None) +.get() +.first.Value; EXPECT_TRUE(ContainsItem(CodeCompletionResults2, "aba")); EXPECT_FALSE(ContainsItem(CodeCompletionResults2, "cbc")); } @@ -135,8 +139,7 @@ CDB.ExtraClangFlags.push_back("-xc++"); IgnoreDiagnostics DiagConsumer; ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, - EmptyLogger::getInstance()); + /*StorePreamblesInMemory=*/true); auto FooCpp = getVirtualTestFilePath("foo.cpp"); FS.Files[FooCpp] = ""; @@ -150,17 +153,17 @@ int main() { ClassWithMembers().{complete} } )cpp", "complete"); - Server.addDocument(FooCpp, Completion.Text); + Server.addDocument(FooCpp, Completion.Text, buildCtx()); clangd::CodeCompleteOptions Opts; Opts.Limit = 2; /// For after-dot completion we must always get consistent results. auto Results = Server .codeComplete(FooCpp, Completion.MarkerPos, Opts, - StringRef(Completion.Text)) + buildCtx(), StringRef(Completion.Text)) .get() - .Value; + .first.Value; EXPECT_TRUE(Results.isIncomplete); EXPECT_EQ(Opts.Limit, Results.items.size()); @@ -175,8 +178,7 @@ CDB.ExtraClangFlags.push_back("-xc++"); IgnoreDiagnostics DiagConsumer; ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, - EmptyLogger::getInstance()); + /*StorePreamblesInMemory=*/true); auto FooCpp = getVirtualTestFilePath("foo.cpp"); FS.Files[FooCpp] = ""; @@ -194,12 +196,13 @@ StringWithPos Completion = parseTextMarker( formatv("{0} int main() { {1}{{complete}} }", Body, Query).str(), "complete"); -Server.addDo
[PATCH] D40705: [Parser] Diagnose storage classes in template parameter declarations
rogfer01 added inline comments. Comment at: lib/Parse/ParseTemplate.cpp:702 +ReportStorageClass(DS.getStorageClassSpecLoc()); + if (DS.getThreadStorageClassSpec() != DeclSpec::TSCS_unspecified) +ReportStorageClass(DS.getThreadStorageClassSpecLoc()); You added tests for `SCS_*` but not for `TSCS_*`. Is it worth adding at least one for the latter as well? ```lang=cpp template struct Z2; // expected-error {{storage class specified for template parameter 'X'}} ``` https://reviews.llvm.org/D40705 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
ilya-biryukov updated this revision to Diff 126317. ilya-biryukov added a comment. - Return `const Type*` instead of `Type*` in map getters. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 Files: clangd/CMakeLists.txt clangd/Context.cpp clangd/Context.h clangd/TypedValueMap.h unittests/clangd/CMakeLists.txt unittests/clangd/ContextTests.cpp Index: unittests/clangd/ContextTests.cpp === --- /dev/null +++ unittests/clangd/ContextTests.cpp @@ -0,0 +1,71 @@ +//===-- ContextTests.cpp - Context tests *- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "Context.h" + +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { + +TEST(TypedValueMapTests, Simple) { + Key IntParam; + Key ExtraIntParam; + + clangd::TypedValueMap Ctx; + + ASSERT_TRUE(Ctx.emplace(IntParam, 10)); + ASSERT_TRUE(Ctx.emplace(ExtraIntParam, 20)); + + EXPECT_EQ(*Ctx.get(IntParam), 10); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); + + ASSERT_FALSE(Ctx.emplace(IntParam, 30)); + + ASSERT_TRUE(Ctx.remove(IntParam)); + EXPECT_EQ(Ctx.get(IntParam), nullptr); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); + + ASSERT_TRUE(Ctx.emplace(IntParam, 30)); + EXPECT_EQ(*Ctx.get(IntParam), 30); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); +} + +TEST(TypedValueMapTests, MoveOps) { + Key> Param; + + clangd::TypedValueMap Ctx; + Ctx.emplace(Param, llvm::make_unique(10)); + EXPECT_EQ(**Ctx.get(Param), 10); + + clangd::TypedValueMap NewCtx = std::move(Ctx); + EXPECT_EQ(**NewCtx.get(Param), 10); +} + +TEST(ContextTests, Builders) { + Key ParentParam; + Key ParentAndChildParam; + Key ChildParam; + + Context ParentCtx = + buildCtx().add(ParentParam, 10).add(ParentAndChildParam, 20); + Context ChildCtx = + ParentCtx.derive().add(ParentAndChildParam, 30).add(ChildParam, 40); + + EXPECT_EQ(*ParentCtx.get(ParentParam), 10); + EXPECT_EQ(*ParentCtx.get(ParentAndChildParam), 20); + EXPECT_EQ(ParentCtx.get(ChildParam), nullptr); + + EXPECT_EQ(*ChildCtx.get(ParentParam), 10); + EXPECT_EQ(*ChildCtx.get(ParentAndChildParam), 30); + EXPECT_EQ(*ChildCtx.get(ChildParam), 40); +} + +} // namespace clangd +} // namespace clang Index: unittests/clangd/CMakeLists.txt === --- unittests/clangd/CMakeLists.txt +++ unittests/clangd/CMakeLists.txt @@ -11,6 +11,7 @@ add_extra_unittest(ClangdTests ClangdTests.cpp CodeCompleteTests.cpp + ContextTests.cpp FuzzyMatchTests.cpp JSONExprTests.cpp TestFS.cpp Index: clangd/TypedValueMap.h === --- /dev/null +++ clangd/TypedValueMap.h @@ -0,0 +1,104 @@ +//===--- TypedValueMap.h - Type-safe heterogenous key-value map -*- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// Type-safe heterogenous map. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TYPEDVALUEMAP_H_ +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TYPEDVALUEMAP_H_ + +#include "llvm/ADT/DenseMap.h" +#include + +namespace clang { +namespace clangd { + +/// Used as identity for map values. Non-movable and non-copyable. Address of +/// this object is used internally to as keys in a map. +template class Key { +public: + static_assert(!std::is_reference::value, +"Reference arguments to Key<> are not allowed"); + + Key() = default; + + Key(Key const &) = delete; + Key &operator=(Key const &) = delete; + Key(Key &&) = delete; + Key &operator=(Key &&) = delete; +}; + +/// A type-safe map from Key to T. +class TypedValueMap { +public: + TypedValueMap() = default; + TypedValueMap(const TypedValueMap &) = delete; + TypedValueMap(TypedValueMap &&) = default; + + template const Type *get(const Key &Key) const { +return const_cast(this)->get(Key); + } + + template Type *get(const Key &Key) { +auto It = Map.find(&Key); +if (It == Map.end()) + return nullptr; +return static_cast(It->second->getValuePtr()); + } + + template + bool emplace(const Key &Key, Args &&... As) { +bool Added = +Map.try_emplace(&Key, +llvm::make_unique< +TypedAnyStorage::type>>( +std::forward(As)...)) +.second; +return Added; + } + + template bool remove(Key &Key) { +auto It = Map.find(&Key); +if (It == Map.end()) + r
[PATCH] D40489: [clangd] Changed tracing interfaces
ilya-biryukov updated this revision to Diff 126318. ilya-biryukov added a comment. - Update the patch to accomodate changes from tracing and Context patches. - Updated tracing after changes to Context. We now store a clone() of Context in Span instead of ContextData. - Pass Context by const-ref instead of by ref. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40489 Files: clangd/JSONRPCDispatcher.cpp clangd/Trace.cpp clangd/Trace.h Index: clangd/Trace.h === --- clangd/Trace.h +++ clangd/Trace.h @@ -28,12 +28,21 @@ namespace trace { /// A consumer of trace events. The events are produced by Spans and trace::log. +/// Calls to begin_event and end_event with the same name are always properly +/// nested by using a RAII object (Span). +/// Implmentations of this interface must be thread-safe. class EventTracer { public: virtual ~EventTracer() = default; - /// Consume a trace event. - virtual void event(const ContextData &Ctx, llvm::StringRef Phase, - json::obj &&Contents) = 0; + + /// Called when event with \p Name starts. + virtual void begin_event(const Context &Ctx, llvm::StringRef Name) = 0; + /// Called when event with \p Name ends. + virtual void end_event(const Context &Ctx, llvm::StringRef Name, + json::obj &&Args) = 0; + /// Called for instant events. + virtual void instant_event(const Context &Ctx, llvm::StringRef Name, + json::obj &&Args) = 0; }; /// Sets up a global EventTracer that consumes events produced by Span and @@ -57,7 +66,7 @@ bool Pretty = false); /// Records a single instant event, associated with the current thread. -void log(Context &Ctx, const llvm::Twine &Name); +void log(const Context &Ctx, const llvm::Twine &Name); /// Records an event whose duration is the lifetime of the Span object. /// This is the main public interface for producing tracing events. @@ -67,15 +76,16 @@ /// SomeJSONExpr is evaluated and copied only if actually needed. class Span { public: - Span(Context &Ctx, std::string Name); + Span(const Context &Ctx, llvm::StringRef Name); ~Span(); /// Returns mutable span metadata if this span is interested. /// Prefer to use SPAN_ATTACH rather than accessing this directly. json::obj *args() { return Args.get(); } private: - const ContextData &Ctx; + llvm::Optional Ctx; + std::string Name; std::unique_ptr Args; }; Index: clangd/Trace.cpp === --- clangd/Trace.cpp +++ clangd/Trace.cpp @@ -44,10 +44,23 @@ Out.flush(); } + void begin_event(const Context &Ctx, llvm::StringRef Name) override { +jsonEvent("B", json::obj{{"name", Name}}); + } + + void end_event(const Context &Ctx, llvm::StringRef Name, + json::obj &&Args) override { +jsonEvent("E", json::obj{{"args", std::move(Args)}}); + } + + void instant_event(const Context &Ctx, llvm::StringRef Name, + json::obj &&Args) override { +jsonEvent("i", json::obj{{"name", Name}, {"args", std::move(Args)}}); + } + // Record an event on the current thread. ph, pid, tid, ts are set. // Contents must be a list of the other JSON key/values. - void event(const ContextData &Ctx, StringRef Phase, - json::obj &&Contents) override { + void jsonEvent(StringRef Phase, json::obj &&Contents) { uint64_t TID = get_threadid(); std::lock_guard Lock(Mu); // If we haven't already, emit metadata describing this thread. @@ -106,30 +119,27 @@ return llvm::make_unique(OS, Pretty); } -void log(Context &Ctx, const Twine &Message) { +void log(const Context &Ctx, const Twine &Message) { if (!T) return; - T->event(*Ctx, "i", - json::obj{ - {"name", "Log"}, - {"args", json::obj{{"Message", Message.str()}}}, - }); + T->instant_event(Ctx, "Log", json::obj{{"Message", Message.str()}}); } -Span::Span(Context &Ctx, std::string Name) : Ctx(*Ctx) { +Span::Span(const Context &Ctx, llvm::StringRef Name) : Name(Name) { if (!T) return; - T->event(this->Ctx, "B", json::obj{{"name", std::move(Name)}}); + // Clone the context, so that the original Context can be moved. + this->Ctx.emplace(Ctx.clone()); + + T->begin_event(*this->Ctx, this->Name); Args = llvm::make_unique(); } Span::~Span() { if (!T) return; - if (!Args) -Args = llvm::make_unique(); - T->event(Ctx, "E", - Args ? json::obj{{"args", std::move(*Args)}} : json::obj{}); + assert(Args && "Args can't be null at this point"); + T->end_event(*this->Ctx, Name, std::move(*Args)); } } // namespace trace Index: clangd/JSONRPCDispatcher.cpp === --- clangd/JSONRPCDispatcher.cpp +++ clangd/JSONRPCDispatcher.cpp @@ -60,14 +60,1
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
ilya-biryukov updated this revision to Diff 126319. ilya-biryukov marked 4 inline comments as done. ilya-biryukov added a comment. - Added a comment about the Parent vs Data lifetimes. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 Files: clangd/CMakeLists.txt clangd/Context.cpp clangd/Context.h clangd/TypedValueMap.h unittests/clangd/CMakeLists.txt unittests/clangd/ContextTests.cpp Index: unittests/clangd/ContextTests.cpp === --- /dev/null +++ unittests/clangd/ContextTests.cpp @@ -0,0 +1,71 @@ +//===-- ContextTests.cpp - Context tests *- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "Context.h" + +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { + +TEST(TypedValueMapTests, Simple) { + Key IntParam; + Key ExtraIntParam; + + clangd::TypedValueMap Ctx; + + ASSERT_TRUE(Ctx.emplace(IntParam, 10)); + ASSERT_TRUE(Ctx.emplace(ExtraIntParam, 20)); + + EXPECT_EQ(*Ctx.get(IntParam), 10); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); + + ASSERT_FALSE(Ctx.emplace(IntParam, 30)); + + ASSERT_TRUE(Ctx.remove(IntParam)); + EXPECT_EQ(Ctx.get(IntParam), nullptr); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); + + ASSERT_TRUE(Ctx.emplace(IntParam, 30)); + EXPECT_EQ(*Ctx.get(IntParam), 30); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); +} + +TEST(TypedValueMapTests, MoveOps) { + Key> Param; + + clangd::TypedValueMap Ctx; + Ctx.emplace(Param, llvm::make_unique(10)); + EXPECT_EQ(**Ctx.get(Param), 10); + + clangd::TypedValueMap NewCtx = std::move(Ctx); + EXPECT_EQ(**NewCtx.get(Param), 10); +} + +TEST(ContextTests, Builders) { + Key ParentParam; + Key ParentAndChildParam; + Key ChildParam; + + Context ParentCtx = + buildCtx().add(ParentParam, 10).add(ParentAndChildParam, 20); + Context ChildCtx = + ParentCtx.derive().add(ParentAndChildParam, 30).add(ChildParam, 40); + + EXPECT_EQ(*ParentCtx.get(ParentParam), 10); + EXPECT_EQ(*ParentCtx.get(ParentAndChildParam), 20); + EXPECT_EQ(ParentCtx.get(ChildParam), nullptr); + + EXPECT_EQ(*ChildCtx.get(ParentParam), 10); + EXPECT_EQ(*ChildCtx.get(ParentAndChildParam), 30); + EXPECT_EQ(*ChildCtx.get(ChildParam), 40); +} + +} // namespace clangd +} // namespace clang Index: unittests/clangd/CMakeLists.txt === --- unittests/clangd/CMakeLists.txt +++ unittests/clangd/CMakeLists.txt @@ -11,6 +11,7 @@ add_extra_unittest(ClangdTests ClangdTests.cpp CodeCompleteTests.cpp + ContextTests.cpp FuzzyMatchTests.cpp JSONExprTests.cpp TestFS.cpp Index: clangd/TypedValueMap.h === --- /dev/null +++ clangd/TypedValueMap.h @@ -0,0 +1,104 @@ +//===--- TypedValueMap.h - Type-safe heterogenous key-value map -*- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// Type-safe heterogenous map. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TYPEDVALUEMAP_H_ +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TYPEDVALUEMAP_H_ + +#include "llvm/ADT/DenseMap.h" +#include + +namespace clang { +namespace clangd { + +/// Used as identity for map values. Non-movable and non-copyable. Address of +/// this object is used internally to as keys in a map. +template class Key { +public: + static_assert(!std::is_reference::value, +"Reference arguments to Key<> are not allowed"); + + Key() = default; + + Key(Key const &) = delete; + Key &operator=(Key const &) = delete; + Key(Key &&) = delete; + Key &operator=(Key &&) = delete; +}; + +/// A type-safe map from Key to T. +class TypedValueMap { +public: + TypedValueMap() = default; + TypedValueMap(const TypedValueMap &) = delete; + TypedValueMap(TypedValueMap &&) = default; + + template const Type *get(const Key &Key) const { +return const_cast(this)->get(Key); + } + + template Type *get(const Key &Key) { +auto It = Map.find(&Key); +if (It == Map.end()) + return nullptr; +return static_cast(It->second->getValuePtr()); + } + + template + bool emplace(const Key &Key, Args &&... As) { +bool Added = +Map.try_emplace(&Key, +llvm::make_unique< +TypedAnyStorage::type>>( +std::forward(As)...)) +.second; +return Added; + } + + template bool remove(Key &Key) { +auto It = Map.fi
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
ilya-biryukov updated this revision to Diff 126320. ilya-biryukov added a comment. - Rephrase the comment. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 Files: clangd/CMakeLists.txt clangd/Context.cpp clangd/Context.h clangd/TypedValueMap.h unittests/clangd/CMakeLists.txt unittests/clangd/ContextTests.cpp Index: unittests/clangd/ContextTests.cpp === --- /dev/null +++ unittests/clangd/ContextTests.cpp @@ -0,0 +1,71 @@ +//===-- ContextTests.cpp - Context tests *- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "Context.h" + +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { + +TEST(TypedValueMapTests, Simple) { + Key IntParam; + Key ExtraIntParam; + + clangd::TypedValueMap Ctx; + + ASSERT_TRUE(Ctx.emplace(IntParam, 10)); + ASSERT_TRUE(Ctx.emplace(ExtraIntParam, 20)); + + EXPECT_EQ(*Ctx.get(IntParam), 10); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); + + ASSERT_FALSE(Ctx.emplace(IntParam, 30)); + + ASSERT_TRUE(Ctx.remove(IntParam)); + EXPECT_EQ(Ctx.get(IntParam), nullptr); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); + + ASSERT_TRUE(Ctx.emplace(IntParam, 30)); + EXPECT_EQ(*Ctx.get(IntParam), 30); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); +} + +TEST(TypedValueMapTests, MoveOps) { + Key> Param; + + clangd::TypedValueMap Ctx; + Ctx.emplace(Param, llvm::make_unique(10)); + EXPECT_EQ(**Ctx.get(Param), 10); + + clangd::TypedValueMap NewCtx = std::move(Ctx); + EXPECT_EQ(**NewCtx.get(Param), 10); +} + +TEST(ContextTests, Builders) { + Key ParentParam; + Key ParentAndChildParam; + Key ChildParam; + + Context ParentCtx = + buildCtx().add(ParentParam, 10).add(ParentAndChildParam, 20); + Context ChildCtx = + ParentCtx.derive().add(ParentAndChildParam, 30).add(ChildParam, 40); + + EXPECT_EQ(*ParentCtx.get(ParentParam), 10); + EXPECT_EQ(*ParentCtx.get(ParentAndChildParam), 20); + EXPECT_EQ(ParentCtx.get(ChildParam), nullptr); + + EXPECT_EQ(*ChildCtx.get(ParentParam), 10); + EXPECT_EQ(*ChildCtx.get(ParentAndChildParam), 30); + EXPECT_EQ(*ChildCtx.get(ChildParam), 40); +} + +} // namespace clangd +} // namespace clang Index: unittests/clangd/CMakeLists.txt === --- unittests/clangd/CMakeLists.txt +++ unittests/clangd/CMakeLists.txt @@ -11,6 +11,7 @@ add_extra_unittest(ClangdTests ClangdTests.cpp CodeCompleteTests.cpp + ContextTests.cpp FuzzyMatchTests.cpp JSONExprTests.cpp TestFS.cpp Index: clangd/TypedValueMap.h === --- /dev/null +++ clangd/TypedValueMap.h @@ -0,0 +1,104 @@ +//===--- TypedValueMap.h - Type-safe heterogenous key-value map -*- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// Type-safe heterogenous map. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TYPEDVALUEMAP_H_ +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TYPEDVALUEMAP_H_ + +#include "llvm/ADT/DenseMap.h" +#include + +namespace clang { +namespace clangd { + +/// Used as identity for map values. Non-movable and non-copyable. Address of +/// this object is used internally to as keys in a map. +template class Key { +public: + static_assert(!std::is_reference::value, +"Reference arguments to Key<> are not allowed"); + + Key() = default; + + Key(Key const &) = delete; + Key &operator=(Key const &) = delete; + Key(Key &&) = delete; + Key &operator=(Key &&) = delete; +}; + +/// A type-safe map from Key to T. +class TypedValueMap { +public: + TypedValueMap() = default; + TypedValueMap(const TypedValueMap &) = delete; + TypedValueMap(TypedValueMap &&) = default; + + template const Type *get(const Key &Key) const { +return const_cast(this)->get(Key); + } + + template Type *get(const Key &Key) { +auto It = Map.find(&Key); +if (It == Map.end()) + return nullptr; +return static_cast(It->second->getValuePtr()); + } + + template + bool emplace(const Key &Key, Args &&... As) { +bool Added = +Map.try_emplace(&Key, +llvm::make_unique< +TypedAnyStorage::type>>( +std::forward(As)...)) +.second; +return Added; + } + + template bool remove(Key &Key) { +auto It = Map.find(&Key); +if (It == Map.end()) + return false; + +Map.erase(It);
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
ilya-biryukov updated this revision to Diff 126321. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Added r-value overload for derive(). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 Files: clangd/CMakeLists.txt clangd/Context.cpp clangd/Context.h clangd/TypedValueMap.h unittests/clangd/CMakeLists.txt unittests/clangd/ContextTests.cpp Index: unittests/clangd/ContextTests.cpp === --- /dev/null +++ unittests/clangd/ContextTests.cpp @@ -0,0 +1,71 @@ +//===-- ContextTests.cpp - Context tests *- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "Context.h" + +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { + +TEST(TypedValueMapTests, Simple) { + Key IntParam; + Key ExtraIntParam; + + clangd::TypedValueMap Ctx; + + ASSERT_TRUE(Ctx.emplace(IntParam, 10)); + ASSERT_TRUE(Ctx.emplace(ExtraIntParam, 20)); + + EXPECT_EQ(*Ctx.get(IntParam), 10); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); + + ASSERT_FALSE(Ctx.emplace(IntParam, 30)); + + ASSERT_TRUE(Ctx.remove(IntParam)); + EXPECT_EQ(Ctx.get(IntParam), nullptr); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); + + ASSERT_TRUE(Ctx.emplace(IntParam, 30)); + EXPECT_EQ(*Ctx.get(IntParam), 30); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); +} + +TEST(TypedValueMapTests, MoveOps) { + Key> Param; + + clangd::TypedValueMap Ctx; + Ctx.emplace(Param, llvm::make_unique(10)); + EXPECT_EQ(**Ctx.get(Param), 10); + + clangd::TypedValueMap NewCtx = std::move(Ctx); + EXPECT_EQ(**NewCtx.get(Param), 10); +} + +TEST(ContextTests, Builders) { + Key ParentParam; + Key ParentAndChildParam; + Key ChildParam; + + Context ParentCtx = + buildCtx().add(ParentParam, 10).add(ParentAndChildParam, 20); + Context ChildCtx = + ParentCtx.derive().add(ParentAndChildParam, 30).add(ChildParam, 40); + + EXPECT_EQ(*ParentCtx.get(ParentParam), 10); + EXPECT_EQ(*ParentCtx.get(ParentAndChildParam), 20); + EXPECT_EQ(ParentCtx.get(ChildParam), nullptr); + + EXPECT_EQ(*ChildCtx.get(ParentParam), 10); + EXPECT_EQ(*ChildCtx.get(ParentAndChildParam), 30); + EXPECT_EQ(*ChildCtx.get(ChildParam), 40); +} + +} // namespace clangd +} // namespace clang Index: unittests/clangd/CMakeLists.txt === --- unittests/clangd/CMakeLists.txt +++ unittests/clangd/CMakeLists.txt @@ -11,6 +11,7 @@ add_extra_unittest(ClangdTests ClangdTests.cpp CodeCompleteTests.cpp + ContextTests.cpp FuzzyMatchTests.cpp JSONExprTests.cpp TestFS.cpp Index: clangd/TypedValueMap.h === --- /dev/null +++ clangd/TypedValueMap.h @@ -0,0 +1,104 @@ +//===--- TypedValueMap.h - Type-safe heterogenous key-value map -*- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// Type-safe heterogenous map. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TYPEDVALUEMAP_H_ +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TYPEDVALUEMAP_H_ + +#include "llvm/ADT/DenseMap.h" +#include + +namespace clang { +namespace clangd { + +/// Used as identity for map values. Non-movable and non-copyable. Address of +/// this object is used internally to as keys in a map. +template class Key { +public: + static_assert(!std::is_reference::value, +"Reference arguments to Key<> are not allowed"); + + Key() = default; + + Key(Key const &) = delete; + Key &operator=(Key const &) = delete; + Key(Key &&) = delete; + Key &operator=(Key &&) = delete; +}; + +/// A type-safe map from Key to T. +class TypedValueMap { +public: + TypedValueMap() = default; + TypedValueMap(const TypedValueMap &) = delete; + TypedValueMap(TypedValueMap &&) = default; + + template const Type *get(const Key &Key) const { +return const_cast(this)->get(Key); + } + + template Type *get(const Key &Key) { +auto It = Map.find(&Key); +if (It == Map.end()) + return nullptr; +return static_cast(It->second->getValuePtr()); + } + + template + bool emplace(const Key &Key, Args &&... As) { +bool Added = +Map.try_emplace(&Key, +llvm::make_unique< +TypedAnyStorage::type>>( +std::forward(As)...)) +.second; +return Added; + } + + template bool remove(Key &Key) { +auto It = Map.find(&Key); +
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
ilya-biryukov updated this revision to Diff 126322. ilya-biryukov added a comment. - Replaced emptyCtx with Context::empty Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 Files: clangd/CMakeLists.txt clangd/Context.cpp clangd/Context.h clangd/TypedValueMap.h unittests/clangd/CMakeLists.txt unittests/clangd/ContextTests.cpp Index: unittests/clangd/ContextTests.cpp === --- /dev/null +++ unittests/clangd/ContextTests.cpp @@ -0,0 +1,71 @@ +//===-- ContextTests.cpp - Context tests *- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "Context.h" + +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { + +TEST(TypedValueMapTests, Simple) { + Key IntParam; + Key ExtraIntParam; + + clangd::TypedValueMap Ctx; + + ASSERT_TRUE(Ctx.emplace(IntParam, 10)); + ASSERT_TRUE(Ctx.emplace(ExtraIntParam, 20)); + + EXPECT_EQ(*Ctx.get(IntParam), 10); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); + + ASSERT_FALSE(Ctx.emplace(IntParam, 30)); + + ASSERT_TRUE(Ctx.remove(IntParam)); + EXPECT_EQ(Ctx.get(IntParam), nullptr); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); + + ASSERT_TRUE(Ctx.emplace(IntParam, 30)); + EXPECT_EQ(*Ctx.get(IntParam), 30); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); +} + +TEST(TypedValueMapTests, MoveOps) { + Key> Param; + + clangd::TypedValueMap Ctx; + Ctx.emplace(Param, llvm::make_unique(10)); + EXPECT_EQ(**Ctx.get(Param), 10); + + clangd::TypedValueMap NewCtx = std::move(Ctx); + EXPECT_EQ(**NewCtx.get(Param), 10); +} + +TEST(ContextTests, Builders) { + Key ParentParam; + Key ParentAndChildParam; + Key ChildParam; + + Context ParentCtx = + buildCtx().add(ParentParam, 10).add(ParentAndChildParam, 20); + Context ChildCtx = + ParentCtx.derive().add(ParentAndChildParam, 30).add(ChildParam, 40); + + EXPECT_EQ(*ParentCtx.get(ParentParam), 10); + EXPECT_EQ(*ParentCtx.get(ParentAndChildParam), 20); + EXPECT_EQ(ParentCtx.get(ChildParam), nullptr); + + EXPECT_EQ(*ChildCtx.get(ParentParam), 10); + EXPECT_EQ(*ChildCtx.get(ParentAndChildParam), 30); + EXPECT_EQ(*ChildCtx.get(ChildParam), 40); +} + +} // namespace clangd +} // namespace clang Index: unittests/clangd/CMakeLists.txt === --- unittests/clangd/CMakeLists.txt +++ unittests/clangd/CMakeLists.txt @@ -11,6 +11,7 @@ add_extra_unittest(ClangdTests ClangdTests.cpp CodeCompleteTests.cpp + ContextTests.cpp FuzzyMatchTests.cpp JSONExprTests.cpp TestFS.cpp Index: clangd/TypedValueMap.h === --- /dev/null +++ clangd/TypedValueMap.h @@ -0,0 +1,104 @@ +//===--- TypedValueMap.h - Type-safe heterogenous key-value map -*- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// Type-safe heterogenous map. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TYPEDVALUEMAP_H_ +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TYPEDVALUEMAP_H_ + +#include "llvm/ADT/DenseMap.h" +#include + +namespace clang { +namespace clangd { + +/// Used as identity for map values. Non-movable and non-copyable. Address of +/// this object is used internally to as keys in a map. +template class Key { +public: + static_assert(!std::is_reference::value, +"Reference arguments to Key<> are not allowed"); + + Key() = default; + + Key(Key const &) = delete; + Key &operator=(Key const &) = delete; + Key(Key &&) = delete; + Key &operator=(Key &&) = delete; +}; + +/// A type-safe map from Key to T. +class TypedValueMap { +public: + TypedValueMap() = default; + TypedValueMap(const TypedValueMap &) = delete; + TypedValueMap(TypedValueMap &&) = default; + + template const Type *get(const Key &Key) const { +return const_cast(this)->get(Key); + } + + template Type *get(const Key &Key) { +auto It = Map.find(&Key); +if (It == Map.end()) + return nullptr; +return static_cast(It->second->getValuePtr()); + } + + template + bool emplace(const Key &Key, Args &&... As) { +bool Added = +Map.try_emplace(&Key, +llvm::make_unique< +TypedAnyStorage::type>>( +std::forward(As)...)) +.second; +return Added; + } + + template bool remove(Key &Key) { +auto It = Map.find(&Key); +if (It == Map.end()) + return false; + +
[PATCH] D40415: [libcxx] Define istream_iterator equality comparison operators out-of-line
rogfer01 added a comment. Hi @miyuki I can commit it. https://reviews.llvm.org/D40415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40715: [analyser] different.LabelInsideSwitch checker implementation
alexey.knyshev added inline comments. Comment at: lib/StaticAnalyzer/Checkers/LabelInsideSwitchChecker.cpp:87 +BugReporter &BR) const { + auto LabelStmt = stmt(hasDescendant(switchStmt( + eachOf(has(compoundStmt(forEach(labelStmt().bind("label", a.sidorin wrote: > alexey.knyshev wrote: > > Looks like I have to use `forEachDescendant` instead of `hasDescendant`. > > Please, comment! > 1. Yes, `hasDescendant()` will give you only single match. > `forEachDescendant()` will continue matching after match found and that is > what we should do here. > 2. Maybe we can just use > stmt(forEachDescendant(switchStmt(forEachDescendant(labelStmt()? We don't > distinguish "label" and "label_in_case" anyway. Also, current matcher will > ignore deeply-nested switches: > ``` > switch (x) } > case 1: { > {{ label: }} // ignored > } > } > ``` > Is that intentional? 1. Thanks, got it. 2. I've used `eachOf` for matching 2 different cases: - Label under case stmt such as: ``` switch (x) { case 1: someCodeHere; cas: // missprint ... // other cases } ``` - And label under switch compound stmt: ``` switch (x) { cas: blabla; // this label is not a child of any case stmt case 3: ... ... // etc ``` On the other hand I would like to avoid matching deeply-nested labels inside switch compound stmt for reducing false-positives probability. Actually I'm not sure in my assumption. Repository: rC Clang https://reviews.llvm.org/D40715 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40415: [libcxx] Define istream_iterator equality comparison operators out-of-line
miyuki added a comment. In https://reviews.llvm.org/D40415#950793, @rogfer01 wrote: > Hi @miyuki I can commit it. Please do so. https://reviews.llvm.org/D40415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40746: Correctly handle line table entries without filenames during AST serialization
yvvan added a comment. Can we still have it in 5.0? Repository: rL LLVM https://reviews.llvm.org/D40746 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
ilya-biryukov added inline comments. Comment at: clangd/Context.h:11 +// Context for storing and retrieving implicit data. Useful for passing implicit +// parameters on a per-request basis. +// sammccall wrote: > This could use a bit more I think, e.g. > > A context is an immutable container for per-request data that must be > propagated through layers that don't care about it. > An example is a request ID that we may want to use when logging. > > Conceptually, a context is a heterogeneous map, T>. Each key has > an associated value type, which allows the map to be typesafe. > > You can't add data to an existing context, instead you create a new > immutable context derived from it with extra data added. When you retrieve > data, the context will walk up the parent chain until the key is found. > > Contexts should be: > - passed by reference when calling synchronous functions > - passed by value (move) when calling asynchronous functions. The > callback will receive the context again. > - copied only when 'forking' an asynchronous computation that we don't > wait for > > Some of this is on the class comment - this seems fine but the Context class > should then be the first thing in the file. Done. It's in the class comment, but class is now the first thing in the file. Thanks for making my life a bit easier and coming up with a doc :-) Comment at: clangd/Context.h:25 + +class ContextData { +public: ilya-biryukov wrote: > sammccall wrote: > > IIUC, the only reason we expose separate `Context` and `ContextData` types > > is to give things like Span a stable reference to hold onto > > (`ContextData*`). > > > > Could we use `Context` instead (a copy)? Then we incur `shared_ptr` > > overhead for span creation, but greatly simplify the interface. > > If we want to avoid the overhead, the Span could maybe grab whatever it > > needs out of the context at construction time, for use in the destructor. > > > > Either way, we should make sure `Context` is front-and-center in this > > header, and I hope we can hide ContextData entirely. > I have considered making `Context` the only used interface, but I hated the > fact that each `Span` constructor would copy a `shared_ptr`. (And `Span`s > can be pretty ubiquitous). > On the other hand, that should not happen when tracing is off, so we won't be > adding overhead when `Span` are not used. > Will try moving `ContextData` into `.cpp` file, this should certainly be an > implementation detail. And we can expose it later if we'll actually have a > use-case for that (i.e. not copying `shared_ptr`s). Done. We now store a copied Context in the Spans. Comment at: clangd/Context.h:32 + /// specified for \p Key, return null. + template Type *get(Key &Key) const { +if (auto Val = Data.get(Key)) sammccall wrote: > sammccall wrote: > > nit: const Key&, and below > this isn't usually how we define const-correctness for containers. > A const method shouldn't return a mutable reference to the contained object. Returning const references now. Comment at: clangd/Context.h:42 + /// Must not be called for keys that are not in the map. + template Type &getExisting(Key &Key) const { +auto Val = get(Key); sammccall wrote: > I'd prefer the standard name `at` for this function (assert vs throw being > pretty minor I think), but YMMV We've discussed this offline, but just for the record: I kinda like that `getExisting` is named after `get`. Also `at` usually throws, whereas our function only `asserts` (even though LLVM does not use execptions, we still have undefined behaviour vs crash in runtime). Comment at: clangd/Context.h:139 +/// that require a Context when no explicit Context is not available. +Context &emptyCtx(); + sammccall wrote: > sammccall wrote: > > nit: how do you feel about Context::empty()? it ties the name to the type > > more clearly, I think. > OOC: why reference and not a value? > nit: how do you feel about Context::empty()? it ties the name to the type > more clearly, I think. Done. > OOC: why reference and not a value? Reference gives us just the right semantics with less overhead (no extra shared_ptr copies) when we need to pass empty context by reference (e.g., to sync functions like `log`). Comment at: clangd/Context.h:142 +/// Creates a ContextBuilder with a null parent. +ContextBuilder buildCtx(); + sammccall wrote: > I think we should spell this `emptyCtx().derive()`. > It should be pretty rare to derive from the empty context in production code > - and conceptually it's no different than any other use of the empty context, > I think. I'd argue the separate function is more readable and saves us an extra lookup in the empty context for missing keys. Given that `emptyC
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
ilya-biryukov added inline comments. Comment at: clangd/Context.h:65 + Context *Parent; + TypedValueMap Data; +}; sammccall wrote: > ilya-biryukov wrote: > > sammccall wrote: > > > We add complexity here (implementation and conceptual) to allow multiple > > > properties to be set at the same level (vs having a key and an AnyStorage > > > and making Context a linked list). > > > Is this for performance? I'm not convinced it'll actually be faster for > > > our workloads, or that it matters. > > Conceptually, a `Context` is more convenient to use when it stores multiple > > values. This allows to put a bunch of things and assign meaning to > > `Context` (i.e., a `Context` for processing a single LSP request, global > > context). If `Context`s were a linked list, the intermediate `Context`s > > would be hard to assign the meaning to. > > > > That being said, storage strategy for `Context`s is an implementation > > detail and could be changed anytime. I don't have big preferences here, but > > I think that storing a linked list of maps has, in general, a better > > performance than storing a linked list. > > And given that it's already there, I'd leave it this way. > With the new shared_ptr semantics: > > Context D = move(C).derive(K1, V1).derive(K2, V2); > > Is just as meaningful as > > Context D = move(C).derive().add(K1, V1).add(K2, V2); > > Yeah, the list of maps in an implementation detail. It's one that comes with > a bunch of complexity (`ContextBuilder` and most of `TypedValueMap`). It > really doesn't seem to buy us anything (the performance is both uninteresting > and seems likely to be worse in this specific case with very few entries). The thing I like about it is that the `Context`s are layered properly in a sense that there's a Context corresponding to the request, a Context corresponding to the forked subrequests, etc. If we change the interface, we'll be creating a bunch of temporary Contexts that don't correspond to a nice meaningful abstraction (like request) in my head, even though we don't give those contexts any names. I do agree we currently pay with some complexity for that. Though I'd argue it's all hidden from the users of the interface, as building and consuming contexts is still super-easy and you don't need to mention ContextBuilder or TypedValueMap. And the implementation complexity is totally manageable from my point of view, but I am the one who implemented it in the first place, so there's certainly a bias there. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40705: [Parser] Diagnose storage classes in template parameter declarations
miyuki updated this revision to Diff 126325. miyuki added a comment. Added a test for thead_local. https://reviews.llvm.org/D40705 Files: include/clang/Basic/DiagnosticParseKinds.td lib/Parse/ParseTemplate.cpp test/CXX/temp/temp.param/p2-cpp11.cpp test/CXX/temp/temp.param/p2.cpp Index: test/CXX/temp/temp.param/p2.cpp === --- test/CXX/temp/temp.param/p2.cpp +++ test/CXX/temp/temp.param/p2.cpp @@ -1,5 +1,4 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s -// expected-no-diagnostics // There is no semantic difference between class and typename in a // template-parameter. typename followed by an unqualified-id names a @@ -13,7 +12,8 @@ template::type Value> struct Y1; // A storage class shall not be specified in a template-parameter declaration. -template struct Z; // FIXME: expect an error +template struct Z0; // expected-error {{storage class specified for template parameter 'Value'}} +template struct Z1; // expected-error {{storage class specified for template parameter}} // Make sure that we properly disambiguate non-type template parameters that // start with 'class'. Index: test/CXX/temp/temp.param/p2-cpp11.cpp === --- /dev/null +++ test/CXX/temp/temp.param/p2-cpp11.cpp @@ -0,0 +1,5 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s + +// A storage class shall not be specified in a template-parameter declaration. +template struct Z0; // expected-error {{storage class specified for template parameter 'Value'}} +template struct Z1; // expected-error {{storage class specified for template parameter}} Index: lib/Parse/ParseTemplate.cpp === --- lib/Parse/ParseTemplate.cpp +++ lib/Parse/ParseTemplate.cpp @@ -686,6 +686,22 @@ return nullptr; } + // C++ [temp.param]p2: + // A storage class shall not be specified in a template-parameter + // declaration. + auto ReportStorageClass = [this, &ParamDecl](SourceLocation Loc) { +if (ParamDecl.getIdentifier()) + Diag(Loc, diag::err_storage_class_template_parameter) +<< ParamDecl.getIdentifier() << FixItHint::CreateRemoval(Loc); +else + Diag(Loc, diag::err_storage_class_template_parameter_anon) +<< FixItHint::CreateRemoval(Loc); + }; + if (DS.getStorageClassSpec() != DeclSpec::SCS_unspecified) +ReportStorageClass(DS.getStorageClassSpecLoc()); + if (DS.getThreadStorageClassSpec() != DeclSpec::TSCS_unspecified) +ReportStorageClass(DS.getThreadStorageClassSpecLoc()); + // Recover from misplaced ellipsis. SourceLocation EllipsisLoc; if (TryConsumeToken(tok::ellipsis, EllipsisLoc)) Index: include/clang/Basic/DiagnosticParseKinds.td === --- include/clang/Basic/DiagnosticParseKinds.td +++ include/clang/Basic/DiagnosticParseKinds.td @@ -668,6 +668,11 @@ def warn_missing_dependent_template_keyword : ExtWarn< "use 'template' keyword to treat '%0' as a dependent template name">; +def err_storage_class_template_parameter : Error< + "storage class specified for template parameter %0">; +def err_storage_class_template_parameter_anon : Error< + "storage class specified for template parameter">; + def ext_extern_template : Extension< "extern templates are a C++11 extension">, InGroup; def warn_cxx98_compat_extern_template : Warning< Index: test/CXX/temp/temp.param/p2.cpp === --- test/CXX/temp/temp.param/p2.cpp +++ test/CXX/temp/temp.param/p2.cpp @@ -1,5 +1,4 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s -// expected-no-diagnostics // There is no semantic difference between class and typename in a // template-parameter. typename followed by an unqualified-id names a @@ -13,7 +12,8 @@ template::type Value> struct Y1; // A storage class shall not be specified in a template-parameter declaration. -template struct Z; // FIXME: expect an error +template struct Z0; // expected-error {{storage class specified for template parameter 'Value'}} +template struct Z1; // expected-error {{storage class specified for template parameter}} // Make sure that we properly disambiguate non-type template parameters that // start with 'class'. Index: test/CXX/temp/temp.param/p2-cpp11.cpp === --- /dev/null +++ test/CXX/temp/temp.param/p2-cpp11.cpp @@ -0,0 +1,5 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s + +// A storage class shall not be specified in a template-parameter declaration. +template struct Z0; // expected-error {{storage class specified for template parameter 'Value'}} +template struct Z1; // expected-error {{storage class specified for template parameter}} Index: lib/Parse/ParseTemplate.cpp === --- lib/Parse/P
[PATCH] D40486: [clangd] Implemented logging using Context
ilya-biryukov updated this revision to Diff 126327. ilya-biryukov added a comment. Udpated the patch after changes in Context Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40486 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/ClangdUnitStore.cpp clangd/ClangdUnitStore.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/GlobalCompilationDatabase.cpp clangd/GlobalCompilationDatabase.h clangd/JSONRPCDispatcher.cpp clangd/JSONRPCDispatcher.h clangd/Logger.cpp clangd/Logger.h clangd/Protocol.h clangd/ProtocolHandlers.cpp clangd/ProtocolHandlers.h clangd/tool/ClangdMain.cpp unittests/clangd/ClangdTests.cpp unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -8,6 +8,7 @@ //===--===// #include "ClangdServer.h" #include "Compiler.h" +#include "Context.h" #include "Protocol.h" #include "TestFS.h" #include "gtest/gtest.h" @@ -74,8 +75,7 @@ MockCompilationDatabase CDB; ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, - EmptyLogger::getInstance()); + /*StorePreamblesInMemory=*/true); auto FooCpp = getVirtualTestFilePath("foo.cpp"); const auto SourceContents = R"cpp( @@ -101,29 +101,33 @@ // No need to sync reparses here as there are no asserts on diagnostics (or // other async operations). - Server.addDocument(FooCpp, SourceContents); + Server.addDocument(FooCpp, SourceContents, buildCtx()); { auto CodeCompletionResults1 = -Server.codeComplete(FooCpp, CompletePos, CCOpts, None).get().Value; +Server.codeComplete(FooCpp, CompletePos, CCOpts, buildCtx(), None) +.get() +.first.Value; EXPECT_TRUE(ContainsItem(CodeCompletionResults1, "aba")); EXPECT_FALSE(ContainsItem(CodeCompletionResults1, "cbc")); } { auto CodeCompletionResultsOverriden = Server -.codeComplete(FooCpp, CompletePos, CCOpts, +.codeComplete(FooCpp, CompletePos, CCOpts, buildCtx(), StringRef(OverridenSourceContents)) .get() -.Value; +.first.Value; EXPECT_TRUE(ContainsItem(CodeCompletionResultsOverriden, "cbc")); EXPECT_FALSE(ContainsItem(CodeCompletionResultsOverriden, "aba")); } { auto CodeCompletionResults2 = -Server.codeComplete(FooCpp, CompletePos, CCOpts, None).get().Value; +Server.codeComplete(FooCpp, CompletePos, CCOpts, buildCtx(), None) +.get() +.first.Value; EXPECT_TRUE(ContainsItem(CodeCompletionResults2, "aba")); EXPECT_FALSE(ContainsItem(CodeCompletionResults2, "cbc")); } @@ -135,8 +139,7 @@ CDB.ExtraClangFlags.push_back("-xc++"); IgnoreDiagnostics DiagConsumer; ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, - EmptyLogger::getInstance()); + /*StorePreamblesInMemory=*/true); auto FooCpp = getVirtualTestFilePath("foo.cpp"); FS.Files[FooCpp] = ""; @@ -150,17 +153,17 @@ int main() { ClassWithMembers().{complete} } )cpp", "complete"); - Server.addDocument(FooCpp, Completion.Text); + Server.addDocument(FooCpp, Completion.Text, buildCtx()); clangd::CodeCompleteOptions Opts; Opts.Limit = 2; /// For after-dot completion we must always get consistent results. auto Results = Server .codeComplete(FooCpp, Completion.MarkerPos, Opts, - StringRef(Completion.Text)) + buildCtx(), StringRef(Completion.Text)) .get() - .Value; + .first.Value; EXPECT_TRUE(Results.isIncomplete); EXPECT_EQ(Opts.Limit, Results.items.size()); @@ -175,8 +178,7 @@ CDB.ExtraClangFlags.push_back("-xc++"); IgnoreDiagnostics DiagConsumer; ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, - EmptyLogger::getInstance()); + /*StorePreamblesInMemory=*/true); auto FooCpp = getVirtualTestFilePath("foo.cpp"); FS.Files[FooCpp] = ""; @@ -194,12 +196,13 @@ StringWithPos Completion = parseTextMarker( formatv("{0} int main() { {1}{{complete}} }", Body, Query).str(), "complete"); -Server.addDocument(FooCpp, Completion.Text); +Server.addDocument(FooCpp, Completion.Text, buildCtx());
[PATCH] D40488: [clangd] Implemented tracing using Context
ilya-biryukov updated this revision to Diff 126328. ilya-biryukov added a comment. Updated the patch after changes to Context Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40488 Files: clangd/ClangdUnit.cpp clangd/JSONRPCDispatcher.cpp clangd/Trace.cpp clangd/Trace.h clangd/tool/ClangdMain.cpp unittests/clangd/TraceTests.cpp Index: unittests/clangd/TraceTests.cpp === --- unittests/clangd/TraceTests.cpp +++ unittests/clangd/TraceTests.cpp @@ -7,6 +7,7 @@ // //===--===// +#include "Context.h" #include "Trace.h" #include "llvm/ADT/DenseMap.h" @@ -74,10 +75,11 @@ std::string JSON; { raw_string_ostream OS(JSON); -auto Session = trace::Session::create(OS); +auto JSONTracer = trace::createJSONTracer(OS); +trace::TracingSession Session(*JSONTracer); { - trace::Span S("A"); - trace::log("B"); + trace::Span S(Context::empty(), "A"); + trace::log(Context::empty(), "B"); } } Index: clangd/tool/ClangdMain.cpp === --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -115,19 +115,25 @@ << EC.message(); } } + + // Setup tracing facilities. llvm::Optional TraceStream; - std::unique_ptr TraceSession; + std::unique_ptr Tracer; if (!TraceFile.empty()) { std::error_code EC; TraceStream.emplace(TraceFile, /*ref*/ EC, llvm::sys::fs::F_RW); if (EC) { TraceFile.reset(); llvm::errs() << "Error while opening trace file: " << EC.message(); } else { - TraceSession = trace::Session::create(*TraceStream, PrettyPrint); + Tracer = trace::createJSONTracer(*TraceStream, PrettyPrint); } } + llvm::Optional TracingSession; + if (Tracer) +TracingSession.emplace(*Tracer); + llvm::raw_ostream &Outs = llvm::outs(); llvm::raw_ostream &Logs = llvm::errs(); JSONOutput Out(Outs, Logs, Index: clangd/Trace.h === --- clangd/Trace.h +++ clangd/Trace.h @@ -8,60 +8,74 @@ //===--===// // // Supports writing performance traces describing clangd's behavior. -// Traces are written in the Trace Event format supported by chrome's trace -// viewer (chrome://tracing). +// Traces are consumed by implementations of the EventTracer interface. // -// The format is documented here: -// https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview // // All APIs are no-ops unless a Session is active (created by ClangdMain). // //===--===// #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRACE_H_ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRACE_H_ +#include "Context.h" #include "JSONExpr.h" #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" namespace clang { namespace clangd { namespace trace { -// A session directs the output of trace events. Only one Session can exist. -// It should be created before clangd threads are spawned, and destroyed after -// they exit. -// TODO: we may want to add pluggable support for other tracing backends. -class Session { +/// A consumer of trace events. The events are produced by Spans and trace::log. +class EventTracer { public: - // Starts a sessions capturing trace events and writing Trace Event JSON. - static std::unique_ptr create(llvm::raw_ostream &OS, - bool Pretty = false); - ~Session(); + virtual ~EventTracer() = default; + /// Consume a trace event. + virtual void event(const Context &Ctx, llvm::StringRef Phase, + json::obj &&Contents) = 0; +}; -private: - Session() = default; +/// Sets up a global EventTracer that consumes events produced by Span and +/// trace::log. Only one TracingSession can be active at a time and it should be +/// set up before calling any clangd-specific functions. +class TracingSession { +public: + TracingSession(EventTracer &Tracer); + ~TracingSession(); }; -// Records a single instant event, associated with the current thread. -void log(const llvm::Twine &Name); +/// Create an instance of EventTracer that produces an output in the Trace Event +/// format supported by Chrome's trace viewer (chrome://tracing). +/// +/// The format is documented here: +/// https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview +/// +/// The implementation supports concurrent calls and can be used as a global +/// tracer (i.e., can be put into a global Context). +std::unique_ptr createJSONTracer(llvm::raw_ostream &OS, + bool Pretty = false); -// Records an event whose duration is the lifetime of the Span object. -// -/
[PATCH] D40489: [clangd] Changed tracing interfaces
ilya-biryukov updated this revision to Diff 126329. ilya-biryukov added a comment. Updated the patch after changes to Context Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40489 Files: clangd/Trace.cpp clangd/Trace.h Index: clangd/Trace.h === --- clangd/Trace.h +++ clangd/Trace.h @@ -28,12 +28,21 @@ namespace trace { /// A consumer of trace events. The events are produced by Spans and trace::log. +/// Calls to begin_event and end_event with the same name are always properly +/// nested by using a RAII object (Span). +/// Implmentations of this interface must be thread-safe. class EventTracer { public: virtual ~EventTracer() = default; - /// Consume a trace event. - virtual void event(const Context &Ctx, llvm::StringRef Phase, - json::obj &&Contents) = 0; + + /// Called when event with \p Name starts. + virtual void begin_event(const Context &Ctx, llvm::StringRef Name) = 0; + /// Called when event with \p Name ends. + virtual void end_event(const Context &Ctx, llvm::StringRef Name, + json::obj &&Args) = 0; + /// Called for instant events. + virtual void instant_event(const Context &Ctx, llvm::StringRef Name, + json::obj &&Args) = 0; }; /// Sets up a global EventTracer that consumes events produced by Span and @@ -67,15 +76,16 @@ /// SomeJSONExpr is evaluated and copied only if actually needed. class Span { public: - Span(const Context &Ctx, std::string Name); + Span(const Context &Ctx, llvm::StringRef Name); ~Span(); /// Returns mutable span metadata if this span is interested. /// Prefer to use SPAN_ATTACH rather than accessing this directly. json::obj *args() { return Args.get(); } private: llvm::Optional Ctx; + std::string Name; std::unique_ptr Args; }; Index: clangd/Trace.cpp === --- clangd/Trace.cpp +++ clangd/Trace.cpp @@ -44,10 +44,23 @@ Out.flush(); } + void begin_event(const Context &Ctx, llvm::StringRef Name) override { +jsonEvent("B", json::obj{{"name", Name}}); + } + + void end_event(const Context &Ctx, llvm::StringRef Name, + json::obj &&Args) override { +jsonEvent("E", json::obj{{"args", std::move(Args)}}); + } + + void instant_event(const Context &Ctx, llvm::StringRef Name, + json::obj &&Args) override { +jsonEvent("i", json::obj{{"name", Name}, {"args", std::move(Args)}}); + } + // Record an event on the current thread. ph, pid, tid, ts are set. // Contents must be a list of the other JSON key/values. - void event(const Context &Ctx, StringRef Phase, - json::obj &&Contents) override { + void jsonEvent(StringRef Phase, json::obj &&Contents) { uint64_t TID = get_threadid(); std::lock_guard Lock(Mu); // If we haven't already, emit metadata describing this thread. @@ -109,30 +122,24 @@ void log(const Context &Ctx, const Twine &Message) { if (!T) return; - T->event(Ctx, "i", - json::obj{ - {"name", "Log"}, - {"args", json::obj{{"Message", Message.str()}}}, - }); + T->instant_event(Ctx, "Log", json::obj{{"Message", Message.str()}}); } -Span::Span(const Context &Ctx, std::string Name) { +Span::Span(const Context &Ctx, llvm::StringRef Name) : Name(Name) { if (!T) return; // Clone the context, so that the original Context can be moved. this->Ctx.emplace(Ctx.clone()); - T->event(*this->Ctx, "B", json::obj{{"name", std::move(Name)}}); + T->begin_event(*this->Ctx, this->Name); Args = llvm::make_unique(); } Span::~Span() { if (!T) return; - if (!Args) -Args = llvm::make_unique(); - T->event(*Ctx, "E", - Args ? json::obj{{"args", std::move(*Args)}} : json::obj{}); + assert(Args && "Args can't be null at this point"); + T->end_event(*this->Ctx, Name, std::move(*Args)); } } // namespace trace ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35624: Removal of microMIPS64R6
abeserminji updated this revision to Diff 126331. abeserminji added a comment. Comment addressed. https://reviews.llvm.org/D35624 Files: include/clang/Basic/DiagnosticCommonKinds.td lib/Basic/Targets/Mips.cpp Index: lib/Basic/Targets/Mips.cpp === --- lib/Basic/Targets/Mips.cpp +++ lib/Basic/Targets/Mips.cpp @@ -206,6 +206,13 @@ } bool MipsTargetInfo::validateTarget(DiagnosticsEngine &Diags) const { + // microMIPS64R6 backend is removed + if ((getTriple().getArch() == llvm::Triple::mips64 || + getTriple().getArch() == llvm::Triple::mips64el) && + IsMicromips && (ABI == "n32" || ABI == "n64")) { +Diags.Report(diag::err_target_unsupported_cpu_for_micromips) << CPU; +return false; + } // FIXME: It's valid to use O32 on a 64-bit CPU but the backend can't handle //this yet. It's better to fail here than on the backend assertion. if (processorSupportsGPR64() && ABI == "o32") { Index: include/clang/Basic/DiagnosticCommonKinds.td === --- include/clang/Basic/DiagnosticCommonKinds.td +++ include/clang/Basic/DiagnosticCommonKinds.td @@ -185,6 +185,8 @@ def err_target_unknown_triple : Error< "unknown target triple '%0', please use -triple or -arch">; def err_target_unknown_cpu : Error<"unknown target CPU '%0'">; +def err_target_unsupported_cpu_for_micromips : Error< + "micromips is not supported for target CPU '%0'">; def err_target_unknown_abi : Error<"unknown target ABI '%0'">; def err_target_unsupported_abi : Error<"ABI '%0' is not supported on CPU '%1'">; def err_target_unsupported_abi_for_triple : Error< Index: lib/Basic/Targets/Mips.cpp === --- lib/Basic/Targets/Mips.cpp +++ lib/Basic/Targets/Mips.cpp @@ -206,6 +206,13 @@ } bool MipsTargetInfo::validateTarget(DiagnosticsEngine &Diags) const { + // microMIPS64R6 backend is removed + if ((getTriple().getArch() == llvm::Triple::mips64 || + getTriple().getArch() == llvm::Triple::mips64el) && + IsMicromips && (ABI == "n32" || ABI == "n64")) { +Diags.Report(diag::err_target_unsupported_cpu_for_micromips) << CPU; +return false; + } // FIXME: It's valid to use O32 on a 64-bit CPU but the backend can't handle //this yet. It's better to fail here than on the backend assertion. if (processorSupportsGPR64() && ABI == "o32") { Index: include/clang/Basic/DiagnosticCommonKinds.td === --- include/clang/Basic/DiagnosticCommonKinds.td +++ include/clang/Basic/DiagnosticCommonKinds.td @@ -185,6 +185,8 @@ def err_target_unknown_triple : Error< "unknown target triple '%0', please use -triple or -arch">; def err_target_unknown_cpu : Error<"unknown target CPU '%0'">; +def err_target_unsupported_cpu_for_micromips : Error< + "micromips is not supported for target CPU '%0'">; def err_target_unknown_abi : Error<"unknown target ABI '%0'">; def err_target_unsupported_abi : Error<"ABI '%0' is not supported on CPU '%1'">; def err_target_unsupported_abi_for_triple : Error< ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39451: P0620 follow-up: deducing `auto` from braced-init-list in new expr
rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1992 +def ext_auto_new_list_init : Extension< + "ISO C++ standards before C++17 does not allow new expression for " + "type %0 using list-initialization">, InGroup; does not -> do not, using -> to use Comment at: lib/Sema/SemaExprCXX.cpp:1763-1765 +if (Braced && !getLangOpts().CPlusPlus17) + Diag(Initializer->getLocStart(), diag::ext_auto_new_list_init) + << AllocType << TypeRange; Move this after the "ctor_multiple_expressions" diagnostic below. Repository: rC Clang https://reviews.llvm.org/D39451 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35624: Removal of microMIPS64R6
sdardis added inline comments. Comment at: lib/Basic/Targets/Mips.cpp:209 bool MipsTargetInfo::validateTarget(DiagnosticsEngine &Diags) const { + // microMIPS64R6 backend is removed + if ((getTriple().getArch() == llvm::Triple::mips64 || "was removed." https://reviews.llvm.org/D35624 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35624: Removal of microMIPS64R6
This revision was automatically updated to reflect the committed changes. Closed by commit rL320351: [mips] Removal of microMIPS64R6 (authored by abeserminji). Changed prior to commit: https://reviews.llvm.org/D35624?vs=126331&id=126332#toc Repository: rL LLVM https://reviews.llvm.org/D35624 Files: cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td cfe/trunk/lib/Basic/Targets/Mips.cpp Index: cfe/trunk/lib/Basic/Targets/Mips.cpp === --- cfe/trunk/lib/Basic/Targets/Mips.cpp +++ cfe/trunk/lib/Basic/Targets/Mips.cpp @@ -206,6 +206,13 @@ } bool MipsTargetInfo::validateTarget(DiagnosticsEngine &Diags) const { + // microMIPS64R6 backend is removed + if ((getTriple().getArch() == llvm::Triple::mips64 || + getTriple().getArch() == llvm::Triple::mips64el) && + IsMicromips && (ABI == "n32" || ABI == "n64")) { +Diags.Report(diag::err_target_unsupported_cpu_for_micromips) << CPU; +return false; + } // FIXME: It's valid to use O32 on a 64-bit CPU but the backend can't handle //this yet. It's better to fail here than on the backend assertion. if (processorSupportsGPR64() && ABI == "o32") { Index: cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td === --- cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td +++ cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td @@ -185,6 +185,8 @@ def err_target_unknown_triple : Error< "unknown target triple '%0', please use -triple or -arch">; def err_target_unknown_cpu : Error<"unknown target CPU '%0'">; +def err_target_unsupported_cpu_for_micromips : Error< + "micromips is not supported for target CPU '%0'">; def err_target_unknown_abi : Error<"unknown target ABI '%0'">; def err_target_unsupported_abi : Error<"ABI '%0' is not supported on CPU '%1'">; def err_target_unsupported_abi_for_triple : Error< Index: cfe/trunk/lib/Basic/Targets/Mips.cpp === --- cfe/trunk/lib/Basic/Targets/Mips.cpp +++ cfe/trunk/lib/Basic/Targets/Mips.cpp @@ -206,6 +206,13 @@ } bool MipsTargetInfo::validateTarget(DiagnosticsEngine &Diags) const { + // microMIPS64R6 backend is removed + if ((getTriple().getArch() == llvm::Triple::mips64 || + getTriple().getArch() == llvm::Triple::mips64el) && + IsMicromips && (ABI == "n32" || ABI == "n64")) { +Diags.Report(diag::err_target_unsupported_cpu_for_micromips) << CPU; +return false; + } // FIXME: It's valid to use O32 on a 64-bit CPU but the backend can't handle //this yet. It's better to fail here than on the backend assertion. if (processorSupportsGPR64() && ABI == "o32") { Index: cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td === --- cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td +++ cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td @@ -185,6 +185,8 @@ def err_target_unknown_triple : Error< "unknown target triple '%0', please use -triple or -arch">; def err_target_unknown_cpu : Error<"unknown target CPU '%0'">; +def err_target_unsupported_cpu_for_micromips : Error< + "micromips is not supported for target CPU '%0'">; def err_target_unknown_abi : Error<"unknown target ABI '%0'">; def err_target_unsupported_abi : Error<"ABI '%0' is not supported on CPU '%1'">; def err_target_unsupported_abi_for_triple : Error< ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40897: [clangd] Introduce a "Symbol" class.
sammccall added a comment. Thanks for the restructuring? I want to take another pass today, but wanted to mention some SymbolID things. Comment at: clangd/Symbol.h:37 +// The class presents a C++ symbol, e.g. class, function. +struct Symbol { + // The symbol identifier, using USR. malaperle wrote: > malaperle wrote: > > sammccall wrote: > > > hokein wrote: > > > > malaperle wrote: > > > > > sammccall wrote: > > > > > > hokein wrote: > > > > > > > malaperle wrote: > > > > > > > > I think it would be nice to have methods as an interface to get > > > > > > > > this data instead of storing them directly. So that an > > > > > > > > index-on-disk could go fetch the data. Especially the > > > > > > > > occurrences which can take a lot of memory (I'm working on a > > > > > > > > branch that does that). But perhaps defining that interface is > > > > > > > > not within the scope of this patch and could be better > > > > > > > > discussed in D40548 ? > > > > > > > I agree. We can't load all the symbol occurrences into the memory > > > > > > > since they are too large. We need to design interface for the > > > > > > > symbol occurrences. > > > > > > > > > > > > > > We could discuss the interface here, but CodeCompletion is the > > > > > > > main thing which this patch focuses on. > > > > > > > We can't load all the symbol occurrences into the memory since > > > > > > > they are too large > > > > > > > > > > > > I've heard this often, but never backed up by data :-) > > > > > > > > > > > > Naively an array of references for a symbol could be doc ID + > > > > > > offset + length, let's say 16 bytes. > > > > > > > > > > > > If a source file consisted entirely of references to 1-character > > > > > > symbols separated by punctuation (1 reference per 2 bytes) then the > > > > > > total size of these references would be 8x the size of the source > > > > > > file - in practice much less. That's not very big. > > > > > > > > > > > > (Maybe there are edge cases with macros/templates, but we can keep > > > > > > them under control) > > > > > I'd have to break down how much memory it used by what, I'll come > > > > > back to you on that. Indexing llvm with ClangdIndexDataStorage, which > > > > > is pretty packed is about 200MB. That's already a lot considering we > > > > > want to index code bases many times bigger. But I'll try to come up > > > > > with more precise numbers. I'm open to different strategies. > > > > > > > > > I can see two points here: > > > > > > > > 1. For all symbol occurrences of a TU, it is not quite large, and we > > > > can keep them in memory. > > > > 2. For all symbol occurrences of a whole project, it's not a good idea > > > > to load all of them into memory (For LLVM project, the size of YAML > > > > dataset is ~1.2G). > > > (This is still a sidebar - not asking for any changes) > > > > > > The YAML dataset is not a good proxy for how big the data is (at least > > > without an effort to estimate constant factor). > > > And "it's not a good idea" isn't an assertion that can hold without > > > reasons, assumptions, and data. > > > If the size turns out to be, say, 120MB for LLVM, and we want to scale to > > > 10x that, and we're spending 500MB/file for ASTs, then it might well be a > > > good trade. > > > The YAML dataset is not a good proxy for how big the data is (at least > > > without an effort to estimate constant factor). > > > > Indeed. I'll try to come up with more realistic numbers. There are other > > things not accounted for in the 16 bytes mentioned above, like storing > > roles and relations. > > > > > 500MB/file for ASTs > > > > What do you mean? 500MB worth of occurrences per file? Or Preambles perhaps? > > What do you mean? 500MB worth of occurrences per file? Or Preambles perhaps? > > Oh I see, the AST must be in memory for fast reparse. I just tried opening 3 > files at the same time I it was already around 500MB. Hmm, that's a bit > alarming. Right, just that we have to consider RAM usage for the index in the context of clangd's overall requirements - if other parts of clangd use 1G of ram for typical work on a large project, then we shouldn't rule out spending a couple of 100MB on the index if it adds a lot of value. Comment at: clangd/Symbol.h:72 + + void addSymbol(Symbol S); + bool hasSymbol(StringRef Identifier) const; hokein wrote: > sammccall wrote: > > This is dangerous if called after reads, as it invalidates iterators and > > pointers. > > I don't think we actually indend to support such mutation, so I'd suggest > > adding an explicit freeze() function. addSymbol() is only valid before > > freeze(), and reading functions are only valid after. An assert can enforce > > this. > > (This is a cheap version of a builder, which are more painful to write but > > may also be worth it). > > > > If we choose not to enforce this at all, the requirement s
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
sammccall added a comment. Thanks for the changes. I don't think `TypedValueMap`/`ContextBuilder` pull their weight, but let's get another opinion on this. Comment at: clangd/Context.h:65 + Context *Parent; + TypedValueMap Data; +}; ilya-biryukov wrote: > sammccall wrote: > > ilya-biryukov wrote: > > > sammccall wrote: > > > > We add complexity here (implementation and conceptual) to allow > > > > multiple properties to be set at the same level (vs having a key and an > > > > AnyStorage and making Context a linked list). > > > > Is this for performance? I'm not convinced it'll actually be faster for > > > > our workloads, or that it matters. > > > Conceptually, a `Context` is more convenient to use when it stores > > > multiple values. This allows to put a bunch of things and assign meaning > > > to `Context` (i.e., a `Context` for processing a single LSP request, > > > global context). If `Context`s were a linked list, the intermediate > > > `Context`s would be hard to assign the meaning to. > > > > > > That being said, storage strategy for `Context`s is an implementation > > > detail and could be changed anytime. I don't have big preferences here, > > > but I think that storing a linked list of maps has, in general, a better > > > performance than storing a linked list. > > > And given that it's already there, I'd leave it this way. > > With the new shared_ptr semantics: > > > > Context D = move(C).derive(K1, V1).derive(K2, V2); > > > > Is just as meaningful as > > > > Context D = move(C).derive().add(K1, V1).add(K2, V2); > > > > Yeah, the list of maps in an implementation detail. It's one that comes > > with a bunch of complexity (`ContextBuilder` and most of `TypedValueMap`). > > It really doesn't seem to buy us anything (the performance is both > > uninteresting and seems likely to be worse in this specific case with very > > few entries). > The thing I like about it is that the `Context`s are layered properly in a > sense that there's a Context corresponding to the request, a Context > corresponding to the forked subrequests, etc. > If we change the interface, we'll be creating a bunch of temporary Contexts > that don't correspond to a nice meaningful abstraction (like request) in my > head, even though we don't give those contexts any names. > > I do agree we currently pay with some complexity for that. Though I'd argue > it's all hidden from the users of the interface, as building and consuming > contexts is still super-easy and you don't need to mention ContextBuilder or > TypedValueMap. And the implementation complexity is totally manageable from > my point of view, but I am the one who implemented it in the first place, so > there's certainly a bias there. I don't see temporary unnamed `Context`s being any different from temporary unnamed `ContextBuilder`s. But we've gone around on this point a bit, and this really seems to be a question of taste. @klimek, can we have a third opinion? The options we're looking at are: - `Context` stores a map and a parent pointer. `derive()` returns a `ContextBuilder` used to create new contexts containing 0 or more new KV pairs. `TypedValueMap` stores the payloads. - `Context` stores a single KV pair and a parent pointer. `derive(K, V)` is used to create a new context with one new KV pair. A Key-pointer and AnyStorage in `Context` store the payloads, the rest of `TypedValueMap` goes away. Comment at: clangd/Context.h:142 +/// Creates a ContextBuilder with a null parent. +ContextBuilder buildCtx(); + ilya-biryukov wrote: > sammccall wrote: > > I think we should spell this `emptyCtx().derive()`. > > It should be pretty rare to derive from the empty context in production > > code - and conceptually it's no different than any other use of the empty > > context, I think. > I'd argue the separate function is more readable and saves us an extra lookup > in the empty context for missing keys. > Given that `emptyCtx` is now `Context::empty`, maybe we should also change > `buildCtx` to `Context::build`? I think this isn't a path we want to make smooth or obvious - usually what you want is to accept a context, derive from it, and use the result. Embedders will need to create root contexts - we discussed `LSPServer` offline, and there it seems natural to own a background context and derive from it per handled call. Comment at: clangd/Context.h:92 + ContextBuilder derive() const &; + ContextBuilder derive() const &&; + `&&`, not `const&&` :-) Maybe add a trailing `//takes ownership` Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40486: [clangd] Implemented logging using Context
ilya-biryukov updated this revision to Diff 126337. ilya-biryukov marked 15 inline comments as done. ilya-biryukov added a comment. - Copy Context in forceReparse's call to scheduleCancelRebuild. - Renamed Key<> for ID, Out and Span. - Removed the FIXME - Got rid of getExisting(RequestSpan) - Remove EmptyLogger, branch in log instead. - Renamed GlobalLogger to L - Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40486 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/ClangdUnitStore.cpp clangd/ClangdUnitStore.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/GlobalCompilationDatabase.cpp clangd/GlobalCompilationDatabase.h clangd/JSONRPCDispatcher.cpp clangd/JSONRPCDispatcher.h clangd/Logger.cpp clangd/Logger.h clangd/Protocol.h clangd/ProtocolHandlers.cpp clangd/ProtocolHandlers.h clangd/tool/ClangdMain.cpp unittests/clangd/ClangdTests.cpp unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -8,6 +8,7 @@ //===--===// #include "ClangdServer.h" #include "Compiler.h" +#include "Context.h" #include "Protocol.h" #include "TestFS.h" #include "gtest/gtest.h" @@ -74,8 +75,7 @@ MockCompilationDatabase CDB; ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, - EmptyLogger::getInstance()); + /*StorePreamblesInMemory=*/true); auto FooCpp = getVirtualTestFilePath("foo.cpp"); const auto SourceContents = R"cpp( @@ -101,29 +101,33 @@ // No need to sync reparses here as there are no asserts on diagnostics (or // other async operations). - Server.addDocument(FooCpp, SourceContents); + Server.addDocument(FooCpp, SourceContents, buildCtx()); { auto CodeCompletionResults1 = -Server.codeComplete(FooCpp, CompletePos, CCOpts, None).get().Value; +Server.codeComplete(FooCpp, CompletePos, CCOpts, buildCtx(), None) +.get() +.first.Value; EXPECT_TRUE(ContainsItem(CodeCompletionResults1, "aba")); EXPECT_FALSE(ContainsItem(CodeCompletionResults1, "cbc")); } { auto CodeCompletionResultsOverriden = Server -.codeComplete(FooCpp, CompletePos, CCOpts, +.codeComplete(FooCpp, CompletePos, CCOpts, buildCtx(), StringRef(OverridenSourceContents)) .get() -.Value; +.first.Value; EXPECT_TRUE(ContainsItem(CodeCompletionResultsOverriden, "cbc")); EXPECT_FALSE(ContainsItem(CodeCompletionResultsOverriden, "aba")); } { auto CodeCompletionResults2 = -Server.codeComplete(FooCpp, CompletePos, CCOpts, None).get().Value; +Server.codeComplete(FooCpp, CompletePos, CCOpts, buildCtx(), None) +.get() +.first.Value; EXPECT_TRUE(ContainsItem(CodeCompletionResults2, "aba")); EXPECT_FALSE(ContainsItem(CodeCompletionResults2, "cbc")); } @@ -135,8 +139,7 @@ CDB.ExtraClangFlags.push_back("-xc++"); IgnoreDiagnostics DiagConsumer; ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, - EmptyLogger::getInstance()); + /*StorePreamblesInMemory=*/true); auto FooCpp = getVirtualTestFilePath("foo.cpp"); FS.Files[FooCpp] = ""; @@ -150,17 +153,17 @@ int main() { ClassWithMembers().{complete} } )cpp", "complete"); - Server.addDocument(FooCpp, Completion.Text); + Server.addDocument(FooCpp, Completion.Text, buildCtx()); clangd::CodeCompleteOptions Opts; Opts.Limit = 2; /// For after-dot completion we must always get consistent results. auto Results = Server .codeComplete(FooCpp, Completion.MarkerPos, Opts, - StringRef(Completion.Text)) + buildCtx(), StringRef(Completion.Text)) .get() - .Value; + .first.Value; EXPECT_TRUE(Results.isIncomplete); EXPECT_EQ(Opts.Limit, Results.items.size()); @@ -175,8 +178,7 @@ CDB.ExtraClangFlags.push_back("-xc++"); IgnoreDiagnostics DiagConsumer; ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, - EmptyLogger::getInstance()); + /*StorePreamblesInMemory=*/true); auto FooCpp = getVirtualTestFilePath("foo.cpp"); FS.Files[FooCpp] = ""; @@ -194,12 +
[PATCH] D41064: Suppress -Wuser-defined-literals for and
dim created this revision. When compiling and/or with -Wsystem-headers, in C++14 or higher mode, clang produces warnings about the literal suffixes defined in them, e.g.: $ cat test.cpp #include $ clang -std=c++14 -Wsystem-headers -Wall -Wextra -c test.cpp In file included from test.cpp:1: In file included from /usr/include/c++/v1/string:470: /usr/include/c++/v1/string_view:763:29: warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals] basic_string_view operator "" sv(const char *__str, size_t __len) ^ /usr/include/c++/v1/string_view:769:32: warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals] basic_string_view operator "" sv(const wchar_t *__str, size_t __len) ^ /usr/include/c++/v1/string_view:775:33: warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals] basic_string_view operator "" sv(const char16_t *__str, size_t __len) ^ /usr/include/c++/v1/string_view:781:33: warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals] basic_string_view operator "" sv(const char32_t *__str, size_t __len) ^ In file included from test.cpp:1: /usr/include/c++/v1/string:4012:24: warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals] basic_string operator "" s( const char *__str, size_t __len ) ^ /usr/include/c++/v1/string:4018:27: warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals] basic_string operator "" s( const wchar_t *__str, size_t __len ) ^ /usr/include/c++/v1/string:4024:28: warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals] basic_string operator "" s( const char16_t *__str, size_t __len ) ^ /usr/include/c++/v1/string:4030:28: warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals] basic_string operator "" s( const char32_t *__str, size_t __len ) ^ 8 warnings generated. Similar to what is done in , suppress these warnings using `#pragma clang diagnostic`. Repository: rCXX libc++ https://reviews.llvm.org/D41064 Files: include/string include/string_view Index: include/string_view === --- include/string_view +++ include/string_view @@ -792,6 +792,10 @@ { inline namespace string_view_literals { +#if defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wuser-defined-literals" +#endif inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR basic_string_view operator "" sv(const char *__str, size_t __len) _NOEXCEPT { @@ -815,6 +819,9 @@ { return basic_string_view (__str, __len); } +#if defined(__clang__) +#pragma clang diagnostic pop +#endif } } #endif Index: include/string === --- include/string +++ include/string @@ -4042,6 +4042,10 @@ { inline namespace string_literals { +#if defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wuser-defined-literals" +#endif inline _LIBCPP_INLINE_VISIBILITY basic_string operator "" s( const char *__str, size_t __len ) { @@ -4065,6 +4069,9 @@ { return basic_string (__str, __len); } +#if defined(__clang__) +#pragma clang diagnostic pop +#endif } } #endif Index: include/string_view === --- include/string_view +++ include/string_view @@ -792,6 +792,10 @@ { inline namespace string_view_literals { +#if defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wuser-defined-literals" +#endif inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR basic_string_view operator "" sv(const char *__str, size_t __len) _NOEXCEPT { @@ -815,6 +819,9 @@ { return basic_string_view (__str, __len); } +#if defined(__clang__) +#pragma clang diagnostic pop +#endif } } #endif Index: include/string === --- include/string +++ include/string @@ -4042,6 +4042,10 @@ { inline namespace string_literals { +#if defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wuser-defined-literals" +#endif inline _LIBCPP_INLINE_VISIBILITY basic_string operator "" s( const char *__str, size_t __len ) { @@ -4065,6 +4069,9 @@ { return basic_string (__str, __len); } +#if defined(__cla
[PATCH] D40486: [clangd] Implemented logging using Context
ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.cpp:220 - // Note that std::future from this cleanup action is ignored. - scheduleCancelRebuild(std::move(Recreated.RemovedFile)); + // Note that std::future from this cleanup action. + // FIXME(ibiryukov): We use a global context here, should not fork the action sammccall wrote: > You dropped the end of this comment Missed that. Thanks. Comment at: clangd/ClangdServer.cpp:221 + // Note that std::future from this cleanup action. + // FIXME(ibiryukov): We use a global context here, should not fork the action + // instead. sammccall wrote: > I don't quite understand what this is saying should be better. Is it saying > we do these two things together instead of running them in parallel? > > And why not clone the context instead? The first implementation did not allow to copy the `Context`, hence the comment. I've removed it and the context is now copied. Comment at: clangd/ClangdUnit.cpp:436 + RebuildInProgress(false), PCHs(std::move(PCHs)) { + log(emptyCtx(), "Opened file " + FileName + " with command [" + + this->Command.Directory + "] " + sammccall wrote: > why are we dropping the context here? > It's important that in general we don't store the ctx in the object and > blindly reuse it, but passing it into the constructor for use *in* the > constructor seems fine. > > Reading on... OK, it saves a *lot* of plumbing to avoid attributing this one > API call. I still don't totally understand how the CppFile API/lifetime > works, so up to you. Exactly, plumbing it here is hard, so we're dropping the context here for now. Most of the useful work is still done outside the constructor, so this seems fine. The threading patch will make it easier to pass proper Context here. I've added a FIXME. Comment at: clangd/ClangdUnit.h:257 +/// Get signature help at a specified \p Pos in \p FileName. +SignatureHelp sammccall wrote: > I think this is a bad merge - signatureHelp has moved to CodeComplete.h It is totally a bad merge. Thanks. Comment at: clangd/GlobalCompilationDatabase.h:37 virtual llvm::Optional getCompileCommand(PathRef File) const = 0; sammccall wrote: > Do we want Ctx in this interface? > It's an extension point, and the ability for embedders to track calls flowing > between their API calls and their extension points is one of the reasons we > have ctx... I think we do, but I'd add this in a separate patch. (We're currently not changing the behavior, simply plumbing the Context through all the APIs). Happy to come up with a follow-up patch, though. Comment at: clangd/JSONRPCDispatcher.cpp:48 +void JSONOutput::logImpl(Context &Ctx, const Twine &Message) { + // FIXME(ibiryukov): get rid of trace::log here. trace::log(Message); sammccall wrote: > why? This was a reminder to myself to discuss whether we want it globally in tracing+logging, not only in `JSONOutput`. Removed it from the code. Comment at: clangd/JSONRPCDispatcher.cpp:70 + + SPAN_ATTACH(*Ctx->getExisting(TracerKey), "Reply", Result); + Ctx->getPtr(OutKey)->writeMessage(json::obj{ sammccall wrote: > Depending on a span tracer existing seems needlessly brittle. What about > > if (auto *Tracer = Ctx->get(TracerKey)) > SPAN_ATTACH(*Tracer, ...) Done. Turned out to be `SPAN(**Tracer, ...`. (Note the double dereference). Comment at: clangd/JSONRPCDispatcher.h:29 /// them. class JSONOutput : public Logger { public: sammccall wrote: > does this class need to be public? Is it staying around for the long haul? I'm not sure. I'd rather keep it private, but we don't have the right abstractions to hide it yet, right? I'd say this is not directly related to the current patch and should be addressed separately. Comment at: clangd/JSONRPCDispatcher.h:39 /// Write a line to the logging stream. - void log(const Twine &Message) override; + void logImpl(Context &Ctx, const Twine &Message) override; sammccall wrote: > Is this temporary? Renamed back to `log`. It's a leftover from an older version of the patch. Comment at: clangd/Logger.cpp:22 +EmptyLogger Empty; +Logger *GlobalLogger = &Empty; +} // namespace sammccall wrote: > There are no non-global loggers. Just L, or Current, or something? This is really just a preference. To me the global prefix is not about distinguishing it from other things in this file, but rather about signalling what is this thing so that I don't have to read the declaration before reading the usages to understand what's going on. Having `L` or `Current` requires to be in the context of the file. Updated the code to use `L` instead.
[PATCH] D40486: [clangd] Implemented logging using Context
ilya-biryukov updated this revision to Diff 126340. ilya-biryukov added a comment. - Remove mention of globalLogger() from the comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40486 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/ClangdUnitStore.cpp clangd/ClangdUnitStore.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/GlobalCompilationDatabase.cpp clangd/GlobalCompilationDatabase.h clangd/JSONRPCDispatcher.cpp clangd/JSONRPCDispatcher.h clangd/Logger.cpp clangd/Logger.h clangd/Protocol.h clangd/ProtocolHandlers.cpp clangd/ProtocolHandlers.h clangd/tool/ClangdMain.cpp unittests/clangd/ClangdTests.cpp unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -8,6 +8,7 @@ //===--===// #include "ClangdServer.h" #include "Compiler.h" +#include "Context.h" #include "Protocol.h" #include "TestFS.h" #include "gtest/gtest.h" @@ -74,8 +75,7 @@ MockCompilationDatabase CDB; ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, - EmptyLogger::getInstance()); + /*StorePreamblesInMemory=*/true); auto FooCpp = getVirtualTestFilePath("foo.cpp"); const auto SourceContents = R"cpp( @@ -101,29 +101,33 @@ // No need to sync reparses here as there are no asserts on diagnostics (or // other async operations). - Server.addDocument(FooCpp, SourceContents); + Server.addDocument(FooCpp, SourceContents, buildCtx()); { auto CodeCompletionResults1 = -Server.codeComplete(FooCpp, CompletePos, CCOpts, None).get().Value; +Server.codeComplete(FooCpp, CompletePos, CCOpts, buildCtx(), None) +.get() +.first.Value; EXPECT_TRUE(ContainsItem(CodeCompletionResults1, "aba")); EXPECT_FALSE(ContainsItem(CodeCompletionResults1, "cbc")); } { auto CodeCompletionResultsOverriden = Server -.codeComplete(FooCpp, CompletePos, CCOpts, +.codeComplete(FooCpp, CompletePos, CCOpts, buildCtx(), StringRef(OverridenSourceContents)) .get() -.Value; +.first.Value; EXPECT_TRUE(ContainsItem(CodeCompletionResultsOverriden, "cbc")); EXPECT_FALSE(ContainsItem(CodeCompletionResultsOverriden, "aba")); } { auto CodeCompletionResults2 = -Server.codeComplete(FooCpp, CompletePos, CCOpts, None).get().Value; +Server.codeComplete(FooCpp, CompletePos, CCOpts, buildCtx(), None) +.get() +.first.Value; EXPECT_TRUE(ContainsItem(CodeCompletionResults2, "aba")); EXPECT_FALSE(ContainsItem(CodeCompletionResults2, "cbc")); } @@ -135,8 +139,7 @@ CDB.ExtraClangFlags.push_back("-xc++"); IgnoreDiagnostics DiagConsumer; ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, - EmptyLogger::getInstance()); + /*StorePreamblesInMemory=*/true); auto FooCpp = getVirtualTestFilePath("foo.cpp"); FS.Files[FooCpp] = ""; @@ -150,17 +153,17 @@ int main() { ClassWithMembers().{complete} } )cpp", "complete"); - Server.addDocument(FooCpp, Completion.Text); + Server.addDocument(FooCpp, Completion.Text, buildCtx()); clangd::CodeCompleteOptions Opts; Opts.Limit = 2; /// For after-dot completion we must always get consistent results. auto Results = Server .codeComplete(FooCpp, Completion.MarkerPos, Opts, - StringRef(Completion.Text)) + buildCtx(), StringRef(Completion.Text)) .get() - .Value; + .first.Value; EXPECT_TRUE(Results.isIncomplete); EXPECT_EQ(Opts.Limit, Results.items.size()); @@ -175,8 +178,7 @@ CDB.ExtraClangFlags.push_back("-xc++"); IgnoreDiagnostics DiagConsumer; ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, - EmptyLogger::getInstance()); + /*StorePreamblesInMemory=*/true); auto FooCpp = getVirtualTestFilePath("foo.cpp"); FS.Files[FooCpp] = ""; @@ -194,12 +196,13 @@ StringWithPos Completion = parseTextMarker( formatv("{0} int main() { {1}{{complete}} }", Body, Query).str(), "complete"); -Server.addDocument(FooCpp, Completion.Text); +Server.addDocument(FooCpp, Completion.Text, bu
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
ilya-biryukov updated this revision to Diff 126345. ilya-biryukov added a comment. - Use `derive() &&` instead of `derive() const &&`. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 Files: clangd/CMakeLists.txt clangd/Context.cpp clangd/Context.h clangd/TypedValueMap.h unittests/clangd/CMakeLists.txt unittests/clangd/ContextTests.cpp Index: unittests/clangd/ContextTests.cpp === --- /dev/null +++ unittests/clangd/ContextTests.cpp @@ -0,0 +1,71 @@ +//===-- ContextTests.cpp - Context tests *- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "Context.h" + +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { + +TEST(TypedValueMapTests, Simple) { + Key IntParam; + Key ExtraIntParam; + + clangd::TypedValueMap Ctx; + + ASSERT_TRUE(Ctx.emplace(IntParam, 10)); + ASSERT_TRUE(Ctx.emplace(ExtraIntParam, 20)); + + EXPECT_EQ(*Ctx.get(IntParam), 10); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); + + ASSERT_FALSE(Ctx.emplace(IntParam, 30)); + + ASSERT_TRUE(Ctx.remove(IntParam)); + EXPECT_EQ(Ctx.get(IntParam), nullptr); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); + + ASSERT_TRUE(Ctx.emplace(IntParam, 30)); + EXPECT_EQ(*Ctx.get(IntParam), 30); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); +} + +TEST(TypedValueMapTests, MoveOps) { + Key> Param; + + clangd::TypedValueMap Ctx; + Ctx.emplace(Param, llvm::make_unique(10)); + EXPECT_EQ(**Ctx.get(Param), 10); + + clangd::TypedValueMap NewCtx = std::move(Ctx); + EXPECT_EQ(**NewCtx.get(Param), 10); +} + +TEST(ContextTests, Builders) { + Key ParentParam; + Key ParentAndChildParam; + Key ChildParam; + + Context ParentCtx = + buildCtx().add(ParentParam, 10).add(ParentAndChildParam, 20); + Context ChildCtx = + ParentCtx.derive().add(ParentAndChildParam, 30).add(ChildParam, 40); + + EXPECT_EQ(*ParentCtx.get(ParentParam), 10); + EXPECT_EQ(*ParentCtx.get(ParentAndChildParam), 20); + EXPECT_EQ(ParentCtx.get(ChildParam), nullptr); + + EXPECT_EQ(*ChildCtx.get(ParentParam), 10); + EXPECT_EQ(*ChildCtx.get(ParentAndChildParam), 30); + EXPECT_EQ(*ChildCtx.get(ChildParam), 40); +} + +} // namespace clangd +} // namespace clang Index: unittests/clangd/CMakeLists.txt === --- unittests/clangd/CMakeLists.txt +++ unittests/clangd/CMakeLists.txt @@ -11,6 +11,7 @@ add_extra_unittest(ClangdTests ClangdTests.cpp CodeCompleteTests.cpp + ContextTests.cpp FuzzyMatchTests.cpp JSONExprTests.cpp TestFS.cpp Index: clangd/TypedValueMap.h === --- /dev/null +++ clangd/TypedValueMap.h @@ -0,0 +1,104 @@ +//===--- TypedValueMap.h - Type-safe heterogenous key-value map -*- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// Type-safe heterogenous map. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TYPEDVALUEMAP_H_ +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TYPEDVALUEMAP_H_ + +#include "llvm/ADT/DenseMap.h" +#include + +namespace clang { +namespace clangd { + +/// Used as identity for map values. Non-movable and non-copyable. Address of +/// this object is used internally to as keys in a map. +template class Key { +public: + static_assert(!std::is_reference::value, +"Reference arguments to Key<> are not allowed"); + + Key() = default; + + Key(Key const &) = delete; + Key &operator=(Key const &) = delete; + Key(Key &&) = delete; + Key &operator=(Key &&) = delete; +}; + +/// A type-safe map from Key to T. +class TypedValueMap { +public: + TypedValueMap() = default; + TypedValueMap(const TypedValueMap &) = delete; + TypedValueMap(TypedValueMap &&) = default; + + template const Type *get(const Key &Key) const { +return const_cast(this)->get(Key); + } + + template Type *get(const Key &Key) { +auto It = Map.find(&Key); +if (It == Map.end()) + return nullptr; +return static_cast(It->second->getValuePtr()); + } + + template + bool emplace(const Key &Key, Args &&... As) { +bool Added = +Map.try_emplace(&Key, +llvm::make_unique< +TypedAnyStorage::type>>( +std::forward(As)...)) +.second; +return Added; + } + + template bool remove(Key &Key) { +auto It = Map.find(&Key); +if (It == Map.end()) + return
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
ilya-biryukov added inline comments. Comment at: clangd/Context.h:92 + ContextBuilder derive() const &; + ContextBuilder derive() const &&; + sammccall wrote: > `&&`, not `const&&` :-) > > Maybe add a trailing `//takes ownership` Right. Copy-paste is gonna kill me some day. Added the comment, too. Although that should be obvious from the `&&` in the signature. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40561: [libclang] Fix cursors for functions with trailing return type
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D40561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41064: Suppress -Wuser-defined-literals for and
aaron.ballman added a comment. I think that it would be more appropriate to fix this in Clang rather than libc++. For instance, we don't want libstdc++ to have to silence our same diagnostic here. Repository: rCXX libc++ https://reviews.llvm.org/D41064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
ilya-biryukov updated this revision to Diff 126346. ilya-biryukov added a comment. - Removed buildCtx(). Now Contexts can only be created using emptyCtx().derive() Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 Files: clangd/CMakeLists.txt clangd/Context.cpp clangd/Context.h clangd/TypedValueMap.h unittests/clangd/CMakeLists.txt unittests/clangd/ContextTests.cpp Index: unittests/clangd/ContextTests.cpp === --- /dev/null +++ unittests/clangd/ContextTests.cpp @@ -0,0 +1,73 @@ +//===-- ContextTests.cpp - Context tests *- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "Context.h" + +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { + +TEST(TypedValueMapTests, Simple) { + Key IntParam; + Key ExtraIntParam; + + clangd::TypedValueMap Ctx; + + ASSERT_TRUE(Ctx.emplace(IntParam, 10)); + ASSERT_TRUE(Ctx.emplace(ExtraIntParam, 20)); + + EXPECT_EQ(*Ctx.get(IntParam), 10); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); + + ASSERT_FALSE(Ctx.emplace(IntParam, 30)); + + ASSERT_TRUE(Ctx.remove(IntParam)); + EXPECT_EQ(Ctx.get(IntParam), nullptr); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); + + ASSERT_TRUE(Ctx.emplace(IntParam, 30)); + EXPECT_EQ(*Ctx.get(IntParam), 30); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); +} + +TEST(TypedValueMapTests, MoveOps) { + Key> Param; + + clangd::TypedValueMap Ctx; + Ctx.emplace(Param, llvm::make_unique(10)); + EXPECT_EQ(**Ctx.get(Param), 10); + + clangd::TypedValueMap NewCtx = std::move(Ctx); + EXPECT_EQ(**NewCtx.get(Param), 10); +} + +TEST(ContextTests, Builders) { + Key ParentParam; + Key ParentAndChildParam; + Key ChildParam; + + Context ParentCtx = Context::empty() + .derive() + .add(ParentParam, 10) + .add(ParentAndChildParam, 20); + Context ChildCtx = + ParentCtx.derive().add(ParentAndChildParam, 30).add(ChildParam, 40); + + EXPECT_EQ(*ParentCtx.get(ParentParam), 10); + EXPECT_EQ(*ParentCtx.get(ParentAndChildParam), 20); + EXPECT_EQ(ParentCtx.get(ChildParam), nullptr); + + EXPECT_EQ(*ChildCtx.get(ParentParam), 10); + EXPECT_EQ(*ChildCtx.get(ParentAndChildParam), 30); + EXPECT_EQ(*ChildCtx.get(ChildParam), 40); +} + +} // namespace clangd +} // namespace clang Index: unittests/clangd/CMakeLists.txt === --- unittests/clangd/CMakeLists.txt +++ unittests/clangd/CMakeLists.txt @@ -11,6 +11,7 @@ add_extra_unittest(ClangdTests ClangdTests.cpp CodeCompleteTests.cpp + ContextTests.cpp FuzzyMatchTests.cpp JSONExprTests.cpp TestFS.cpp Index: clangd/TypedValueMap.h === --- /dev/null +++ clangd/TypedValueMap.h @@ -0,0 +1,104 @@ +//===--- TypedValueMap.h - Type-safe heterogenous key-value map -*- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// Type-safe heterogenous map. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TYPEDVALUEMAP_H_ +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TYPEDVALUEMAP_H_ + +#include "llvm/ADT/DenseMap.h" +#include + +namespace clang { +namespace clangd { + +/// Used as identity for map values. Non-movable and non-copyable. Address of +/// this object is used internally to as keys in a map. +template class Key { +public: + static_assert(!std::is_reference::value, +"Reference arguments to Key<> are not allowed"); + + Key() = default; + + Key(Key const &) = delete; + Key &operator=(Key const &) = delete; + Key(Key &&) = delete; + Key &operator=(Key &&) = delete; +}; + +/// A type-safe map from Key to T. +class TypedValueMap { +public: + TypedValueMap() = default; + TypedValueMap(const TypedValueMap &) = delete; + TypedValueMap(TypedValueMap &&) = default; + + template const Type *get(const Key &Key) const { +return const_cast(this)->get(Key); + } + + template Type *get(const Key &Key) { +auto It = Map.find(&Key); +if (It == Map.end()) + return nullptr; +return static_cast(It->second->getValuePtr()); + } + + template + bool emplace(const Key &Key, Args &&... As) { +bool Added = +Map.try_emplace(&Key, +llvm::make_unique< +TypedAnyStorage::type>>( +std::forward(As)...)) +.second; +return A
[libcxx] r320363 - [libcxx] Define istream_iterator equality comparison operators out-of-line
Author: rogfer01 Date: Mon Dec 11 05:54:58 2017 New Revision: 320363 URL: http://llvm.org/viewvc/llvm-project?rev=320363&view=rev Log: [libcxx] Define istream_iterator equality comparison operators out-of-line Currently libc++ defines operator== and operator!= as friend functions in the definition of the istream_iterator class template. Such definition has a subtle difference from an out-of-line definition required by the C++ Standard: these functions can only be found by argument-dependent lookup, but not by qualified lookup. This patch changes the definition, so that it conforms to the C++ Standard and adds a check involving qualified lookup to the test suite. Patch contributed by Mikhail Maltsev. Differential Revision: https://reviews.llvm.org/D40415 Modified: libcxx/trunk/include/iterator libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp Modified: libcxx/trunk/include/iterator URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/iterator?rev=320363&r1=320362&r2=320363&view=diff == --- libcxx/trunk/include/iterator (original) +++ libcxx/trunk/include/iterator Mon Dec 11 05:54:58 2017 @@ -904,15 +904,37 @@ public: _LIBCPP_INLINE_VISIBILITY istream_iterator operator++(int) {istream_iterator __t(*this); ++(*this); return __t;} +template friend _LIBCPP_INLINE_VISIBILITY -bool operator==(const istream_iterator& __x, const istream_iterator& __y) -{return __x.__in_stream_ == __y.__in_stream_;} +bool +operator==(const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __x, + const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __y); +template friend _LIBCPP_INLINE_VISIBILITY -bool operator!=(const istream_iterator& __x, const istream_iterator& __y) -{return !(__x == __y);} +bool +operator==(const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __x, + const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __y); }; +template +inline _LIBCPP_INLINE_VISIBILITY +bool +operator==(const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __x, + const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __y) +{ +return __x.__in_stream_ == __y.__in_stream_; +} + +template +inline _LIBCPP_INLINE_VISIBILITY +bool +operator!=(const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __x, + const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __y) +{ +return !(__x == __y); +} + template > class _LIBCPP_TEMPLATE_VIS ostream_iterator : public iterator Modified: libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp?rev=320363&r1=320362&r2=320363&view=diff == --- libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp (original) +++ libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp Mon Dec 11 05:54:58 2017 @@ -49,4 +49,7 @@ int main() assert(i4 == i4); assert(i4 == i5); + +assert(std::operator==(i1, i2)); +assert(std::operator!=(i1, i3)); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40415: [libcxx] Define istream_iterator equality comparison operators out-of-line
This revision was automatically updated to reflect the committed changes. Closed by commit rL320363: [libcxx] Define istream_iterator equality comparison operators out-of-line (authored by rogfer01). Changed prior to commit: https://reviews.llvm.org/D40415?vs=124155&id=126349#toc Repository: rL LLVM https://reviews.llvm.org/D40415 Files: libcxx/trunk/include/iterator libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp Index: libcxx/trunk/include/iterator === --- libcxx/trunk/include/iterator +++ libcxx/trunk/include/iterator @@ -904,15 +904,37 @@ _LIBCPP_INLINE_VISIBILITY istream_iterator operator++(int) {istream_iterator __t(*this); ++(*this); return __t;} +template friend _LIBCPP_INLINE_VISIBILITY -bool operator==(const istream_iterator& __x, const istream_iterator& __y) -{return __x.__in_stream_ == __y.__in_stream_;} +bool +operator==(const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __x, + const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __y); +template friend _LIBCPP_INLINE_VISIBILITY -bool operator!=(const istream_iterator& __x, const istream_iterator& __y) -{return !(__x == __y);} +bool +operator==(const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __x, + const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __y); }; +template +inline _LIBCPP_INLINE_VISIBILITY +bool +operator==(const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __x, + const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __y) +{ +return __x.__in_stream_ == __y.__in_stream_; +} + +template +inline _LIBCPP_INLINE_VISIBILITY +bool +operator!=(const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __x, + const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __y) +{ +return !(__x == __y); +} + template > class _LIBCPP_TEMPLATE_VIS ostream_iterator : public iterator Index: libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp === --- libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp +++ libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp @@ -49,4 +49,7 @@ assert(i4 == i4); assert(i4 == i5); + +assert(std::operator==(i1, i2)); +assert(std::operator!=(i1, i3)); } Index: libcxx/trunk/include/iterator === --- libcxx/trunk/include/iterator +++ libcxx/trunk/include/iterator @@ -904,15 +904,37 @@ _LIBCPP_INLINE_VISIBILITY istream_iterator operator++(int) {istream_iterator __t(*this); ++(*this); return __t;} +template friend _LIBCPP_INLINE_VISIBILITY -bool operator==(const istream_iterator& __x, const istream_iterator& __y) -{return __x.__in_stream_ == __y.__in_stream_;} +bool +operator==(const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __x, + const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __y); +template friend _LIBCPP_INLINE_VISIBILITY -bool operator!=(const istream_iterator& __x, const istream_iterator& __y) -{return !(__x == __y);} +bool +operator==(const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __x, + const istream_iterator<_Up, _CharU, _TraitsU, _DistanceU>& __y); }; +template +inline _LIBCPP_INLINE_VISIBILITY +bool +operator==(const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __x, + const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __y) +{ +return __x.__in_stream_ == __y.__in_stream_; +} + +template +inline _LIBCPP_INLINE_VISIBILITY +bool +operator!=(const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __x, + const istream_iterator<_Tp, _CharT, _Traits, _Distance>& __y) +{ +return !(__x == __y); +} + template > class _LIBCPP_TEMPLATE_VIS ostream_iterator : public iterator Index: libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp === --- libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp +++ libcxx/trunk/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.ops/equal.pass.cpp @@ -49,4 +49,7 @@ assert(i4 == i4); assert(i4 == i5); + +assert(std::operator==(i1, i2)); +assert(std::operator!=(i1, i3)); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40712: [Driver] Add flag enabling the function stack size section that was added in r319430
seaneveson updated this revision to Diff 126350. seaneveson added a comment. Improve tests. https://reviews.llvm.org/D40712 Files: include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.def lib/CodeGen/BackendUtil.cpp lib/Driver/ToolChains/Clang.cpp lib/Frontend/CompilerInvocation.cpp test/CodeGen/stack-size-section.c test/Driver/stack-size-section.c Index: test/Driver/stack-size-section.c === --- test/Driver/stack-size-section.c +++ test/Driver/stack-size-section.c @@ -0,0 +1,9 @@ +// RUN: %clang -target x86_64-unknown %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ABSENT +// RUN: %clang -target x86_64-scei-ps4 -fno-stack-size-section %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ABSENT +// CHECK-ABSENT-NOT: -fstack-size-section + +// RUN: %clang -target x86_64-unknown -fstack-size-section -### 2>&1 | FileCheck %s --check-prefix=CHECK-PRESENT +// RUN: %clang -target x86_64-scei-ps4 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-PRESENT +// CHECK-PRESENT: -fstack-size-section + +int foo() { return 42; } Index: test/CodeGen/stack-size-section.c === --- test/CodeGen/stack-size-section.c +++ test/CodeGen/stack-size-section.c @@ -0,0 +1,7 @@ +// RUN: %clang_cc1 -triple x86_64-unknown %s -S -o - | FileCheck %s --check-prefix=CHECK-ABSENT +// CHECK-ABSENT-NOT: section .stack_sizes + +// RUN: %clang_cc1 -triple x86_64-unknown -fstack-size-section %s -S -o - | FileCheck %s --check-prefix=CHECK-PRESENT +// CHECK-PRESENT: section .stack_sizes + +int foo() { return 42; } Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -681,6 +681,8 @@ OPT_fno_function_sections, false); Opts.DataSections = Args.hasFlag(OPT_fdata_sections, OPT_fno_data_sections, false); + Opts.StackSizeSection = Args.hasFlag( + OPT_fstack_size_section, OPT_fno_stack_size_section, Triple.isPS4()); Opts.UniqueSectionNames = Args.hasFlag(OPT_funique_section_names, OPT_fno_unique_section_names, true); Index: lib/Driver/ToolChains/Clang.cpp === --- lib/Driver/ToolChains/Clang.cpp +++ lib/Driver/ToolChains/Clang.cpp @@ -3775,6 +3775,12 @@ CmdArgs.push_back(A->getValue()); } + if (Args.hasFlag(options::OPT_fstack_size_section, + options::OPT_fno_stack_size_section, RawTriple.isPS4())) +CmdArgs.push_back("-fstack-size-section"); + else +CmdArgs.push_back("-fno-stack-size-section"); + CmdArgs.push_back("-ferror-limit"); if (Arg *A = Args.getLastArg(options::OPT_ferror_limit_EQ)) CmdArgs.push_back(A->getValue()); Index: lib/CodeGen/BackendUtil.cpp === --- lib/CodeGen/BackendUtil.cpp +++ lib/CodeGen/BackendUtil.cpp @@ -444,6 +444,7 @@ Options.UniqueSectionNames = CodeGenOpts.UniqueSectionNames; Options.EmulatedTLS = CodeGenOpts.EmulatedTLS; Options.DebuggerTuning = CodeGenOpts.getDebuggerTuning(); + Options.EmitStackSizeSection = CodeGenOpts.StackSizeSection; if (CodeGenOpts.EnableSplitDwarf) Options.MCOptions.SplitDwarfFile = CodeGenOpts.SplitDwarfFile; Index: include/clang/Frontend/CodeGenOptions.def === --- include/clang/Frontend/CodeGenOptions.def +++ include/clang/Frontend/CodeGenOptions.def @@ -83,6 +83,7 @@ CODEGENOPT(XRayInstrumentFunctions , 1, 0) ///< Set when -fxray-instrument is ///< enabled. +CODEGENOPT(StackSizeSection , 1, 0) ///< Set when -fstack-size-section is enabled. ///< Set when -fxray-always-emit-customevents is enabled. CODEGENOPT(XRayAlwaysEmitCustomEvents , 1, 0) Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -1564,6 +1564,10 @@ Flags<[CC1Option]>, HelpText<"Place each data in its own section (ELF Only)">; def fno_data_sections : Flag <["-"], "fno-data-sections">, Group, Flags<[CC1Option]>; +def fstack_size_section : Flag<["-"], "fstack-size-section">, Group, Flags<[CC1Option]>, + HelpText<"Emit section containing metadata on function stack sizes">; +def fno_stack_size_section : Flag<["-"], "fno-stack-size-section">, Group, Flags<[CC1Option]>, + HelpText<"Don't emit section containing metadata on function stack sizes">; def funique_section_names : Flag <["-"], "funique-section-names">, Group, Flags<[CC1Option]>, Index: test/Driver/stack-size-section.c === --- test/Driver/
[PATCH] D40968: [OpenMP] Diagnose function name on the link clause
ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D40968 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40561: [libclang] Fix cursors for functions with trailing return type
nik added a comment. Thanks for the review. Please submit as I don't have the permissions for this. https://reviews.llvm.org/D40561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41005: Reuse preamble even if an unsaved file does not exist
nik marked 2 inline comments as done. nik added inline comments. Comment at: include/clang/Frontend/PrecompiledPreamble.h:109 + std::chrono::steady_clock::time_point getCreationTimePoint() const { +return CreationTimePoint; ilya-biryukov wrote: > Having this time stamp in the interface of `PrecompiledPreamble` doesn't > really makes sense. > > I propose we remove this method and test in a different way instead. > > For example, we could add a counter to `ASTUnit` that increases when preamble > is built and check this counter. > Or we could add a unit-test that uses `PrecompiledPreamble` directly. > For example, we could add a counter to ASTUnit that increases when preamble > is built and check this counter. OK, I've followed this proposal because there is already test infrastructure in PCHPreambleTest that I can easily reuse. Comment at: lib/Frontend/PrecompiledPreamble.cpp:437 + vfs::OverlayFileSystem OFS(VFS); + IntrusiveRefCntPtr IMFS( ilya-biryukov wrote: > Can we do the same thing without creating an additional `OverlayFileSystem`? > > If we add a separate map to check for those: > ``` > llvm::StringMap OverriddenFilesWithoutFS. > // Populate the map with paths not existing in VFS. > > for (const auto &F : FilesInPreamble) { > vfs::Status Status; > if (!moveOnNoError(OFS.status(F.first()), Status)) { > // If we can't stat the file, check whether it was overriden > auto It = OverriddenFilesWithoutFS[F.first()]; > if (It == OverriddenFilesWithoutFS.end()) > return false; > //. > } > //.. > > } > ``` > Can we do the same thing without creating an additional OverlayFileSystem? Hmm, I thought I had a good reason for this one. I don't remember it and the test still passes, so I did it without it now. Is there any real advantage in first trying the stat, then checking OverriddenFilesWithoutFS? Since I don't see any, I've changed order because the stat can then be avoided; also, it removes some nesting. Comment at: lib/Frontend/PrecompiledPreamble.cpp:444 vfs::Status Status; -if (!moveOnNoError(VFS->status(RB.first), Status)) - return false; - +assert(moveOnNoError(IMFS->status(RB.first), Status)); OverriddenFiles[Status.getUniqueID()] = ilya-biryukov wrote: > `assert` macro is a no-op when `NDEBUG` is defined. > One must never put an operation with side-effects inside `assert`. Huch, forgot to remove it on cleanup. Anyway, it's obsolete now. Repository: rC Clang https://reviews.llvm.org/D41005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41005: Reuse preamble even if an unsaved file does not exist
nik updated this revision to Diff 126353. nik marked 2 inline comments as done. nik added a comment. Addressed Ilya's comments. Repository: rC Clang https://reviews.llvm.org/D41005 Files: include/clang/Frontend/ASTUnit.h lib/Frontend/ASTUnit.cpp lib/Frontend/PrecompiledPreamble.cpp unittests/Frontend/PCHPreambleTest.cpp Index: unittests/Frontend/PCHPreambleTest.cpp === --- unittests/Frontend/PCHPreambleTest.cpp +++ unittests/Frontend/PCHPreambleTest.cpp @@ -124,6 +124,22 @@ } }; +TEST_F(PCHPreambleTest, ReparseDoesNotInvalidatePreambleDueToNotExistingUnsavedFile) { + std::string Header1 = "//./header1.h"; + std::string MainName = "//./main.cpp"; + AddFile(MainName, "#include \"//./header1.h\"\n" +"int main() { return ZERO; }"); + + std::unique_ptr AST(ParseAST(MainName)); + ASSERT_TRUE(AST.get()); + ASSERT_EQ(AST->getPreambleCounter(), 1U); + + RemapFile(Header1, "#define ZERO 0\n"); + ASSERT_TRUE(ReparseAST(AST)); + + ASSERT_EQ(AST->getPreambleCounter(), 1U); +} + TEST_F(PCHPreambleTest, ReparseWithOverriddenFileDoesNotInvalidatePreamble) { std::string Header1 = "//./header1.h"; std::string Header2 = "//./header2.h"; Index: lib/Frontend/PrecompiledPreamble.cpp === --- lib/Frontend/PrecompiledPreamble.cpp +++ lib/Frontend/PrecompiledPreamble.cpp @@ -434,21 +434,28 @@ Status.getSize(), llvm::sys::toTimeT(Status.getLastModificationTime())); } + llvm::StringMap OverridenFileBuffers; for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) { -vfs::Status Status; -if (!moveOnNoError(VFS->status(RB.first), Status)) - return false; - -OverriddenFiles[Status.getUniqueID()] = +OverridenFileBuffers[RB.first] = PreambleFileHash::createForMemoryBuffer(RB.second); } // Check whether anything has changed. for (const auto &F : FilesInPreamble) { +auto OverridenFileBuffer = OverridenFileBuffers.find(F.first()); +if (OverridenFileBuffer != OverridenFileBuffers.end()) { + // The file's buffer was remapped; check whether it matches up + // with the previous mapping. + if (OverridenFileBuffer->second != F.second) +return false; + continue; +} + vfs::Status Status; if (!moveOnNoError(VFS->status(F.first()), Status)) { - // If we can't stat the file, assume that something horrible happened. - return false; +// If the file's buffer is not remapped and we can't stat it, +// assume that something horrible happened. +return false; } std::map::iterator Overridden = @@ -461,9 +468,10 @@ continue; } -// The file was not remapped; check whether it has changed on disk. +// Neither the file's buffer nor the file itself was remapped; +// check whether it has changed on disk. if (Status.getSize() != uint64_t(F.second.Size) || -llvm::sys::toTimeT(Status.getLastModificationTime()) != + llvm::sys::toTimeT(Status.getLastModificationTime()) != F.second.ModTime) return false; } Index: lib/Frontend/ASTUnit.cpp === --- lib/Frontend/ASTUnit.cpp +++ lib/Frontend/ASTUnit.cpp @@ -190,6 +190,7 @@ OwnsRemappedFileBuffers(true), NumStoredDiagnosticsFromDriver(0), PreambleRebuildCounter(0), +PreambleCounter(0), NumWarningsInPreamble(0), ShouldCacheCodeCompletionResults(false), IncludeBriefCommentsInCodeCompletion(false), UserFilesAreVolatile(false), @@ -1296,6 +1297,7 @@ PCHContainerOps, /*StoreInMemory=*/false, Callbacks); if (NewPreamble) { Preamble = std::move(*NewPreamble); + ++PreambleCounter; PreambleRebuildCounter = 1; } else { switch (static_cast(NewPreamble.getError().value())) { Index: include/clang/Frontend/ASTUnit.h === --- include/clang/Frontend/ASTUnit.h +++ include/clang/Frontend/ASTUnit.h @@ -192,6 +192,9 @@ /// some number of calls. unsigned PreambleRebuildCounter; + /// \brief Counter indicating how often the preamble was build in total. + unsigned PreambleCounter; + /// \brief Cache pairs "filename - source location" /// /// Cache contains only source locations from preamble so it is @@ -547,7 +550,11 @@ return SourceRange(mapLocationToPreamble(R.getBegin()), mapLocationToPreamble(R.getEnd())); } - + + unsigned getPreambleCounter() const { +return PreambleCounter; + } + // Retrieve the diagnostics associated with this AST typedef StoredDiagnostic *stored_diag_iterator; typedef const StoredDiagnostic *stored_diag_const_iterator; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm
[PATCH] D41005: Reuse preamble even if an unsaved file does not exist
nik added inline comments. Comment at: include/clang/Frontend/ASTUnit.h:196 + /// \brief Counter indicating how often the preamble was build in total. + unsigned PreambleCounter; + Any better name for this one? Otherwise I would suggest renaming PreambleRebuildCounter to PreambleRebuildCountdownCounter to make them more distinct. Repository: rC Clang https://reviews.llvm.org/D41005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r320354 - [mips] Minor update to the comment (NFC)
Author: abeserminji Date: Mon Dec 11 04:12:16 2017 New Revision: 320354 URL: http://llvm.org/viewvc/llvm-project?rev=320354&view=rev Log: [mips] Minor update to the comment (NFC) Modified: cfe/trunk/lib/Basic/Targets/Mips.cpp Modified: cfe/trunk/lib/Basic/Targets/Mips.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/Mips.cpp?rev=320354&r1=320353&r2=320354&view=diff == --- cfe/trunk/lib/Basic/Targets/Mips.cpp (original) +++ cfe/trunk/lib/Basic/Targets/Mips.cpp Mon Dec 11 04:12:16 2017 @@ -206,7 +206,7 @@ ArrayRef MipsTargetInfo:: } bool MipsTargetInfo::validateTarget(DiagnosticsEngine &Diags) const { - // microMIPS64R6 backend is removed + // microMIPS64R6 backend was removed. if ((getTriple().getArch() == llvm::Triple::mips64 || getTriple().getArch() == llvm::Triple::mips64el) && IsMicromips && (ABI == "n32" || ABI == "n64")) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis
gerazo added inline comments. Comment at: tools/scan-build-py/libscanbuild/analyze.py:44 +CTU_FUNCTION_MAP_FILENAME = 'externalFnMap.txt' +CTU_TEMP_FNMAP_FOLDER = 'tmpExternalFnMaps' george.karpenkov wrote: > gerazo wrote: > > george.karpenkov wrote: > > > What would happen when multiple analyzer runs are launched concurrently > > > in the same directory? Would they race on this file? > > Yes and no. The 1st, collect part of CTU creates this file by aggregating > > all data from the build system, while the 2nd part which does the analysis > > itself only reads it. Multiple analysis can use this file simultaneously > > without problem. However, multiple collect phases launched on a system does > > not make sense. In this case, the later one would write over the previous > > results with the same data. > > However, multiple collect phases launched on a system does not make sense > > Why not? What about a system with multiple users, where code is in the shared > directory? Or even simpler: I've launched a background process, forgot about > it, and then launched it again? > > > In this case, the later one would write over the previous results with the > > same data. > > That is probably fine, I am more worried about racing, where process B would > be reading a partially overriden file (not completely sure whether it is > possible) > I see your point. In order to create the multiple user scenario you've mentioned, those users need to give the exact same output folders for their jobs. Our original bet was that our users are not doing this as the scan-build tool itself is also cannot be used this way. Still the process left there is something to consider. I will plan some kind of locking mechanism to avoid situations like this. Comment at: tools/scan-build-py/libscanbuild/analyze.py:609 +# Recover namedtuple from json when coming from analyze_cc +if not hasattr(ctu_config, 'collect'): +ctu_config = CtuConfig(collect=ctu_config[0], george.karpenkov wrote: > gerazo wrote: > > george.karpenkov wrote: > > > In which case is this branch hit? Isn't improperly formed input argument > > > indicative of an internal error at this stage? > > An other part of scan-build-py, analyze_cc uses namedtuple to json format > > to communicate. However, the names are not coming back from json, so this > > code helps in this. This is the case when someone uses the whole toolset > > with compiler wrapping. All the environment variable hassle is also > > happening because of this. So these env vars are not for user modification > > (as you've suggested earlier). > OK so `opts['ctu']` is a tuple or a named tuple depending on how this > function is entered? BTW could you point me to the `analyze_cc` entry point? > > For the purpose of having more uniform code with less cases to care about, do > you think we could just use ordinary tuples instead of constructing a named > one, since we have to deconstruct an ordinary tuple in any case? Using a NamedTuple improves readability of the code a lot with less comments. It is unfortunate that serializing it is not solved by Python. I think moving this code to the entry point would make the whole thing much nicer. The entry point is at analyze_compiler_wrapper Comment at: tools/scan-build-py/libscanbuild/arguments.py:376 +metavar='', +default='ctu-dir', +help="""Defines the temporary directory used between ctu george.karpenkov wrote: > BTW can we also explicitly add `dest='ctu_dir'` here, as otherwise I was > initially very confused as to where the variable is set. Yes, of course. https://reviews.llvm.org/D30691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r320351 - [mips] Removal of microMIPS64R6
Author: abeserminji Date: Mon Dec 11 03:29:17 2017 New Revision: 320351 URL: http://llvm.org/viewvc/llvm-project?rev=320351&view=rev Log: [mips] Removal of microMIPS64R6 microMIPS64R6 is removed from backend, and therefore frontend will show an error when target is microMIPS64R6. This is Clang part of patch. Differential Revision: https://reviews.llvm.org/D35624 Modified: cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td cfe/trunk/lib/Basic/Targets/Mips.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td?rev=320351&r1=320350&r2=320351&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td Mon Dec 11 03:29:17 2017 @@ -185,6 +185,8 @@ def note_invalid_subexpr_in_const_expr : def err_target_unknown_triple : Error< "unknown target triple '%0', please use -triple or -arch">; def err_target_unknown_cpu : Error<"unknown target CPU '%0'">; +def err_target_unsupported_cpu_for_micromips : Error< + "micromips is not supported for target CPU '%0'">; def err_target_unknown_abi : Error<"unknown target ABI '%0'">; def err_target_unsupported_abi : Error<"ABI '%0' is not supported on CPU '%1'">; def err_target_unsupported_abi_for_triple : Error< Modified: cfe/trunk/lib/Basic/Targets/Mips.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/Mips.cpp?rev=320351&r1=320350&r2=320351&view=diff == --- cfe/trunk/lib/Basic/Targets/Mips.cpp (original) +++ cfe/trunk/lib/Basic/Targets/Mips.cpp Mon Dec 11 03:29:17 2017 @@ -206,6 +206,13 @@ ArrayRef MipsTargetInfo:: } bool MipsTargetInfo::validateTarget(DiagnosticsEngine &Diags) const { + // microMIPS64R6 backend is removed + if ((getTriple().getArch() == llvm::Triple::mips64 || + getTriple().getArch() == llvm::Triple::mips64el) && + IsMicromips && (ABI == "n32" || ABI == "n64")) { +Diags.Report(diag::err_target_unsupported_cpu_for_micromips) << CPU; +return false; + } // FIXME: It's valid to use O32 on a 64-bit CPU but the backend can't handle //this yet. It's better to fail here than on the backend assertion. if (processorSupportsGPR64() && ABI == "o32") { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
klimek added inline comments. Comment at: clangd/Context.h:65 + Context *Parent; + TypedValueMap Data; +}; sammccall wrote: > ilya-biryukov wrote: > > sammccall wrote: > > > ilya-biryukov wrote: > > > > sammccall wrote: > > > > > We add complexity here (implementation and conceptual) to allow > > > > > multiple properties to be set at the same level (vs having a key and > > > > > an AnyStorage and making Context a linked list). > > > > > Is this for performance? I'm not convinced it'll actually be faster > > > > > for our workloads, or that it matters. > > > > Conceptually, a `Context` is more convenient to use when it stores > > > > multiple values. This allows to put a bunch of things and assign > > > > meaning to `Context` (i.e., a `Context` for processing a single LSP > > > > request, global context). If `Context`s were a linked list, the > > > > intermediate `Context`s would be hard to assign the meaning to. > > > > > > > > That being said, storage strategy for `Context`s is an implementation > > > > detail and could be changed anytime. I don't have big preferences here, > > > > but I think that storing a linked list of maps has, in general, a > > > > better performance than storing a linked list. > > > > And given that it's already there, I'd leave it this way. > > > With the new shared_ptr semantics: > > > > > > Context D = move(C).derive(K1, V1).derive(K2, V2); > > > > > > Is just as meaningful as > > > > > > Context D = move(C).derive().add(K1, V1).add(K2, V2); > > > > > > Yeah, the list of maps in an implementation detail. It's one that comes > > > with a bunch of complexity (`ContextBuilder` and most of > > > `TypedValueMap`). It really doesn't seem to buy us anything (the > > > performance is both uninteresting and seems likely to be worse in this > > > specific case with very few entries). > > The thing I like about it is that the `Context`s are layered properly in a > > sense that there's a Context corresponding to the request, a Context > > corresponding to the forked subrequests, etc. > > If we change the interface, we'll be creating a bunch of temporary Contexts > > that don't correspond to a nice meaningful abstraction (like request) in my > > head, even though we don't give those contexts any names. > > > > I do agree we currently pay with some complexity for that. Though I'd argue > > it's all hidden from the users of the interface, as building and consuming > > contexts is still super-easy and you don't need to mention ContextBuilder > > or TypedValueMap. And the implementation complexity is totally manageable > > from my point of view, but I am the one who implemented it in the first > > place, so there's certainly a bias there. > I don't see temporary unnamed `Context`s being any different from temporary > unnamed `ContextBuilder`s. > > But we've gone around on this point a bit, and this really seems to be a > question of taste. @klimek, can we have a third opinion? > > The options we're looking at are: > - `Context` stores a map and a parent pointer. `derive()` returns a > `ContextBuilder` used to create new contexts containing 0 or more new KV > pairs. `TypedValueMap` stores the payloads. > - `Context` stores a single KV pair and a parent pointer. `derive(K, V)` is > used to create a new context with one new KV pair. A Key-pointer and > AnyStorage in `Context` store the payloads, the rest of `TypedValueMap` goes > away. I'd agree that Context::derive(K, V) would be simpler here, mainly because there is only one way to pass around contexts while we're building them up; specifically, there's no temptation to pass around ContextBuilders. Comment at: clangd/TypedValueMap.h:38 + +/// A type-safe map from Key to T. +class TypedValueMap { Should we say "from Key*" or &? If you say Key I expect something that compares by value. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40860: [clangd] Fix diagnostic positions
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Sorry for the delay getting this reviewed. LG, thanks for fixing this! Just style nits. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:124 +/// Convert a clang::SourceLocation to a clangd::Position +Position SourceLocToClangdPosition(const SourceManager &SrcMgr, + SourceLocation Location) { nit: functions are `lowerCamelCase`. Here and below. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:136 +/// Convert a clang::FullSourceLoc to a clangd::Position +Position SourceLocToClangdPosition(const FullSourceLoc &Location) { + return SourceLocToClangdPosition(Location.getManager(), Location); only used once and private - inline this for now? Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:150 +/// Convert a clang::CharSourceRange to a clangd::Range +Range CharSourceRangeToClangdRange(const SourceManager &SrcMgr, + const LangOptions &Options, It seems like this could be decomposed into - CharSourceRange -> `SourceRange` - existing `SourceRangeToClangdRange` The first probably belongs (or exists) elsewhere in clang, though I can't find it - fine to keep it here. Comment at: clang-tools-extra/test/clangd/diagnostics.test:32 # CHECK-NEXT: "end": { -# CHECK-NEXT:"character": 1, +# CHECK-NEXT:"character": 0, # CHECK-NEXT:"line": 0 Incidentally, these tests seem to be wrong! The ranges shouldn't be empty (at least this one). Unrelated to your patch though, I'll look into it separately. https://reviews.llvm.org/D40860 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41056: [clang-tidy] New check misc-uniqueptr-release-unused-retval
alexfh added a comment. In https://reviews.llvm.org/D41056#950605, @khuttun wrote: > In https://reviews.llvm.org/D41056#950570, @Eugene.Zelenko wrote: > > > May be //bugprone// is better module then //misc//? > > > Maybe. I can move it if all the reviewers think that it would be better > suited there. Yup, bugprone- should be a better category for this, IMO. I wonder whether libc++ folks are interested in marking unique_ptr::release() with `__attribute__ ((warn_unused_result))`. A compiler warning (with -Werror) would be a better protection against this kind of a bug. Repository: rL LLVM https://reviews.llvm.org/D41056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38425: [clangd] Document highlights for clangd
Nebiroth updated this revision to Diff 126371. Nebiroth marked 3 inline comments as done. Nebiroth added a comment. Herald added a subscriber: mgrang. Merged with latest llvm/clang Minor code cleanup Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D38425 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/Protocol.cpp clangd/Protocol.h clangd/ProtocolHandlers.cpp clangd/ProtocolHandlers.h test/clangd/documenthighlight.test test/clangd/initialize-params-invalid.test test/clangd/initialize-params.test Index: test/clangd/initialize-params.test === --- test/clangd/initialize-params.test +++ test/clangd/initialize-params.test @@ -20,6 +20,7 @@ # CHECK-NEXT: }, # CHECK-NEXT: "definitionProvider": true, # CHECK-NEXT: "documentFormattingProvider": true, +# CHECK-NEXT: "documentHighlightProvider": true, # CHECK-NEXT: "documentOnTypeFormattingProvider": { # CHECK-NEXT:"firstTriggerCharacter": "}", # CHECK-NEXT:"moreTriggerCharacter": [] Index: test/clangd/initialize-params-invalid.test === --- test/clangd/initialize-params-invalid.test +++ test/clangd/initialize-params-invalid.test @@ -20,6 +20,7 @@ # CHECK-NEXT: }, # CHECK-NEXT: "definitionProvider": true, # CHECK-NEXT: "documentFormattingProvider": true, +# CHECK-NEXT: "documentHighlightProvider": true, # CHECK-NEXT: "documentOnTypeFormattingProvider": { # CHECK-NEXT:"firstTriggerCharacter": "}", # CHECK-NEXT:"moreTriggerCharacter": [] Index: test/clangd/documenthighlight.test === --- /dev/null +++ test/clangd/documenthighlight.test @@ -0,0 +1,42 @@ +# RUN: clangd -run-synchronously < %s | FileCheck %s +# It is absolutely vital that this file has CRLF line endings. +# +Content-Length: 125 + +{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}} + +Content-Length: 479 + +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"#define MACRO 1\nnamespace ns1 {\nstruct MyClass {\nint xasd;\nvoid anotherOperation() {\n}\nstatic int foo(MyClass*) {\nreturn 0;\n}\n\n};\nstruct Foo {\nint xasd;\n};\n}\nint main() {\nint bonjour;\nbonjour = 2;\nint test1 = bonjour;\nns1::Foo bar = { xasd : 1};\nbar.xasd = 3;\nns1::MyClass* Params;\nParams->anotherOperation();\n}\n"}}} + +Content-Length: 156 + +{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":17,"character":2}}} +# Verify local variable +# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":11,"line":16},"start":{"character":4,"line":16}}},{"kind":3,"range":{"end":{"character":7,"line":17},"start":{"character":0,"line":17}}},{"kind":2,"range":{"end":{"character":19,"line":18},"start":{"character":12,"line":18}}}]} + +Content-Length: 157 + +{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":18,"character":17}}} +# Verify struct highlight +# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":11,"line":16},"start":{"character":4,"line":16}}},{"kind":3,"range":{"end":{"character":7,"line":17},"start":{"character":0,"line":17}}},{"kind":2,"range":{"end":{"character":19,"line":18},"start":{"character":12,"line":18}}}]} + +Content-Length: 157 + +{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":21,"character":10}}} +# Verify method highlight +# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":14,"line":2},"start":{"character":7,"line":2}}},{"kind":1,"range":{"end":{"character":22,"line":6},"start":{"character":15,"line":6}}},{"kind":1,"range":{"end":{"character":12,"line":21},"start":{"character":5,"line":21}}}]} + +Content-Length: 157 + +{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":18,"character":14}}} +# Verify Read-access of a symbol (kind = 2) +# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":11,"line":16},"start":{"character":4,"line":16}}},{"kind":3,"range":{"end":{"character":7,"line":17},"start":{"character":0,"line":17}}},{"kind":2,"range":{"end":{"character":19,"line":18},"start":{"character":12,"line":18}}}]} + +Content-Length: 48 + +{"jsonrpc":"2.0","id":1,"method":"shutdown"} + +Content-Length: 33 + +{"jsonrpc":"2.0":"method":"exit"} \ No newline at end of file Index: c
[PATCH] D38425: [clangd] Document highlights for clangd
Nebiroth added a comment. @ilya-biryukov Need someone to land this. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D38425 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
ilya-biryukov updated this revision to Diff 126373. ilya-biryukov added a comment. - Removed ContextBuilder and TypedValueMap. - Updated the docs. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 Files: clangd/CMakeLists.txt clangd/Context.cpp clangd/Context.h unittests/clangd/CMakeLists.txt unittests/clangd/ContextTests.cpp Index: unittests/clangd/ContextTests.cpp === --- /dev/null +++ unittests/clangd/ContextTests.cpp @@ -0,0 +1,57 @@ +//===-- ContextTests.cpp - Context tests *- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "Context.h" + +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { + +TEST(ContextTests, Simple) { + Key IntParam; + Key ExtraIntParam; + + Context Ctx = Context::empty().derive(IntParam, 10).derive(ExtraIntParam, 20); + + EXPECT_EQ(*Ctx.get(IntParam), 10); + EXPECT_EQ(*Ctx.get(ExtraIntParam), 20); +} + +TEST(ContextTests, MoveOps) { + Key> Param; + + Context Ctx = Context::empty().derive(Param, llvm::make_unique(10)); + EXPECT_EQ(**Ctx.get(Param), 10); + + Context NewCtx = std::move(Ctx); + EXPECT_EQ(**NewCtx.get(Param), 10); +} + +TEST(ContextTests, Builders) { + Key ParentParam; + Key ParentAndChildParam; + Key ChildParam; + + Context ParentCtx = + Context::empty().derive(ParentParam, 10).derive(ParentAndChildParam, 20); + Context ChildCtx = + ParentCtx.derive(ParentAndChildParam, 30).derive(ChildParam, 40); + + EXPECT_EQ(*ParentCtx.get(ParentParam), 10); + EXPECT_EQ(*ParentCtx.get(ParentAndChildParam), 20); + EXPECT_EQ(ParentCtx.get(ChildParam), nullptr); + + EXPECT_EQ(*ChildCtx.get(ParentParam), 10); + EXPECT_EQ(*ChildCtx.get(ParentAndChildParam), 30); + EXPECT_EQ(*ChildCtx.get(ChildParam), 40); +} + +} // namespace clangd +} // namespace clang Index: unittests/clangd/CMakeLists.txt === --- unittests/clangd/CMakeLists.txt +++ unittests/clangd/CMakeLists.txt @@ -11,6 +11,7 @@ add_extra_unittest(ClangdTests ClangdTests.cpp CodeCompleteTests.cpp + ContextTests.cpp FuzzyMatchTests.cpp JSONExprTests.cpp TestFS.cpp Index: clangd/Context.h === --- /dev/null +++ clangd/Context.h @@ -0,0 +1,183 @@ +//===--- Context.h - Mechanism for passing implicit data *- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// Context for storing and retrieving implicit data. Useful for passing implicit +// parameters on a per-request basis. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONTEXT_H_ +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONTEXT_H_ + +#include "llvm/ADT/STLExtras.h" +#include +#include + +namespace clang { +namespace clangd { + +/// A key for a value of type \p Type, stored inside a context. Keys are +/// non-movable and non-copyable. See documentation of the Context class for +/// more details and usage examples. +template class Key { +public: + static_assert(!std::is_reference::value, +"Reference arguments to Key<> are not allowed"); + + Key() = default; + + Key(Key const &) = delete; + Key &operator=(Key const &) = delete; + Key(Key &&) = delete; + Key &operator=(Key &&) = delete; +}; + +/// A context is an immutable container for per-request data that must be +/// propagated through layers that don't care about it. An example is a request +/// ID that we may want to use when logging. +/// +/// Conceptually, a context is a heterogeneous map, T>. Each key has +/// an associated value type, which allows the map to be typesafe. +/// +/// You can't add data to an existing context, instead you create a new +/// immutable context derived from it with extra data added. When you retrieve +/// data, the context will walk up the parent chain until the key is found. +/// +/// Contexts should be: +/// - passed by reference when calling synchronous functions +/// - passed by value (move) when calling asynchronous functions. The result +///callback of async operations will receive the context again. +/// - cloned only when 'forking' an asynchronous computation that we don't wait +///for. +/// +/// Copy operations for this class are deleted, use an explicit clone() method +/// when you need a copy of the context instead. +/// +/// To derive a child context use derive() function, e.g. +/// Context C
[PATCH] D40486: [clangd] Implemented logging using Context
ilya-biryukov updated this revision to Diff 126375. ilya-biryukov added a comment. - Use derive(key, val) instead of derive().add(key, val). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40486 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/ClangdUnitStore.cpp clangd/ClangdUnitStore.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/GlobalCompilationDatabase.cpp clangd/GlobalCompilationDatabase.h clangd/JSONRPCDispatcher.cpp clangd/JSONRPCDispatcher.h clangd/Logger.cpp clangd/Logger.h clangd/Protocol.h clangd/ProtocolHandlers.cpp clangd/ProtocolHandlers.h clangd/tool/ClangdMain.cpp unittests/clangd/ClangdTests.cpp unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -8,6 +8,7 @@ //===--===// #include "ClangdServer.h" #include "Compiler.h" +#include "Context.h" #include "Protocol.h" #include "TestFS.h" #include "gtest/gtest.h" @@ -17,6 +18,10 @@ namespace { using namespace llvm; +/// Creates an empty context that is used for running async methods inside the +/// testing code. +Context testCtx() { return Context::empty().clone(); } + class IgnoreDiagnostics : public DiagnosticsConsumer { void onDiagnosticsReady( PathRef File, Tagged> Diagnostics) override {} @@ -74,8 +79,7 @@ MockCompilationDatabase CDB; ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, - EmptyLogger::getInstance()); + /*StorePreamblesInMemory=*/true); auto FooCpp = getVirtualTestFilePath("foo.cpp"); const auto SourceContents = R"cpp( @@ -101,29 +105,33 @@ // No need to sync reparses here as there are no asserts on diagnostics (or // other async operations). - Server.addDocument(FooCpp, SourceContents); + Server.addDocument(FooCpp, SourceContents, testCtx()); { auto CodeCompletionResults1 = -Server.codeComplete(FooCpp, CompletePos, CCOpts, None).get().Value; +Server.codeComplete(FooCpp, CompletePos, CCOpts, testCtx(), None) +.get() +.first.Value; EXPECT_TRUE(ContainsItem(CodeCompletionResults1, "aba")); EXPECT_FALSE(ContainsItem(CodeCompletionResults1, "cbc")); } { auto CodeCompletionResultsOverriden = Server -.codeComplete(FooCpp, CompletePos, CCOpts, +.codeComplete(FooCpp, CompletePos, CCOpts, testCtx(), StringRef(OverridenSourceContents)) .get() -.Value; +.first.Value; EXPECT_TRUE(ContainsItem(CodeCompletionResultsOverriden, "cbc")); EXPECT_FALSE(ContainsItem(CodeCompletionResultsOverriden, "aba")); } { auto CodeCompletionResults2 = -Server.codeComplete(FooCpp, CompletePos, CCOpts, None).get().Value; +Server.codeComplete(FooCpp, CompletePos, CCOpts, testCtx(), None) +.get() +.first.Value; EXPECT_TRUE(ContainsItem(CodeCompletionResults2, "aba")); EXPECT_FALSE(ContainsItem(CodeCompletionResults2, "cbc")); } @@ -135,8 +143,7 @@ CDB.ExtraClangFlags.push_back("-xc++"); IgnoreDiagnostics DiagConsumer; ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, - EmptyLogger::getInstance()); + /*StorePreamblesInMemory=*/true); auto FooCpp = getVirtualTestFilePath("foo.cpp"); FS.Files[FooCpp] = ""; @@ -150,17 +157,17 @@ int main() { ClassWithMembers().{complete} } )cpp", "complete"); - Server.addDocument(FooCpp, Completion.Text); + Server.addDocument(FooCpp, Completion.Text, testCtx()); clangd::CodeCompleteOptions Opts; Opts.Limit = 2; /// For after-dot completion we must always get consistent results. auto Results = Server .codeComplete(FooCpp, Completion.MarkerPos, Opts, - StringRef(Completion.Text)) + testCtx(), StringRef(Completion.Text)) .get() - .Value; + .first.Value; EXPECT_TRUE(Results.isIncomplete); EXPECT_EQ(Opts.Limit, Results.items.size()); @@ -175,8 +182,7 @@ CDB.ExtraClangFlags.push_back("-xc++"); IgnoreDiagnostics DiagConsumer; ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, - EmptyLogger::getInstance()); + /*StorePreamblesInMemory=*/true);
[PATCH] D40488: [clangd] Implemented tracing using Context
ilya-biryukov updated this revision to Diff 126376. ilya-biryukov added a comment. - Use derive(key, value) instead of derive().add(key, value). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40488 Files: clangd/ClangdUnit.cpp clangd/JSONRPCDispatcher.cpp clangd/Trace.cpp clangd/Trace.h clangd/tool/ClangdMain.cpp unittests/clangd/TraceTests.cpp Index: unittests/clangd/TraceTests.cpp === --- unittests/clangd/TraceTests.cpp +++ unittests/clangd/TraceTests.cpp @@ -7,6 +7,7 @@ // //===--===// +#include "Context.h" #include "Trace.h" #include "llvm/ADT/DenseMap.h" @@ -74,10 +75,11 @@ std::string JSON; { raw_string_ostream OS(JSON); -auto Session = trace::Session::create(OS); +auto JSONTracer = trace::createJSONTracer(OS); +trace::TracingSession Session(*JSONTracer); { - trace::Span S("A"); - trace::log("B"); + trace::Span S(Context::empty(), "A"); + trace::log(Context::empty(), "B"); } } Index: clangd/tool/ClangdMain.cpp === --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -115,19 +115,25 @@ << EC.message(); } } + + // Setup tracing facilities. llvm::Optional TraceStream; - std::unique_ptr TraceSession; + std::unique_ptr Tracer; if (!TraceFile.empty()) { std::error_code EC; TraceStream.emplace(TraceFile, /*ref*/ EC, llvm::sys::fs::F_RW); if (EC) { TraceFile.reset(); llvm::errs() << "Error while opening trace file: " << EC.message(); } else { - TraceSession = trace::Session::create(*TraceStream, PrettyPrint); + Tracer = trace::createJSONTracer(*TraceStream, PrettyPrint); } } + llvm::Optional TracingSession; + if (Tracer) +TracingSession.emplace(*Tracer); + llvm::raw_ostream &Outs = llvm::outs(); llvm::raw_ostream &Logs = llvm::errs(); JSONOutput Out(Outs, Logs, Index: clangd/Trace.h === --- clangd/Trace.h +++ clangd/Trace.h @@ -8,60 +8,74 @@ //===--===// // // Supports writing performance traces describing clangd's behavior. -// Traces are written in the Trace Event format supported by chrome's trace -// viewer (chrome://tracing). +// Traces are consumed by implementations of the EventTracer interface. // -// The format is documented here: -// https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview // // All APIs are no-ops unless a Session is active (created by ClangdMain). // //===--===// #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRACE_H_ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRACE_H_ +#include "Context.h" #include "JSONExpr.h" #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" namespace clang { namespace clangd { namespace trace { -// A session directs the output of trace events. Only one Session can exist. -// It should be created before clangd threads are spawned, and destroyed after -// they exit. -// TODO: we may want to add pluggable support for other tracing backends. -class Session { +/// A consumer of trace events. The events are produced by Spans and trace::log. +class EventTracer { public: - // Starts a sessions capturing trace events and writing Trace Event JSON. - static std::unique_ptr create(llvm::raw_ostream &OS, - bool Pretty = false); - ~Session(); + virtual ~EventTracer() = default; + /// Consume a trace event. + virtual void event(const Context &Ctx, llvm::StringRef Phase, + json::obj &&Contents) = 0; +}; -private: - Session() = default; +/// Sets up a global EventTracer that consumes events produced by Span and +/// trace::log. Only one TracingSession can be active at a time and it should be +/// set up before calling any clangd-specific functions. +class TracingSession { +public: + TracingSession(EventTracer &Tracer); + ~TracingSession(); }; -// Records a single instant event, associated with the current thread. -void log(const llvm::Twine &Name); +/// Create an instance of EventTracer that produces an output in the Trace Event +/// format supported by Chrome's trace viewer (chrome://tracing). +/// +/// The format is documented here: +/// https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview +/// +/// The implementation supports concurrent calls and can be used as a global +/// tracer (i.e., can be put into a global Context). +std::unique_ptr createJSONTracer(llvm::raw_ostream &OS, + bool Pretty = false); -// Records an event whose duration is the lifetime of the
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clangd/Context.h:65 + Context *Parent; + TypedValueMap Data; +}; klimek wrote: > sammccall wrote: > > ilya-biryukov wrote: > > > sammccall wrote: > > > > ilya-biryukov wrote: > > > > > sammccall wrote: > > > > > > We add complexity here (implementation and conceptual) to allow > > > > > > multiple properties to be set at the same level (vs having a key > > > > > > and an AnyStorage and making Context a linked list). > > > > > > Is this for performance? I'm not convinced it'll actually be faster > > > > > > for our workloads, or that it matters. > > > > > Conceptually, a `Context` is more convenient to use when it stores > > > > > multiple values. This allows to put a bunch of things and assign > > > > > meaning to `Context` (i.e., a `Context` for processing a single LSP > > > > > request, global context). If `Context`s were a linked list, the > > > > > intermediate `Context`s would be hard to assign the meaning to. > > > > > > > > > > That being said, storage strategy for `Context`s is an implementation > > > > > detail and could be changed anytime. I don't have big preferences > > > > > here, but I think that storing a linked list of maps has, in general, > > > > > a better performance than storing a linked list. > > > > > And given that it's already there, I'd leave it this way. > > > > With the new shared_ptr semantics: > > > > > > > > Context D = move(C).derive(K1, V1).derive(K2, V2); > > > > > > > > Is just as meaningful as > > > > > > > > Context D = move(C).derive().add(K1, V1).add(K2, V2); > > > > > > > > Yeah, the list of maps in an implementation detail. It's one that comes > > > > with a bunch of complexity (`ContextBuilder` and most of > > > > `TypedValueMap`). It really doesn't seem to buy us anything (the > > > > performance is both uninteresting and seems likely to be worse in this > > > > specific case with very few entries). > > > The thing I like about it is that the `Context`s are layered properly in > > > a sense that there's a Context corresponding to the request, a Context > > > corresponding to the forked subrequests, etc. > > > If we change the interface, we'll be creating a bunch of temporary > > > Contexts that don't correspond to a nice meaningful abstraction (like > > > request) in my head, even though we don't give those contexts any names. > > > > > > I do agree we currently pay with some complexity for that. Though I'd > > > argue it's all hidden from the users of the interface, as building and > > > consuming contexts is still super-easy and you don't need to mention > > > ContextBuilder or TypedValueMap. And the implementation complexity is > > > totally manageable from my point of view, but I am the one who > > > implemented it in the first place, so there's certainly a bias there. > > I don't see temporary unnamed `Context`s being any different from temporary > > unnamed `ContextBuilder`s. > > > > But we've gone around on this point a bit, and this really seems to be a > > question of taste. @klimek, can we have a third opinion? > > > > The options we're looking at are: > > - `Context` stores a map and a parent pointer. `derive()` returns a > > `ContextBuilder` used to create new contexts containing 0 or more new KV > > pairs. `TypedValueMap` stores the payloads. > > - `Context` stores a single KV pair and a parent pointer. `derive(K, V)` > > is used to create a new context with one new KV pair. A Key-pointer and > > AnyStorage in `Context` store the payloads, the rest of `TypedValueMap` > > goes away. > I'd agree that Context::derive(K, V) would be simpler here, mainly because > there is only one way to pass around contexts while we're building them up; > specifically, there's no temptation to pass around ContextBuilders. Done. `ContextBuilder` and `TypedValueMap` are now removed. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40897: [clangd] Introduce a "Symbol" class.
hokein updated this revision to Diff 126378. hokein marked 5 inline comments as done. hokein added a comment. Address comments on SymbolID. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40897 Files: clangd/CMakeLists.txt clangd/index/CMakeLists.txt clangd/index/Index.cpp clangd/index/Index.h clangd/index/SymbolCollector.cpp clangd/index/SymbolCollector.h unittests/clangd/CMakeLists.txt unittests/clangd/SymbolCollectorTests.cpp Index: unittests/clangd/SymbolCollectorTests.cpp === --- /dev/null +++ unittests/clangd/SymbolCollectorTests.cpp @@ -0,0 +1,112 @@ +//===-- SymbolCollectorTests.cpp ---*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "index/SymbolCollector.h" +#include "clang/Index/IndexingAction.h" +#include "clang/Basic/FileManager.h" +#include "clang/Basic/FileSystemOptions.h" +#include "clang/Basic/VirtualFileSystem.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Tooling/Tooling.h" +#include "llvm/ADT/IntrusiveRefCntPtr.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/MemoryBuffer.h" +#include "gtest/gtest.h" +#include "gmock/gmock.h" + +#include +#include + +using testing::UnorderedElementsAre; +using testing::Eq; +using testing::Field; + +// GMock helpers for matching Symbol. +MATCHER_P(QName, Name, "") { return arg.second.QualifiedName == Name; } + +namespace clang { +namespace clangd { + +namespace { +class SymbolIndexActionFactory : public tooling::FrontendActionFactory { + public: + SymbolIndexActionFactory() = default; + + clang::FrontendAction *create() override { +index::IndexingOptions IndexOpts; +IndexOpts.SystemSymbolFilter = +index::IndexingOptions::SystemSymbolFilterKind::All; +IndexOpts.IndexFunctionLocals = false; +Collector = std::make_shared(); +FrontendAction *Action = +index::createIndexingAction(Collector, IndexOpts, nullptr).release(); +return Action; + } + + std::shared_ptr Collector; +}; + +class SymbolCollectorTest : public ::testing::Test { +public: + bool runSymbolCollector(StringRef HeaderCode, StringRef MainCode) { +llvm::IntrusiveRefCntPtr InMemoryFileSystem( +new vfs::InMemoryFileSystem); +llvm::IntrusiveRefCntPtr Files( +new FileManager(FileSystemOptions(), InMemoryFileSystem)); + +const std::string FileName = "symbol.cc"; +const std::string HeaderName = "symbols.h"; +auto Factory = llvm::make_unique(); + +tooling::ToolInvocation Invocation( +{"symbol_collector", "-fsyntax-only", "-std=c++11", FileName}, +Factory->create(), Files.get(), +std::make_shared()); + +InMemoryFileSystem->addFile(HeaderName, 0, +llvm::MemoryBuffer::getMemBuffer(HeaderCode)); + +std::string Content = "#include\"" + std::string(HeaderName) + "\""; +Content += "\n" + MainCode.str(); +InMemoryFileSystem->addFile(FileName, 0, +llvm::MemoryBuffer::getMemBuffer(Content)); +Invocation.run(); +Symbols = Factory->Collector->takeSymbols(); + +EXPECT_EQ(FileName, Factory->Collector->getFilename()); +return true; + } + +protected: + SymbolSlab Symbols; +}; + +TEST_F(SymbolCollectorTest, CollectSymbol) { + const std::string Header = R"( +class Foo { + void f(); +}; +void f1(); +inline void f2() {} + )"; + const std::string Main = R"( +namespace { +void ff() {} // ignore +} +void f1() {} + )"; + runSymbolCollector(Header, Main); + EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Foo::f"), +QName("f1"), QName("f2"))); +} + +} // namespace +} // namespace clangd +} // namespace clang Index: unittests/clangd/CMakeLists.txt === --- unittests/clangd/CMakeLists.txt +++ unittests/clangd/CMakeLists.txt @@ -15,12 +15,14 @@ JSONExprTests.cpp TestFS.cpp TraceTests.cpp + SymbolCollectorTests.cpp ) target_link_libraries(ClangdTests PRIVATE clangBasic clangDaemon + clangdIndex clangFormat clangFrontend clangSema Index: clangd/index/SymbolCollector.h === --- /dev/null +++ clangd/index/SymbolCollector.h @@ -0,0 +1,52 @@ +//===--- SymbolCollector.h ---*- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===-
[PATCH] D41056: [clang-tidy] New check misc-uniqueptr-release-unused-retval
aaron.ballman added a comment. In https://reviews.llvm.org/D41056#951083, @alexfh wrote: > In https://reviews.llvm.org/D41056#950605, @khuttun wrote: > > > In https://reviews.llvm.org/D41056#950570, @Eugene.Zelenko wrote: > > > > > May be //bugprone// is better module then //misc//? > > > > > > Maybe. I can move it if all the reviewers think that it would be better > > suited there. > > > Yup, bugprone- should be a better category for this, IMO. > > I wonder whether libc++ folks are interested in marking unique_ptr::release() > with `__attribute__ ((warn_unused_result))`. A compiler warning (with > -Werror) would be a better protection against this kind of a bug. There's a push in WG21 to mark more of the library with `[[nodiscard]]`: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0600r1.pdf If we have a check for this, I do not think it should be specific to `unique_ptr::release()`, but instead be more broadly applicable to APIs that should be marked `[[nodiscard]]` but are not (currently). P0600R1 is a good place to start, but I'm guessing there are POSIX APIs (among others) that would also qualify. Repository: rL LLVM https://reviews.llvm.org/D41056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40486: [clangd] Implemented logging using Context
ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.cpp:36 - ~FulfillPromiseGuard() { Promise.set_value(); } + ~FulfillContextPromiseGuard() { Promise.set_value(std::move(Ctx)); } sammccall wrote: > Yikes, I can see how we got here, but we really don't get to move out of > something we received an lvalue-reference to... > > I think a safer idiom here is turning a whole lambda into a destructor: > >auto Fulfil = MakeDestructor([&]{ > DonePromise.set_value(std::move(Ctx)); >}); > > I'm not sure if LLVM has a shared utility for this, I've seen it in other > codebases. Either way we could just define it here. > > (Or we could copy the context to avoid the awkwardness) Thanks for suggestion. I totally agree, callbacks on scope exit are better. I couldn't find an utility like that in LLVM, but I think it's a useful one. I'll add this locally to this file, but maybe we should make this helper public and put it into clangd? What are you thoughts on that? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40486 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.
ioeric updated this revision to Diff 126379. ioeric marked 8 inline comments as done. ioeric added a comment. - Address review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40548 Files: clangd/CMakeLists.txt clangd/ClangdIndex.cpp clangd/ClangdIndex.h clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/ClangdUnitStore.cpp clangd/ClangdUnitStore.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/Symbol.cpp clangd/Symbol.h clangd/SymbolCompletionInfo.cpp clangd/SymbolCompletionInfo.h clangd/SymbolIndex.h clangd/tool/ClangdMain.cpp unittests/clangd/CMakeLists.txt unittests/clangd/SymbolCollectorTests.cpp Index: unittests/clangd/SymbolCollectorTests.cpp === --- /dev/null +++ unittests/clangd/SymbolCollectorTests.cpp @@ -0,0 +1,110 @@ +//===-- SymbolCollectorTests.cpp ---*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "Symbol.h" +#include "clang/Index/IndexingAction.h" +#include "clang/Basic/FileManager.h" +#include "clang/Basic/FileSystemOptions.h" +#include "clang/Basic/VirtualFileSystem.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Tooling/Tooling.h" +#include "llvm/ADT/IntrusiveRefCntPtr.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/MemoryBuffer.h" +#include "gtest/gtest.h" +#include "gmock/gmock.h" + +#include +#include + +using testing::ElementsAre; +using testing::Eq; +using testing::Field; + +namespace clang { +namespace clangd { + +namespace { +class SymbolIndexActionFactory : public tooling::FrontendActionFactory { + public: + SymbolIndexActionFactory() = default; + + clang::FrontendAction *create() override { +index::IndexingOptions IndexOpts; +IndexOpts.SystemSymbolFilter = +index::IndexingOptions::SystemSymbolFilterKind::All; +IndexOpts.IndexFunctionLocals = false; +Collector = std::make_shared(); +FrontendAction *Action = +index::createIndexingAction(Collector, IndexOpts, nullptr).release(); +return Action; + } + + std::shared_ptr Collector; +}; + +class SymbolCollectorTest : public ::testing::Test { +public: + bool runSymbolCollector(StringRef HeaderCode, StringRef MainCode) { +llvm::IntrusiveRefCntPtr InMemoryFileSystem( +new vfs::InMemoryFileSystem); +llvm::IntrusiveRefCntPtr Files( +new FileManager(FileSystemOptions(), InMemoryFileSystem)); + +const std::string FileName = "symbol.cc"; +const std::string HeaderName = "symbols.h"; +auto Factory = llvm::make_unique(); + +tooling::ToolInvocation Invocation( +{"symbol_collector", "-fsyntax-only", "-std=c++11", FileName}, +Factory->create(), Files.get(), +std::make_shared()); + +InMemoryFileSystem->addFile(HeaderName, 0, +llvm::MemoryBuffer::getMemBuffer(HeaderCode)); + +std::string Content = "#include\"" + std::string(HeaderName) + "\""; +Content += "\n" + MainCode.str(); +InMemoryFileSystem->addFile(FileName, 0, +llvm::MemoryBuffer::getMemBuffer(Content)); +Invocation.run(); +Symbols = Factory->Collector->getSymbols(); +return true; + } + +protected: + std::set Symbols; +}; + +TEST_F(SymbolCollectorTest, CollectSymbol) { + const std::string Header = R"( +class Foo { + void f(); +}; +void f1(); +inline void f2() {} + )"; + const std::string Main = R"( +namespace { +void ff() {} // ignore +} +void f1() {} + )"; + runSymbolCollector(Header, Main); + EXPECT_THAT(Symbols, + UnorderedElementsAre(Field(&Symbol::QualifiedName, Eq("Foo")), + Field(&Symbol::QualifiedName, Eq("Foo::f")), + Field(&Symbol::QualifiedName, Eq("f1")), + Field(&Symbol::QualifiedName, Eq("f2"; +} + +} // namespace +} // namespace clangd +} // namespace clang Index: unittests/clangd/CMakeLists.txt === --- unittests/clangd/CMakeLists.txt +++ unittests/clangd/CMakeLists.txt @@ -15,6 +15,7 @@ JSONExprTests.cpp TestFS.cpp TraceTests.cpp + SymbolCollectorTests.cpp ) target_link_libraries(ClangdTests Index: clangd/tool/ClangdMain.cpp === --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -90,6 +90,15 @@ "Trace internal events and timestamps in chrome://tracing JSON format"), llvm::cl::init("
[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.
ioeric added inline comments. Comment at: clangd/ClangdIndex.h:1 +//===--- ClangdIndex.h - Symbol indexes for clangd.---*- C++-*-===// +// sammccall wrote: > nit: `Clangd` prefix doesn't do much. > I'd suggest `Index.h` for the interface (including Symbol), and `MemIndex.h` > for the in-memory implementations? I'll do the renaming when D40897 is landed since it defines Symbol and would probably create `Index.h` as well. Comment at: clangd/ClangdIndex.h:50 +// relatively small symbol table built offline. +class InMemoryIndexSourcer { +public: sammccall wrote: > As discussed offline - the lifetime of the contained symbols and the > operations on the index derived from it need to be carefully considered. > > Involving inheritance here seems likely to make these tricky interactions > even trickier. > > Would suggest: > - a concrete class (`SymbolCache`? `FileSymbols`?) to hold a collection of > symbols from multiple files, with the ability to iterate over them and > replace all the symbols from one file at once > - a concrete class `MemIndex` that can be constructed from a sequence of > symbols and implements `Index` > > You probably want to make them both immutable with snapshot semantics, or > have a reader-writer lock that spans both. The current revision implements a `FileSymbols` with the snapshot semantics. Not entirely sure if this is what you are looking for. Let me know... Comment at: clangd/ClangdIndex.h:72 + /// collected. + void update(PathRef Path, ASTContext &Context, + std::shared_ptr PP, sammccall wrote: > The AST -> symbol conversion doesn't seem to have much to do with the rest of > the class - we can move this to a free function I think, which would give the > class a narrower responsibility. Moved the conversion code to ClangdUnit.cpp Comment at: clangd/CodeComplete.cpp:378 +// FIXME: output a warning to logger if there are results from sema. +return qualifiedIdCompletionWithIndex(*Index, S, **OptSS, &Items); + } sammccall wrote: > I don't like the idea of doing index lookups from the middle of a sema > callback! > > Can we just record the CCContext in the Collector instead, and do this work > afterwards? Good point! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40548 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
sammccall accepted this revision. sammccall added a comment. Thanks, this looks like exactly the right amount of magic to me :-) Comment at: clangd/Context.h:91 + /// functions that require a context when no explicit context is available. + static const Context &empty(); + as discussed offline, this could also return a value, which might make some use cases (testing) slightly cleaner. Up to you Comment at: clangd/Context.h:109 + template const Type *get(const Key &Key) const { +const ContextData *DataPtr = this->DataPtr.get(); +while (DataPtr != nullptr) { nit: this is a for loop in disguise :-) Comment at: clangd/Context.h:131 + template + Context derive(const Key &Key, Args &&... As) const & { +return Context(std::make_shared(ContextData{ I'd find the interface (and in particular the error messages) here easier to understand if we took a `Type` by value, rather than having `emplace` semantics. If this function took a `Type`, and `TypedAnyStorage` took a `Type&&`, the cost would be one extra move, which doesn't seem too bad. Up to you, though. (If you change it, don't forget the other `derive` overload) Comment at: clangd/Context.h:168 + + struct ContextData { +// We need to make sure Parent outlives the Value, so the order of members nit: now this is private it could just be called Data Comment at: clangd/Context.h:169 + struct ContextData { +// We need to make sure Parent outlives the Value, so the order of members +// is important. We do that to allow classes stored in Context's child Is this comment still true/relevant? I thought the motivating case was Span, but Span now stores a copy of the parent pointer (and ContextData isn't accessible by it). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D40746: Correctly handle line table entries without filenames during AST serialization
+Tom I expect it's too late since 5.0.1 is virtually out the door already. On Mon, Dec 11, 2017 at 2:35 AM, Ivan Donchevskii via Phabricator < revi...@reviews.llvm.org> wrote: > yvvan added a comment. > > Can we still have it in 5.0? > > > Repository: > rL LLVM > > https://reviews.llvm.org/D40746 > > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r320297 - Fix MSVC 'not all control paths return a value' warning
Thanks! On 10 December 2017 at 03:05, Simon Pilgrim via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rksimon > Date: Sun Dec 10 03:05:14 2017 > New Revision: 320297 > > URL: http://llvm.org/viewvc/llvm-project?rev=320297&view=rev > Log: > Fix MSVC 'not all control paths return a value' warning > > Modified: > cfe/trunk/lib/Driver/ToolChains/Darwin.cpp > > Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ > ToolChains/Darwin.cpp?rev=320297&r1=320296&r2=320297&view=diff > > == > --- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original) > +++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Sun Dec 10 03:05:14 2017 > @@ -1230,6 +1230,7 @@ struct DarwinPlatform { > case DeploymentTargetEnv: >return (llvm::Twine(EnvVarName) + "=" + OSVersion).str(); > } > +llvm_unreachable("Unsupported Darwin Source Kind"); >} > >static DarwinPlatform createOSVersionArg(DarwinPlatformKind Platform, > > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r320391 - For Linux/gnu compatibility, preinclude if the file is available
Author: erichkeane Date: Mon Dec 11 09:36:42 2017 New Revision: 320391 URL: http://llvm.org/viewvc/llvm-project?rev=320391&view=rev Log: For Linux/gnu compatibility, preinclude if the file is available As reported in llvm bugzilla 32377. Here’s a patch to add preinclude of stdc-predef.h. The gcc documentation says “On GNU/Linux, is pre-included.” See https://gcc.gnu.org/gcc-4.8/porting_to.html; The preinclude is inhibited with –ffreestanding. Basically I fixed the failing test cases by adding –ffreestanding which inhibits this behavior. I fixed all the failing tests, including some in extra/test, there's a separate patch for that which is linked here Note: this is a recommit after a test failure took down the original (r318669) Patch By: mibintc Differential Revision: https://reviews.llvm.org/D34158 Added: cfe/trunk/test/Driver/Inputs/stdc-predef/ cfe/trunk/test/Driver/Inputs/stdc-predef/usr/ cfe/trunk/test/Driver/Inputs/stdc-predef/usr/include/ cfe/trunk/test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h cfe/trunk/test/Driver/stdc-predef.c cfe/trunk/test/Driver/stdc-predef.i Modified: cfe/trunk/include/clang/Driver/CC1Options.td cfe/trunk/include/clang/Lex/PreprocessorOptions.h cfe/trunk/lib/Driver/Job.cpp cfe/trunk/lib/Driver/ToolChains/Linux.cpp cfe/trunk/lib/Driver/ToolChains/Linux.h cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/lib/Frontend/InitPreprocessor.cpp cfe/trunk/test/Driver/crash-report-header.h cfe/trunk/test/Driver/crash-report-spaces.c cfe/trunk/test/Driver/crash-report.c cfe/trunk/test/Driver/rewrite-map-in-diagnostics.c cfe/trunk/test/Index/IBOutletCollection.m cfe/trunk/test/Index/annotate-macro-args.m cfe/trunk/test/Index/annotate-tokens-pp.c cfe/trunk/test/Index/annotate-tokens.c cfe/trunk/test/Index/c-index-getCursor-test.m cfe/trunk/test/Index/get-cursor-macro-args.m cfe/trunk/test/Index/get-cursor.cpp cfe/trunk/test/Preprocessor/ignore-pragmas.c cfe/trunk/unittests/Tooling/TestVisitor.h cfe/trunk/unittests/libclang/LibclangTest.cpp Modified: cfe/trunk/include/clang/Driver/CC1Options.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=320391&r1=320390&r2=320391&view=diff == --- cfe/trunk/include/clang/Driver/CC1Options.td (original) +++ cfe/trunk/include/clang/Driver/CC1Options.td Mon Dec 11 09:36:42 2017 @@ -769,6 +769,8 @@ def token_cache : Separate<["-"], "token HelpText<"Use specified token cache file">; def detailed_preprocessing_record : Flag<["-"], "detailed-preprocessing-record">, HelpText<"include a detailed record of preprocessing actions">; +def fsystem_include_if_exists : Separate<["-"], "fsystem-include-if-exists">, MetaVarName<"">, + HelpText<"Include system file before parsing if file exists">; //===--===// // OpenCL Options Modified: cfe/trunk/include/clang/Lex/PreprocessorOptions.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/PreprocessorOptions.h?rev=320391&r1=320390&r2=320391&view=diff == --- cfe/trunk/include/clang/Lex/PreprocessorOptions.h (original) +++ cfe/trunk/include/clang/Lex/PreprocessorOptions.h Mon Dec 11 09:36:42 2017 @@ -60,6 +60,9 @@ public: /// \brief Headers that will be converted to chained PCHs in memory. std::vector ChainedIncludes; + /// \brief System Headers that are pre-included if they exist. + std::vector FSystemIncludeIfExists; + /// \brief When true, disables most of the normal validation performed on /// precompiled headers. bool DisablePCHValidation = false; @@ -183,6 +186,7 @@ public: DumpDeserializedPCHDecls = false; ImplicitPCHInclude.clear(); ImplicitPTHInclude.clear(); +FSystemIncludeIfExists.clear(); TokenCache.clear(); SingleFileParseMode = false; LexEditorPlaceholders = true; Modified: cfe/trunk/lib/Driver/Job.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Job.cpp?rev=320391&r1=320390&r2=320391&view=diff == --- cfe/trunk/lib/Driver/Job.cpp (original) +++ cfe/trunk/lib/Driver/Job.cpp Mon Dec 11 09:36:42 2017 @@ -64,7 +64,7 @@ static bool skipArgs(const char *Flag, b .Cases("-internal-externc-isystem", "-iprefix", true) .Cases("-iwithprefixbefore", "-isystem", "-iquote", true) .Cases("-isysroot", "-I", "-F", "-resource-dir", true) -.Cases("-iframework", "-include-pch", true) +.Cases("-iframework", "-include-pch", "-fsystem-include-if-exists", true) .Default(false); if (IsInclude) return HaveCrashVFS ? false : true; Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/tr
[PATCH] D41042: [analyzer] StackAddrEscape: Delay turning on by default a little bit?
NoQ added a comment. Yeah, we usually try to avoid omissions of modeling in on-by-default checkers because the user may accidentally run into projects in which the unmodeled idiom is common, and then he'd get false positives all over the place. In my case it was just two new positives, both false due to this `dispatch_barrier_sync` idiom. https://reviews.llvm.org/D41042 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available
This revision was automatically updated to reflect the committed changes. Closed by commit rL320391: For Linux/gnu compatibility, preinclude if the file is available (authored by erichkeane). Changed prior to commit: https://reviews.llvm.org/D34158?vs=123613&id=126386#toc Repository: rL LLVM https://reviews.llvm.org/D34158 Files: cfe/trunk/include/clang/Driver/CC1Options.td cfe/trunk/include/clang/Lex/PreprocessorOptions.h cfe/trunk/lib/Driver/Job.cpp cfe/trunk/lib/Driver/ToolChains/Linux.cpp cfe/trunk/lib/Driver/ToolChains/Linux.h cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/lib/Frontend/InitPreprocessor.cpp cfe/trunk/test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h cfe/trunk/test/Driver/crash-report-header.h cfe/trunk/test/Driver/crash-report-spaces.c cfe/trunk/test/Driver/crash-report.c cfe/trunk/test/Driver/rewrite-map-in-diagnostics.c cfe/trunk/test/Driver/stdc-predef.c cfe/trunk/test/Driver/stdc-predef.i cfe/trunk/test/Index/IBOutletCollection.m cfe/trunk/test/Index/annotate-macro-args.m cfe/trunk/test/Index/annotate-tokens-pp.c cfe/trunk/test/Index/annotate-tokens.c cfe/trunk/test/Index/c-index-getCursor-test.m cfe/trunk/test/Index/get-cursor-macro-args.m cfe/trunk/test/Index/get-cursor.cpp cfe/trunk/test/Preprocessor/ignore-pragmas.c cfe/trunk/unittests/Tooling/TestVisitor.h cfe/trunk/unittests/libclang/LibclangTest.cpp Index: cfe/trunk/unittests/Tooling/TestVisitor.h === --- cfe/trunk/unittests/Tooling/TestVisitor.h +++ cfe/trunk/unittests/Tooling/TestVisitor.h @@ -52,6 +52,7 @@ /// \brief Runs the current AST visitor over the given code. bool runOver(StringRef Code, Language L = Lang_CXX) { std::vector Args; +Args.push_back("-ffreestanding"); switch (L) { case Lang_C: Args.push_back("-x"); Index: cfe/trunk/unittests/libclang/LibclangTest.cpp === --- cfe/trunk/unittests/libclang/LibclangTest.cpp +++ cfe/trunk/unittests/libclang/LibclangTest.cpp @@ -434,8 +434,10 @@ "#ifdef KIWIS\n" "printf(\"mmm!!\");\n" "#endif"); + const char *Args[] = { "-ffreestanding" }; + int NumArgs = sizeof(Args) / sizeof(Args[0]); - ClangTU = clang_parseTranslationUnit(Index, Main.c_str(), nullptr, 0, + ClangTU = clang_parseTranslationUnit(Index, Main.c_str(), Args, NumArgs, nullptr, 0, TUFlags); CXSourceRangeList *Ranges = clang_getAllSkippedRanges(ClangTU); Index: cfe/trunk/include/clang/Driver/CC1Options.td === --- cfe/trunk/include/clang/Driver/CC1Options.td +++ cfe/trunk/include/clang/Driver/CC1Options.td @@ -769,6 +769,8 @@ HelpText<"Use specified token cache file">; def detailed_preprocessing_record : Flag<["-"], "detailed-preprocessing-record">, HelpText<"include a detailed record of preprocessing actions">; +def fsystem_include_if_exists : Separate<["-"], "fsystem-include-if-exists">, MetaVarName<"">, + HelpText<"Include system file before parsing if file exists">; //===--===// // OpenCL Options Index: cfe/trunk/include/clang/Lex/PreprocessorOptions.h === --- cfe/trunk/include/clang/Lex/PreprocessorOptions.h +++ cfe/trunk/include/clang/Lex/PreprocessorOptions.h @@ -60,6 +60,9 @@ /// \brief Headers that will be converted to chained PCHs in memory. std::vector ChainedIncludes; + /// \brief System Headers that are pre-included if they exist. + std::vector FSystemIncludeIfExists; + /// \brief When true, disables most of the normal validation performed on /// precompiled headers. bool DisablePCHValidation = false; @@ -183,6 +186,7 @@ DumpDeserializedPCHDecls = false; ImplicitPCHInclude.clear(); ImplicitPTHInclude.clear(); +FSystemIncludeIfExists.clear(); TokenCache.clear(); SingleFileParseMode = false; LexEditorPlaceholders = true; Index: cfe/trunk/test/Index/annotate-tokens.c === --- cfe/trunk/test/Index/annotate-tokens.c +++ cfe/trunk/test/Index/annotate-tokens.c @@ -68,7 +68,7 @@ reg.field = 1; } -// RUN: c-index-test -test-annotate-tokens=%s:4:1:37:1 %s | FileCheck %s +// RUN: c-index-test -test-annotate-tokens=%s:4:1:37:1 -ffreestanding %s | FileCheck %s // CHECK: Identifier: "T" [4:3 - 4:4] TypeRef=T:1:13 // CHECK: Punctuation: "*" [4:4 - 4:5] VarDecl=t_ptr:4:6 (Definition) // CHECK: Identifier: "t_ptr" [4:6 - 4:11] VarDecl=t_ptr:4:6 (Definition) @@ -191,10 +191,10 @@ // CHECK: Punctuation: ")" [36:97 - 36:98] FunctionDecl=test:36:63 (unavailable) (always unavailable: "") // CHECK: Punctuation: ";" [36:98 - 36:99] -// RUN: c-index-test -test-annotate-tokens=%s:4:1:165:32 %s | F
[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available
This revision was automatically updated to reflect the committed changes. Closed by commit rC320391: For Linux/gnu compatibility, preinclude if the file is available (authored by erichkeane). Changed prior to commit: https://reviews.llvm.org/D34158?vs=123613&id=126387#toc Repository: rC Clang https://reviews.llvm.org/D34158 Files: include/clang/Driver/CC1Options.td include/clang/Lex/PreprocessorOptions.h lib/Driver/Job.cpp lib/Driver/ToolChains/Linux.cpp lib/Driver/ToolChains/Linux.h lib/Frontend/CompilerInvocation.cpp lib/Frontend/InitPreprocessor.cpp test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h test/Driver/crash-report-header.h test/Driver/crash-report-spaces.c test/Driver/crash-report.c test/Driver/rewrite-map-in-diagnostics.c test/Driver/stdc-predef.c test/Driver/stdc-predef.i test/Index/IBOutletCollection.m test/Index/annotate-macro-args.m test/Index/annotate-tokens-pp.c test/Index/annotate-tokens.c test/Index/c-index-getCursor-test.m test/Index/get-cursor-macro-args.m test/Index/get-cursor.cpp test/Preprocessor/ignore-pragmas.c unittests/Tooling/TestVisitor.h unittests/libclang/LibclangTest.cpp Index: include/clang/Driver/CC1Options.td === --- include/clang/Driver/CC1Options.td +++ include/clang/Driver/CC1Options.td @@ -769,6 +769,8 @@ HelpText<"Use specified token cache file">; def detailed_preprocessing_record : Flag<["-"], "detailed-preprocessing-record">, HelpText<"include a detailed record of preprocessing actions">; +def fsystem_include_if_exists : Separate<["-"], "fsystem-include-if-exists">, MetaVarName<"">, + HelpText<"Include system file before parsing if file exists">; //===--===// // OpenCL Options Index: include/clang/Lex/PreprocessorOptions.h === --- include/clang/Lex/PreprocessorOptions.h +++ include/clang/Lex/PreprocessorOptions.h @@ -60,6 +60,9 @@ /// \brief Headers that will be converted to chained PCHs in memory. std::vector ChainedIncludes; + /// \brief System Headers that are pre-included if they exist. + std::vector FSystemIncludeIfExists; + /// \brief When true, disables most of the normal validation performed on /// precompiled headers. bool DisablePCHValidation = false; @@ -183,6 +186,7 @@ DumpDeserializedPCHDecls = false; ImplicitPCHInclude.clear(); ImplicitPTHInclude.clear(); +FSystemIncludeIfExists.clear(); TokenCache.clear(); SingleFileParseMode = false; LexEditorPlaceholders = true; Index: test/Driver/crash-report-spaces.c === --- test/Driver/crash-report-spaces.c +++ test/Driver/crash-report-spaces.c @@ -1,7 +1,7 @@ // RUN: rm -rf "%t" // RUN: mkdir "%t" // RUN: cp "%s" "%t/crash report spaces.c" -// RUN: not env TMPDIR="%t" TEMP="%t" TMP="%t" RC_DEBUG_OPTIONS=1 %clang -fsyntax-only "%t/crash report spaces.c" 2>&1 | FileCheck "%s" +// RUN: not env TMPDIR="%t" TEMP="%t" TMP="%t" RC_DEBUG_OPTIONS=1 %clang -ffreestanding -fsyntax-only "%t/crash report spaces.c" 2>&1 | FileCheck "%s" // RUN: cat "%t/crash report spaces"-*.c | FileCheck --check-prefix=CHECKSRC "%s" // RUN: cat "%t/crash report spaces"-*.sh | FileCheck --check-prefix=CHECKSH "%s" // REQUIRES: crash-recovery Index: test/Driver/rewrite-map-in-diagnostics.c === --- test/Driver/rewrite-map-in-diagnostics.c +++ test/Driver/rewrite-map-in-diagnostics.c @@ -1,7 +1,7 @@ // RUN: rm -rf "%t" // RUN: mkdir -p "%t" // RUN: not env TMPDIR="%t" TEMP="%t" TMP="%t" RC_DEBUG_OPTION=1 \ -// RUN: %clang -fsyntax-only -frewrite-map-file %p/Inputs/rewrite.map %s 2>&1 \ +// RUN: %clang -ffreestanding -fsyntax-only -frewrite-map-file %p/Inputs/rewrite.map %s 2>&1 \ // RUN: | FileCheck %s #pragma clang __debug parser_crash Index: test/Driver/stdc-predef.i === --- test/Driver/stdc-predef.i +++ test/Driver/stdc-predef.i @@ -0,0 +1,16 @@ +// The automatic preinclude of stdc-predef.h should not occur if +// the source filename indicates a preprocessed file. +// +// RUN: %clang %s -### -c 2>&1 \ +// RUN: --sysroot=%S/Inputs/stdc-predef \ +// RUN: | FileCheck --implicit-check-not "stdc-predef.h" %s + +int i; +// The automatic preinclude of stdc-predef.h should not occur if +// the source filename indicates a preprocessed file. +// +// RUN: %clang %s -### -c 2>&1 \ +// RUN: --sysroot=%S/Inputs/stdc-predef \ +// RUN: | FileCheck --implicit-check-not "stdc-predef.h" %s + +int i; Index: test/Driver/stdc-predef.c === --- test/Driver/stdc-predef.c +++ test/Driver/stdc-predef.c @@ -0,0 +1,25 @@ +// Test that clang preincludes stdc-predef.h, if th
[PATCH] D40897: [clangd] Introduce a "Symbol" class.
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. \o/ Comment at: clangd/index/Index.cpp:34 + +SymbolSlab::const_iterator SymbolSlab::find(const SymbolID& SymID) const { + return Symbols.find(SymID); assert frozen? (and in begin()) Comment at: clangd/index/Index.h:45 + SymbolID(llvm::StringRef USR); + std::array HashValue; +}; nit: make HashValue private? provide operator== (and use it from DenseMapInfo). Comment at: clangd/index/Index.h:108 + static inline clang::clangd::SymbolID getEmptyKey() { +return clang::clangd::SymbolID("EMPTYKEY"); + } nit: you may want to memoize this in a local static variable, rather than compute it each time: DenseMap calls it a lot. Comment at: clangd/index/SymbolCollector.cpp:37 + << '\n'; + // Handle symbolic link path cases. + // We are trying to get the real file path of the symlink. Can you spell out here which symbolic link cases we're handling, and what problems we're trying to avoid? Offline, we talked about the CWD being a symlink. But this is a different case... Comment at: clangd/index/SymbolCollector.h:35 + + StringRef getFilename() const { +return Filename; What's this for? Seems like we should be able to handle multiple TUs with one collector? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40998: [driver][darwin] Take the OS version specified in "-target" as the target OS instead of inferring it from SDK / environment
arphaman updated this revision to Diff 126388. arphaman marked an inline comment as done. arphaman added a comment. Don't warn about the redundant environment variable Repository: rC Clang https://reviews.llvm.org/D40998 Files: lib/Driver/ToolChains/Darwin.cpp test/Driver/darwin-version.c test/Driver/objc-weak.m test/Driver/pic.c test/Driver/unavailable_aligned_allocation.cpp Index: test/Driver/unavailable_aligned_allocation.cpp === --- test/Driver/unavailable_aligned_allocation.cpp +++ test/Driver/unavailable_aligned_allocation.cpp @@ -10,15 +10,15 @@ // RUN: %clang -target thumbv7-apple-watchos3 -c -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix=UNAVAILABLE // -// RUN: %clang -target x86_64-apple-macosx10.13 -mios-simulator-version-min=10 \ +// RUN: %clang -target x86_64-apple-darwin -mios-simulator-version-min=10 \ // RUN: -c -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix=UNAVAILABLE // -// RUN: %clang -target x86_64-apple-macosx10.13 -mtvos-simulator-version-min=10 \ +// RUN: %clang -target x86_64-apple-darwin -mtvos-simulator-version-min=10 \ // RUN: -c -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix=UNAVAILABLE // -// RUN: %clang -target x86_64-apple-macosx10.13 -mwatchos-simulator-version-min=3 \ +// RUN: %clang -target x86_64-apple-darwin -mwatchos-simulator-version-min=3 \ // RUN: -c -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix=UNAVAILABLE // @@ -39,15 +39,15 @@ // RUN: %clang -target x86_64-unknown-linux-gnu -c -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix=AVAILABLE // -// RUN: %clang -target x86_64-apple-macosx10.12 -mios-simulator-version-min=11 \ +// RUN: %clang -target x86_64-apple-darwin -mios-simulator-version-min=11 \ // RUN: -c -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix=AVAILABLE // -// RUN: %clang -target x86_64-apple-macosx10.12 -mtvos-simulator-version-min=11 \ +// RUN: %clang -target x86_64-apple-darwin -mtvos-simulator-version-min=11 \ // RUN: -c -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix=AVAILABLE // -// RUN: %clang -target x86_64-apple-macosx10.12 -mwatchos-simulator-version-min=4 \ +// RUN: %clang -target x86_64-apple-darwin -mwatchos-simulator-version-min=4 \ // RUN: -c -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix=AVAILABLE // Index: test/Driver/pic.c === --- test/Driver/pic.c +++ test/Driver/pic.c @@ -221,19 +221,19 @@ // // Checks for ARM+Apple+IOS including -fapple-kext, -mkernel, and iphoneos // version boundaries. -// RUN: %clang -c %s -target armv7-apple-ios -fapple-kext -miphoneos-version-min=6.0.0 -### 2>&1 \ +// RUN: %clang -c %s -target armv7-apple-ios6 -fapple-kext -### 2>&1 \ // RUN: | FileCheck %s --check-prefix=CHECK-PIC2 -// RUN: %clang -c %s -target armv7-apple-ios -mkernel -miphoneos-version-min=6.0.0 -### 2>&1 \ +// RUN: %clang -c %s -target armv7-apple-ios6 -mkernel -### 2>&1 \ // RUN: | FileCheck %s --check-prefix=CHECK-PIC2 -// RUN: %clang -c %s -target arm64-apple-ios -mkernel -miphoneos-version-min=7.0.0 -### 2>&1 \ +// RUN: %clang -c %s -target arm64-apple-ios7 -mkernel -### 2>&1 \ // RUN: | FileCheck %s --check-prefix=CHECK-PIC2 -// RUN: %clang -x assembler -c %s -target arm64-apple-ios -mkernel -miphoneos-version-min=7.0.0 -no-integrated-as -### 2>&1 \ +// RUN: %clang -x assembler -c %s -target arm64-apple-ios7 -mkernel -no-integrated-as -### 2>&1 \ // RUN: | FileCheck %s --check-prefix=CHECK-NO-STATIC -// RUN: %clang -c %s -target armv7k-apple-watchos -fapple-kext -mwatchos-version-min=1.0.0 -### 2>&1 \ +// RUN: %clang -c %s -target armv7k-apple-watchos1 -fapple-kext -### 2>&1 \ // RUN: | FileCheck %s --check-prefix=CHECK-PIC2 -// RUN: %clang -c %s -target armv7-apple-ios -fapple-kext -miphoneos-version-min=5.0.0 -### 2>&1 \ +// RUN: %clang -c %s -target armv7-apple-ios5 -fapple-kext -### 2>&1 \ // RUN: | FileCheck %s --check-prefix=CHECK-NO-PIC -// RUN: %clang -c %s -target armv7-apple-ios -fapple-kext -miphoneos-version-min=6.0.0 -static -### 2>&1 \ +// RUN: %clang -c %s -target armv7-apple-ios6 -fapple-kext -static -### 2>&1 \ // RUN: | FileCheck %s --check-prefix=CHECK-NO-PIC // RUN: %clang -c %s -target armv7-apple-unknown-macho -static -### 2>&1 \ // RUN: | FileCheck %s --check-prefix=CHECK-NO-PIC Index: test/Driver/objc-weak.m === --- test/Driver/objc-weak.m +++ test/Driver/objc-weak.m @@ -1,27 +1,27 @@ // Check miscellaneous Objective-C options. -// RUN: %clang -target x86_64-apple-macosx -mmacosx-version-min=10.7 -S -### %s -fobjc-arc -fobjc-weak 2>&1 | FileCheck %s --check-prefix ARC-WEAK -// RUN: %clang -target x86_64-apple-macosx -mmacosx-version-min=10.7 -S -### %s -fno-objc-weak -fobjc-weak -fobjc-arc 2>&1 | FileCheck %s --check-prefix ARC-WEAK +// RUN: %clang -target x86_64-apple-macosx10.7 -S -### %s -fobjc-a
[PATCH] D39053: [Bitfield] Add more cases to making the bitfield a separate location
spetrovic added a comment. ping https://reviews.llvm.org/D39053 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41042: [analyzer] StackAddrEscape: Delay turning on by default a little bit?
NoQ updated this revision to Diff 126390. NoQ added a comment. Add a FIXME test. https://reviews.llvm.org/D41042 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp test/Analysis/stack-capture-leak-arc.mm test/Analysis/stack-capture-leak-no-arc.mm Index: test/Analysis/stack-capture-leak-no-arc.mm === --- test/Analysis/stack-capture-leak-no-arc.mm +++ test/Analysis/stack-capture-leak-no-arc.mm @@ -1,4 +1,4 @@ -// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core -fblocks -verify %s +// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,alpha.core.StackAddressAsyncEscape -fblocks -verify %s typedef struct dispatch_queue_s *dispatch_queue_t; typedef void (^dispatch_block_t)(void); Index: test/Analysis/stack-capture-leak-arc.mm === --- test/Analysis/stack-capture-leak-arc.mm +++ test/Analysis/stack-capture-leak-arc.mm @@ -1,12 +1,13 @@ -// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core -fblocks -fobjc-arc -verify %s +// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,alpha.core.StackAddressAsyncEscape -fblocks -fobjc-arc -verify %s typedef struct dispatch_queue_s *dispatch_queue_t; typedef void (^dispatch_block_t)(void); void dispatch_async(dispatch_queue_t queue, dispatch_block_t block); typedef long dispatch_once_t; void dispatch_once(dispatch_once_t *predicate, dispatch_block_t block); typedef long dispatch_time_t; void dispatch_after(dispatch_time_t when, dispatch_queue_t queue, dispatch_block_t block); +void dispatch_barrier_sync(dispatch_queue_t queue, dispatch_block_t block); extern dispatch_queue_t queue; extern dispatch_once_t *predicate; @@ -173,3 +174,16 @@ // Wait for the asynchronous work to finish dispatch_semaphore_wait(semaphore, 1000); } + +void test_dispatch_barrier_sync() { + int buf[16]; + for (int n = 0; n < 16; ++n) { +int *ptr = &buf[n]; +// FIXME: Should not warn. The dispatch_barrier_sync() call ensures +// that the block does not outlive 'buf'. +dispatch_async(queue, ^{ // expected-warning{{Address of stack memory associated with local variable 'buf' is captured by an asynchronously-executed block}} + (void)ptr; +}); + } + dispatch_barrier_sync(queue, ^{}); +} Index: lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp === --- lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp +++ lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp @@ -37,6 +37,14 @@ mutable std::unique_ptr BT_capturedstackret; public: + enum CheckKind { +CK_StackAddrEscapeChecker, +CK_StackAddrAsyncEscapeChecker, +CK_NumCheckKinds + }; + + DefaultBool ChecksEnabled[CK_NumCheckKinds]; + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const; void checkEndFunction(CheckerContext &Ctx) const; @@ -225,6 +233,8 @@ void StackAddrEscapeChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { + if (!ChecksEnabled[CK_StackAddrAsyncEscapeChecker]) +return; if (!Call.isGlobalCFunction("dispatch_after") && !Call.isGlobalCFunction("dispatch_async")) return; @@ -237,6 +247,8 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const { + if (!ChecksEnabled[CK_StackAddrEscapeChecker]) +return; const Expr *RetE = RS->getRetValue(); if (!RetE) @@ -277,6 +289,9 @@ } void StackAddrEscapeChecker::checkEndFunction(CheckerContext &Ctx) const { + if (!ChecksEnabled[CK_StackAddrEscapeChecker]) +return; + ProgramStateRef State = Ctx.getState(); // Iterate over all bindings to global variables and see if it contains @@ -346,6 +361,12 @@ } } -void ento::registerStackAddrEscapeChecker(CheckerManager &Mgr) { - Mgr.registerChecker(); -} +#define REGISTER_CHECKER(name) \ + void ento::register##name(CheckerManager &Mgr) { \ +StackAddrEscapeChecker *Chk = \ +Mgr.registerChecker(); \ +Chk->ChecksEnabled[StackAddrEscapeChecker::CK_##name] = true; \ + } + +REGISTER_CHECKER(StackAddrEscapeChecker) +REGISTER_CHECKER(StackAddrAsyncEscapeChecker) Index: include/clang/StaticAnalyzer/Checkers/Checkers.td === --- include/clang/StaticAnalyzer/Checkers/Checkers.td +++ include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -188,6 +188,10 @@ HelpText<"Check for cases where the dynamic and the static type of an object are unrelated.">, DescFile<"DynamicTypeChecker.cpp">; +def StackAddrAsyncEscapeChecker : Checker<"StackAddressAsyncEscape">, + HelpTe
[PATCH] D41016: [Sema] Fix crash in unused-lambda-capture warning for VLAs
malcolm.parsons updated this revision to Diff 126394. malcolm.parsons marked an inline comment as done. malcolm.parsons added a comment. Add assert. Repository: rC Clang https://reviews.llvm.org/D41016 Files: include/clang/Sema/ScopeInfo.h lib/Sema/SemaLambda.cpp test/SemaCXX/warn-unused-lambda-capture.cpp Index: test/SemaCXX/warn-unused-lambda-capture.cpp === --- test/SemaCXX/warn-unused-lambda-capture.cpp +++ test/SemaCXX/warn-unused-lambda-capture.cpp @@ -191,3 +191,12 @@ void test_use_template() { test_templated(); // expected-note{{in instantiation of function template specialization 'test_templated' requested here}} } + +namespace pr3 { +int a; +void b() { + int c[a]; + auto vla_used = [&c] { return c[0]; }; + auto vla_unused = [&c] {}; // expected-warning{{lambda capture 'c' is not used}} +} +} // namespace pr3 Index: lib/Sema/SemaLambda.cpp === --- lib/Sema/SemaLambda.cpp +++ lib/Sema/SemaLambda.cpp @@ -1481,6 +1481,9 @@ if (CaptureHasSideEffects(From)) return; + if (From.isVLATypeCapture()) +return; + auto diag = Diag(From.getLocation(), diag::warn_unused_lambda_capture); if (From.isThisCapture()) diag << "'this'"; Index: include/clang/Sema/ScopeInfo.h === --- include/clang/Sema/ScopeInfo.h +++ include/clang/Sema/ScopeInfo.h @@ -560,6 +560,7 @@ void markUsed(bool IsODRUse) { (IsODRUse ? ODRUsed : NonODRUsed) = true; } VarDecl *getVariable() const { + assert(isVariableCapture()); return VarAndNestedAndThis.getPointer(); } Index: test/SemaCXX/warn-unused-lambda-capture.cpp === --- test/SemaCXX/warn-unused-lambda-capture.cpp +++ test/SemaCXX/warn-unused-lambda-capture.cpp @@ -191,3 +191,12 @@ void test_use_template() { test_templated(); // expected-note{{in instantiation of function template specialization 'test_templated' requested here}} } + +namespace pr3 { +int a; +void b() { + int c[a]; + auto vla_used = [&c] { return c[0]; }; + auto vla_unused = [&c] {}; // expected-warning{{lambda capture 'c' is not used}} +} +} // namespace pr3 Index: lib/Sema/SemaLambda.cpp === --- lib/Sema/SemaLambda.cpp +++ lib/Sema/SemaLambda.cpp @@ -1481,6 +1481,9 @@ if (CaptureHasSideEffects(From)) return; + if (From.isVLATypeCapture()) +return; + auto diag = Diag(From.getLocation(), diag::warn_unused_lambda_capture); if (From.isThisCapture()) diag << "'this'"; Index: include/clang/Sema/ScopeInfo.h === --- include/clang/Sema/ScopeInfo.h +++ include/clang/Sema/ScopeInfo.h @@ -560,6 +560,7 @@ void markUsed(bool IsODRUse) { (IsODRUse ? ODRUsed : NonODRUsed) = true; } VarDecl *getVariable() const { + assert(isVariableCapture()); return VarAndNestedAndThis.getPointer(); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r320396 - [Sema] Fix crash in unused-lambda-capture warning for VLAs
Author: malcolm.parsons Date: Mon Dec 11 10:00:36 2017 New Revision: 320396 URL: http://llvm.org/viewvc/llvm-project?rev=320396&view=rev Log: [Sema] Fix crash in unused-lambda-capture warning for VLAs Summary: Clang was crashing when diagnosing an unused-lambda-capture for a VLA because From.getVariable() is null for the capture of a VLA bound. Warning about the VLA bound capture is not helpful, so only warn for the VLA itself. Fixes: PR3 Reviewers: aaron.ballman, dim, rsmith Reviewed By: aaron.ballman, dim Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D41016 Modified: cfe/trunk/include/clang/Sema/ScopeInfo.h cfe/trunk/lib/Sema/SemaLambda.cpp cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp Modified: cfe/trunk/include/clang/Sema/ScopeInfo.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/ScopeInfo.h?rev=320396&r1=320395&r2=320396&view=diff == --- cfe/trunk/include/clang/Sema/ScopeInfo.h (original) +++ cfe/trunk/include/clang/Sema/ScopeInfo.h Mon Dec 11 10:00:36 2017 @@ -560,6 +560,7 @@ public: void markUsed(bool IsODRUse) { (IsODRUse ? ODRUsed : NonODRUsed) = true; } VarDecl *getVariable() const { + assert(isVariableCapture()); return VarAndNestedAndThis.getPointer(); } Modified: cfe/trunk/lib/Sema/SemaLambda.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLambda.cpp?rev=320396&r1=320395&r2=320396&view=diff == --- cfe/trunk/lib/Sema/SemaLambda.cpp (original) +++ cfe/trunk/lib/Sema/SemaLambda.cpp Mon Dec 11 10:00:36 2017 @@ -1481,6 +1481,9 @@ void Sema::DiagnoseUnusedLambdaCapture(c if (CaptureHasSideEffects(From)) return; + if (From.isVLATypeCapture()) +return; + auto diag = Diag(From.getLocation(), diag::warn_unused_lambda_capture); if (From.isThisCapture()) diag << "'this'"; Modified: cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp?rev=320396&r1=320395&r2=320396&view=diff == --- cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp (original) +++ cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp Mon Dec 11 10:00:36 2017 @@ -191,3 +191,12 @@ void test_templated() { void test_use_template() { test_templated(); // expected-note{{in instantiation of function template specialization 'test_templated' requested here}} } + +namespace pr3 { +int a; +void b() { + int c[a]; + auto vla_used = [&c] { return c[0]; }; + auto vla_unused = [&c] {}; // expected-warning{{lambda capture 'c' is not used}} +} +} // namespace pr3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41016: [Sema] Fix crash in unused-lambda-capture warning for VLAs
This revision was automatically updated to reflect the committed changes. Closed by commit rL320396: [Sema] Fix crash in unused-lambda-capture warning for VLAs (authored by malcolm.parsons). Repository: rL LLVM https://reviews.llvm.org/D41016 Files: cfe/trunk/include/clang/Sema/ScopeInfo.h cfe/trunk/lib/Sema/SemaLambda.cpp cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp Index: cfe/trunk/include/clang/Sema/ScopeInfo.h === --- cfe/trunk/include/clang/Sema/ScopeInfo.h +++ cfe/trunk/include/clang/Sema/ScopeInfo.h @@ -560,6 +560,7 @@ void markUsed(bool IsODRUse) { (IsODRUse ? ODRUsed : NonODRUsed) = true; } VarDecl *getVariable() const { + assert(isVariableCapture()); return VarAndNestedAndThis.getPointer(); } Index: cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp === --- cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp +++ cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp @@ -191,3 +191,12 @@ void test_use_template() { test_templated(); // expected-note{{in instantiation of function template specialization 'test_templated' requested here}} } + +namespace pr3 { +int a; +void b() { + int c[a]; + auto vla_used = [&c] { return c[0]; }; + auto vla_unused = [&c] {}; // expected-warning{{lambda capture 'c' is not used}} +} +} // namespace pr3 Index: cfe/trunk/lib/Sema/SemaLambda.cpp === --- cfe/trunk/lib/Sema/SemaLambda.cpp +++ cfe/trunk/lib/Sema/SemaLambda.cpp @@ -1481,6 +1481,9 @@ if (CaptureHasSideEffects(From)) return; + if (From.isVLATypeCapture()) +return; + auto diag = Diag(From.getLocation(), diag::warn_unused_lambda_capture); if (From.isThisCapture()) diag << "'this'"; Index: cfe/trunk/include/clang/Sema/ScopeInfo.h === --- cfe/trunk/include/clang/Sema/ScopeInfo.h +++ cfe/trunk/include/clang/Sema/ScopeInfo.h @@ -560,6 +560,7 @@ void markUsed(bool IsODRUse) { (IsODRUse ? ODRUsed : NonODRUsed) = true; } VarDecl *getVariable() const { + assert(isVariableCapture()); return VarAndNestedAndThis.getPointer(); } Index: cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp === --- cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp +++ cfe/trunk/test/SemaCXX/warn-unused-lambda-capture.cpp @@ -191,3 +191,12 @@ void test_use_template() { test_templated(); // expected-note{{in instantiation of function template specialization 'test_templated' requested here}} } + +namespace pr3 { +int a; +void b() { + int c[a]; + auto vla_used = [&c] { return c[0]; }; + auto vla_unused = [&c] {}; // expected-warning{{lambda capture 'c' is not used}} +} +} // namespace pr3 Index: cfe/trunk/lib/Sema/SemaLambda.cpp === --- cfe/trunk/lib/Sema/SemaLambda.cpp +++ cfe/trunk/lib/Sema/SemaLambda.cpp @@ -1481,6 +1481,9 @@ if (CaptureHasSideEffects(From)) return; + if (From.isVLATypeCapture()) +return; + auto diag = Diag(From.getLocation(), diag::warn_unused_lambda_capture); if (From.isThisCapture()) diag << "'this'"; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41016: [Sema] Fix crash in unused-lambda-capture warning for VLAs
malcolm.parsons added inline comments. Comment at: lib/Sema/SemaLambda.cpp:1491 else diag << From.getVariable(); diag << From.isNonODRUsed(); dim wrote: > Just for sanity's sake, I would still put an assert here, that > `getVariable()` does not return `nullptr`. That might catch similar issues > in the future. (And it is better than a segfault... :) ) > I'm adding an assert to `getVariable()` instead. Repository: rC Clang https://reviews.llvm.org/D41016 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41064: Suppress -Wuser-defined-literals for and
mclow.lists requested changes to this revision. mclow.lists added a comment. This revision now requires changes to proceed. In https://reviews.llvm.org/D41064#950946, @aaron.ballman wrote: > I think that it would be more appropriate to fix this in Clang rather than > libc++. For instance, we don't want libstdc++ to have to silence our same > diagnostic here. I agree. Repository: rCXX libc++ https://reviews.llvm.org/D41064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41073: Wasm: add support in libcxx
ncw created this revision. Herald added subscribers: cfe-commits, aheejin, jfb. Herald added a reviewer: EricWF. It turns out that this is the only change required in libcxx for it to compile with the new `wasm32-unknown-unknown-wasm` target recently added to Clang. I haven't done much testing of whether libc++ //works// with Wasm, but committing the compile fix is at least a start. Adding Marshall Clow and Dan Gohman as reviewers since they landed the last Emscripten-related changes in libcxx. Repository: rCXX libc++ https://reviews.llvm.org/D41073 Files: include/__config Index: include/__config === --- include/__config +++ include/__config @@ -45,6 +45,8 @@ #define _LIBCPP_OBJECT_FORMAT_MACHO 1 #elif defined(_WIN32) #define _LIBCPP_OBJECT_FORMAT_COFF 1 +#elif defined(__wasm__) +#define _LIBCPP_OBJECT_FORMAT_WASM 1 #else #error Unknown object file format #endif Index: include/__config === --- include/__config +++ include/__config @@ -45,6 +45,8 @@ #define _LIBCPP_OBJECT_FORMAT_MACHO 1 #elif defined(_WIN32) #define _LIBCPP_OBJECT_FORMAT_COFF 1 +#elif defined(__wasm__) +#define _LIBCPP_OBJECT_FORMAT_WASM 1 #else #error Unknown object file format #endif ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41064: Suppress -Wuser-defined-literals for and
dim updated this revision to Diff 126397. dim added a comment. Updated to also include and . I think all cases are covered now. Repository: rCXX libc++ https://reviews.llvm.org/D41064 Files: include/chrono include/complex include/string include/string_view Index: include/string_view === --- include/string_view +++ include/string_view @@ -792,6 +792,10 @@ { inline namespace string_view_literals { +#if defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wuser-defined-literals" +#endif inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR basic_string_view operator "" sv(const char *__str, size_t __len) _NOEXCEPT { @@ -815,6 +819,9 @@ { return basic_string_view (__str, __len); } +#if defined(__clang__) +#pragma clang diagnostic pop +#endif } } #endif Index: include/string === --- include/string +++ include/string @@ -4042,6 +4042,10 @@ { inline namespace string_literals { +#if defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wuser-defined-literals" +#endif inline _LIBCPP_INLINE_VISIBILITY basic_string operator "" s( const char *__str, size_t __len ) { @@ -4065,6 +4069,9 @@ { return basic_string (__str, __len); } +#if defined(__clang__) +#pragma clang diagnostic pop +#endif } } #endif Index: include/complex === --- include/complex +++ include/complex @@ -1444,6 +1444,10 @@ { inline namespace complex_literals { +#if defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wuser-defined-literals" +#endif constexpr complex operator""il(long double __im) { return { 0.0l, __im }; @@ -1475,6 +1479,9 @@ { return { 0.0f, static_cast(__im) }; } +#if defined(__clang__) +#pragma clang diagnostic pop +#endif } } #endif Index: include/chrono === --- include/chrono +++ include/chrono @@ -1086,6 +1086,10 @@ { inline namespace chrono_literals { +#if defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wuser-defined-literals" +#endif constexpr chrono::hours operator""h(unsigned long long __h) { @@ -1152,6 +1156,9 @@ return chrono::duration (__ns); } +#if defined(__clang__) +#pragma clang diagnostic pop +#endif }} namespace chrono { // hoist the literals into namespace std::chrono Index: include/string_view === --- include/string_view +++ include/string_view @@ -792,6 +792,10 @@ { inline namespace string_view_literals { +#if defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wuser-defined-literals" +#endif inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR basic_string_view operator "" sv(const char *__str, size_t __len) _NOEXCEPT { @@ -815,6 +819,9 @@ { return basic_string_view (__str, __len); } +#if defined(__clang__) +#pragma clang diagnostic pop +#endif } } #endif Index: include/string === --- include/string +++ include/string @@ -4042,6 +4042,10 @@ { inline namespace string_literals { +#if defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wuser-defined-literals" +#endif inline _LIBCPP_INLINE_VISIBILITY basic_string operator "" s( const char *__str, size_t __len ) { @@ -4065,6 +4069,9 @@ { return basic_string (__str, __len); } +#if defined(__clang__) +#pragma clang diagnostic pop +#endif } } #endif Index: include/complex === --- include/complex +++ include/complex @@ -1444,6 +1444,10 @@ { inline namespace complex_literals { +#if defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wuser-defined-literals" +#endif constexpr complex operator""il(long double __im) { return { 0.0l, __im }; @@ -1475,6 +1479,9 @@ { return { 0.0f, static_cast(__im) }; } +#if defined(__clang__) +#pragma clang diagnostic pop +#endif } } #endif Index: include/chrono === --- include/chrono +++ include/chrono @@ -1086,6 +1086,10 @@ { inline namespace chrono_literals { +#if defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wuser-defined-literals" +#endif constexpr chrono::hours operator""h(unsigned long long __h) { @@ -1152,6 +1156,9 @@ return chrono::duration (__ns); } +#if defined(__clang__) +#pragma clang di
[PATCH] D41074: [ClangFormat] ObjCSpaceBeforeProtocolList should be true in the google style
benhamilton created this revision. Herald added a subscriber: cfe-commits. The Google style guide is neutral on whether there should be a space before the protocol list in an Objective-C @interface or @implementation. The majority of Objective-C code in both Apple's public header files and Google's open-source uses a space before the protocol list, so this changes the google style to default ObjCSpaceBeforeProtocolList to true. Test Plan: make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests Repository: rC Clang https://reviews.llvm.org/D41074 Files: lib/Format/Format.cpp unittests/Format/FormatTestObjC.cpp Index: unittests/Format/FormatTestObjC.cpp === --- unittests/Format/FormatTestObjC.cpp +++ unittests/Format/FormatTestObjC.cpp @@ -200,7 +200,7 @@ "@end"); Style = getGoogleStyle(FormatStyle::LK_ObjC); - verifyFormat("@interface Foo : NSObject {\n" + verifyFormat("@interface Foo : NSObject {\n" " @public\n" " int field1;\n" " @protected\n" @@ -212,15 +212,15 @@ "}\n" "+ (id)init;\n" "@end"); - verifyFormat("@interface Foo : Bar\n" + verifyFormat("@interface Foo : Bar \n" "+ (id)init;\n" "@end"); - verifyFormat("@interface Foo (HackStuff)\n" + verifyFormat("@interface Foo (HackStuff) \n" "+ (id)init;\n" "@end"); Style.BinPackParameters = false; Style.ColumnLimit = 80; - verifyFormat("@interface a ()<\n" + verifyFormat("@interface a () <\n" "a,\n" ",\n" "aa,\n" @@ -343,7 +343,7 @@ "@end"); Style = getGoogleStyle(FormatStyle::LK_ObjC); - verifyFormat("@protocol MyProtocol\n" + verifyFormat("@protocol MyProtocol \n" "- (NSUInteger)numberOfThings;\n" "@end"); } Index: lib/Format/Format.cpp === --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -693,7 +693,7 @@ GoogleStyle.IndentCaseLabels = true; GoogleStyle.KeepEmptyLinesAtTheStartOfBlocks = false; GoogleStyle.ObjCSpaceAfterProperty = false; - GoogleStyle.ObjCSpaceBeforeProtocolList = false; + GoogleStyle.ObjCSpaceBeforeProtocolList = true; GoogleStyle.PointerAlignment = FormatStyle::PAS_Left; GoogleStyle.SpacesBeforeTrailingComments = 2; GoogleStyle.Standard = FormatStyle::LS_Auto; Index: unittests/Format/FormatTestObjC.cpp === --- unittests/Format/FormatTestObjC.cpp +++ unittests/Format/FormatTestObjC.cpp @@ -200,7 +200,7 @@ "@end"); Style = getGoogleStyle(FormatStyle::LK_ObjC); - verifyFormat("@interface Foo : NSObject {\n" + verifyFormat("@interface Foo : NSObject {\n" " @public\n" " int field1;\n" " @protected\n" @@ -212,15 +212,15 @@ "}\n" "+ (id)init;\n" "@end"); - verifyFormat("@interface Foo : Bar\n" + verifyFormat("@interface Foo : Bar \n" "+ (id)init;\n" "@end"); - verifyFormat("@interface Foo (HackStuff)\n" + verifyFormat("@interface Foo (HackStuff) \n" "+ (id)init;\n" "@end"); Style.BinPackParameters = false; Style.ColumnLimit = 80; - verifyFormat("@interface a ()<\n" + verifyFormat("@interface a () <\n" "a,\n" ",\n" "aa,\n" @@ -343,7 +343,7 @@ "@end"); Style = getGoogleStyle(FormatStyle::LK_ObjC); - verifyFormat("@protocol MyProtocol\n" + verifyFormat("@protocol MyProtocol \n" "- (NSUInteger)numberOfThings;\n" "@end"); } Index: lib/Format/Format.cpp === --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -693,7 +693,7 @@ GoogleStyle.IndentCaseLabels = true; GoogleStyle.KeepEmptyLinesAtTheStartOfBlocks = false; GoogleStyle.ObjCSpaceAfterProperty = false; - GoogleStyle.ObjCSpaceBeforeProtocolList = false; + GoogleStyle.ObjCSpaceBeforeProtocolList = true; GoogleStyle.PointerAlignment = FormatStyle::PAS_Left; GoogleStyle.SpacesBeforeTrailingComments = 2; GoogleStyle.Standard = FormatStyle::LS_Auto; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r320398 - Revert 320391: Certain targets are failing, pulling back to diagnose.
Author: erichkeane Date: Mon Dec 11 10:14:51 2017 New Revision: 320398 URL: http://llvm.org/viewvc/llvm-project?rev=320398&view=rev Log: Revert 320391: Certain targets are failing, pulling back to diagnose. Removed: cfe/trunk/test/Driver/Inputs/stdc-predef/ cfe/trunk/test/Driver/stdc-predef.c cfe/trunk/test/Driver/stdc-predef.i Modified: cfe/trunk/include/clang/Driver/CC1Options.td cfe/trunk/include/clang/Lex/PreprocessorOptions.h cfe/trunk/lib/Driver/Job.cpp cfe/trunk/lib/Driver/ToolChains/Linux.cpp cfe/trunk/lib/Driver/ToolChains/Linux.h cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/lib/Frontend/InitPreprocessor.cpp cfe/trunk/test/Driver/crash-report-header.h cfe/trunk/test/Driver/crash-report-spaces.c cfe/trunk/test/Driver/crash-report.c cfe/trunk/test/Driver/rewrite-map-in-diagnostics.c cfe/trunk/test/Index/IBOutletCollection.m cfe/trunk/test/Index/annotate-macro-args.m cfe/trunk/test/Index/annotate-tokens-pp.c cfe/trunk/test/Index/annotate-tokens.c cfe/trunk/test/Index/c-index-getCursor-test.m cfe/trunk/test/Index/get-cursor-macro-args.m cfe/trunk/test/Index/get-cursor.cpp cfe/trunk/test/Preprocessor/ignore-pragmas.c cfe/trunk/unittests/Tooling/TestVisitor.h cfe/trunk/unittests/libclang/LibclangTest.cpp Modified: cfe/trunk/include/clang/Driver/CC1Options.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=320398&r1=320397&r2=320398&view=diff == --- cfe/trunk/include/clang/Driver/CC1Options.td (original) +++ cfe/trunk/include/clang/Driver/CC1Options.td Mon Dec 11 10:14:51 2017 @@ -769,8 +769,6 @@ def token_cache : Separate<["-"], "token HelpText<"Use specified token cache file">; def detailed_preprocessing_record : Flag<["-"], "detailed-preprocessing-record">, HelpText<"include a detailed record of preprocessing actions">; -def fsystem_include_if_exists : Separate<["-"], "fsystem-include-if-exists">, MetaVarName<"">, - HelpText<"Include system file before parsing if file exists">; //===--===// // OpenCL Options Modified: cfe/trunk/include/clang/Lex/PreprocessorOptions.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/PreprocessorOptions.h?rev=320398&r1=320397&r2=320398&view=diff == --- cfe/trunk/include/clang/Lex/PreprocessorOptions.h (original) +++ cfe/trunk/include/clang/Lex/PreprocessorOptions.h Mon Dec 11 10:14:51 2017 @@ -60,9 +60,6 @@ public: /// \brief Headers that will be converted to chained PCHs in memory. std::vector ChainedIncludes; - /// \brief System Headers that are pre-included if they exist. - std::vector FSystemIncludeIfExists; - /// \brief When true, disables most of the normal validation performed on /// precompiled headers. bool DisablePCHValidation = false; @@ -186,7 +183,6 @@ public: DumpDeserializedPCHDecls = false; ImplicitPCHInclude.clear(); ImplicitPTHInclude.clear(); -FSystemIncludeIfExists.clear(); TokenCache.clear(); SingleFileParseMode = false; LexEditorPlaceholders = true; Modified: cfe/trunk/lib/Driver/Job.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Job.cpp?rev=320398&r1=320397&r2=320398&view=diff == --- cfe/trunk/lib/Driver/Job.cpp (original) +++ cfe/trunk/lib/Driver/Job.cpp Mon Dec 11 10:14:51 2017 @@ -64,7 +64,7 @@ static bool skipArgs(const char *Flag, b .Cases("-internal-externc-isystem", "-iprefix", true) .Cases("-iwithprefixbefore", "-isystem", "-iquote", true) .Cases("-isysroot", "-I", "-F", "-resource-dir", true) -.Cases("-iframework", "-include-pch", "-fsystem-include-if-exists", true) +.Cases("-iframework", "-include-pch", true) .Default(false); if (IsInclude) return HaveCrashVFS ? false : true; Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.cpp?rev=320398&r1=320397&r2=320398&view=diff == --- cfe/trunk/lib/Driver/ToolChains/Linux.cpp (original) +++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp Mon Dec 11 10:14:51 2017 @@ -710,8 +710,6 @@ void Linux::AddClangSystemIncludeArgs(co addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/include"); addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include"); - - AddGnuIncludeArgs(DriverArgs, CC1Args); } static std::string DetectLibcxxIncludePath(StringRef base) { @@ -750,16 +748,6 @@ std::string Linux::findLibCxxIncludePath return ""; } -void Linux::AddGnuIncludeArgs(const llvm::opt::ArgList &DriverArgs, - llvm::o
[PATCH] D40991: [libcxx] [test] Fix line endings, avoid unnecessary non-ASCII.
mclow.lists added a comment. Except for the stuff in TODO.txt these look good to me. We need to do some work on that file - it's pretty out of date. https://reviews.llvm.org/D40991 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40743: Make rehash(0) work with ubsan's unsigned-integer-overflow.
mclow.lists added a comment. Dan - I think I need a bit more context here. How does UBSan get triggered? Repository: rCXX libc++ https://reviews.llvm.org/D40743 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r320401 - P0620 follow-up: deducing `auto` from braced-init-list in new expr
Author: lichray Date: Mon Dec 11 10:29:54 2017 New Revision: 320401 URL: http://llvm.org/viewvc/llvm-project?rev=320401&view=rev Log: P0620 follow-up: deducing `auto` from braced-init-list in new expr Summary: This is a side-effect brought in by p0620r0, which allows other placeholder types (derived from `auto` and `decltype(auto)`) to be usable in a `new` expression with a single-clause //braced-init-list// as its initializer (8.3.4 [expr.new]/2). N3922 defined its semantics. References: http://wg21.link/p0620r0 http://wg21.link/n3922 Reviewers: rsmith, aaron.ballman Reviewed By: rsmith Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D39451 Added: cfe/trunk/test/CXX/expr/expr.unary/expr.new/p2-cxx14.cpp cfe/trunk/test/CXX/expr/expr.unary/expr.new/p2-cxx1z.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Sema/SemaExprCXX.cpp cfe/trunk/test/CXX/expr/expr.unary/expr.new/p2-cxx0x.cpp cfe/trunk/test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=320401&r1=320400&r2=320401&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Dec 11 10:29:54 2017 @@ -1988,8 +1988,9 @@ def err_auto_var_requires_init : Error< "declaration of variable %0 with deduced type %1 requires an initializer">; def err_auto_new_requires_ctor_arg : Error< "new expression for type %0 requires a constructor argument">; -def err_auto_new_list_init : Error< - "new expression for type %0 cannot use list-initialization">; +def ext_auto_new_list_init : Extension< + "ISO C++ standards before C++17 do not allow new expression for " + "type %0 to use list-initialization">, InGroup; def err_auto_var_init_no_expression : Error< "initializer for variable %0 with type %1 is empty">; def err_auto_var_init_multiple_expressions : Error< Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=320401&r1=320400&r2=320401&view=diff == --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original) +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Mon Dec 11 10:29:54 2017 @@ -1748,20 +1748,27 @@ Sema::BuildCXXNew(SourceRange Range, boo if (AllocType.isNull()) return ExprError(); } else if (Deduced) { +bool Braced = (initStyle == CXXNewExpr::ListInit); +if (NumInits == 1) { + if (auto p = dyn_cast_or_null(Inits[0])) { +Inits = p->getInits(); +NumInits = p->getNumInits(); +Braced = true; + } +} + if (initStyle == CXXNewExpr::NoInit || NumInits == 0) return ExprError(Diag(StartLoc, diag::err_auto_new_requires_ctor_arg) << AllocType << TypeRange); -if (initStyle == CXXNewExpr::ListInit || -(NumInits == 1 && isa(Inits[0]))) - return ExprError(Diag(Inits[0]->getLocStart(), -diag::err_auto_new_list_init) - << AllocType << TypeRange); if (NumInits > 1) { Expr *FirstBad = Inits[1]; return ExprError(Diag(FirstBad->getLocStart(), diag::err_auto_new_ctor_multiple_expressions) << AllocType << TypeRange); } +if (Braced && !getLangOpts().CPlusPlus17) + Diag(Initializer->getLocStart(), diag::ext_auto_new_list_init) + << AllocType << TypeRange; Expr *Deduce = Inits[0]; QualType DeducedType; if (DeduceAutoType(AllocTypeInfo, Deduce, DeducedType) == DAR_Failed) Modified: cfe/trunk/test/CXX/expr/expr.unary/expr.new/p2-cxx0x.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/expr/expr.unary/expr.new/p2-cxx0x.cpp?rev=320401&r1=320400&r2=320401&view=diff == --- cfe/trunk/test/CXX/expr/expr.unary/expr.new/p2-cxx0x.cpp (original) +++ cfe/trunk/test/CXX/expr/expr.unary/expr.new/p2-cxx0x.cpp Mon Dec 11 10:29:54 2017 @@ -9,12 +9,14 @@ struct only { void f() { only p = new const auto (0); only q = new (auto) (0.0); + only r = new auto {'a'}; new auto; // expected-error{{new expression for type 'auto' requires a constructor argument}} new (const auto)(); // expected-error{{new expression for type 'const auto' requires a constructor argument}} new (auto) (1,2,3); // expected-error{{new expression for type 'auto' contains multiple constructor arguments}} - new auto {1,2,3}; // expected-error{{new expression for type 'auto' cannot use list-initialization}} - new auto ({1,2,3}); // expected-error{{new expression for type 'auto' cannot use list-initialization}}
[PATCH] D39451: P0620 follow-up: deducing `auto` from braced-init-list in new expr
This revision was automatically updated to reflect the committed changes. Closed by commit rC320401: P0620 follow-up: deducing `auto` from braced-init-list in new expr (authored by lichray). Changed prior to commit: https://reviews.llvm.org/D39451?vs=126092&id=126400#toc Repository: rC Clang https://reviews.llvm.org/D39451 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExprCXX.cpp test/CXX/expr/expr.unary/expr.new/p2-cxx0x.cpp test/CXX/expr/expr.unary/expr.new/p2-cxx14.cpp test/CXX/expr/expr.unary/expr.new/p2-cxx1z.cpp test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp Index: test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp === --- test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp +++ test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp @@ -148,7 +148,7 @@ } void dangle() { - new auto{1, 2, 3}; // expected-error {{cannot use list-initialization}} + new auto{1, 2, 3}; // expected-error {{new expression for type 'auto' contains multiple constructor arguments}} new std::initializer_list{1, 2, 3}; // expected-warning {{at the end of the full-expression}} } Index: test/CXX/expr/expr.unary/expr.new/p2-cxx14.cpp === --- test/CXX/expr/expr.unary/expr.new/p2-cxx14.cpp +++ test/CXX/expr/expr.unary/expr.new/p2-cxx14.cpp @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++14 -pedantic + +void f() { + new auto('a'); + new auto {2}; // expected-warning {{ISO C++ standards before C++17 do not allow new expression for type 'auto' to use list-initialization}} + new auto {1, 2}; // expected-error{{new expression for type 'auto' contains multiple constructor arguments}} + new auto {}; // expected-error{{new expression for type 'auto' requires a constructor argument}} + new decltype(auto)({1}); // expected-warning {{ISO C++ standards before C++17 do not allow new expression for type 'decltype(auto)' to use list-initialization}} + new decltype(auto)({1, 2}); // expected-error{{new expression for type 'decltype(auto)' contains multiple constructor arguments}} +} Index: test/CXX/expr/expr.unary/expr.new/p2-cxx1z.cpp === --- test/CXX/expr/expr.unary/expr.new/p2-cxx1z.cpp +++ test/CXX/expr/expr.unary/expr.new/p2-cxx1z.cpp @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++14 +// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++17 -pedantic + +void f() { + new auto('a'); + new auto {2}; + new auto {1, 2}; // expected-error{{new expression for type 'auto' contains multiple constructor arguments}} + new auto {}; // expected-error{{new expression for type 'auto' requires a constructor argument}} + new decltype(auto)({1}); + new decltype(auto)({1, 2}); // expected-error{{new expression for type 'decltype(auto)' contains multiple constructor arguments}} +} Index: test/CXX/expr/expr.unary/expr.new/p2-cxx0x.cpp === --- test/CXX/expr/expr.unary/expr.new/p2-cxx0x.cpp +++ test/CXX/expr/expr.unary/expr.new/p2-cxx0x.cpp @@ -9,12 +9,14 @@ void f() { only p = new const auto (0); only q = new (auto) (0.0); + only r = new auto {'a'}; new auto; // expected-error{{new expression for type 'auto' requires a constructor argument}} new (const auto)(); // expected-error{{new expression for type 'const auto' requires a constructor argument}} new (auto) (1,2,3); // expected-error{{new expression for type 'auto' contains multiple constructor arguments}} - new auto {1,2,3}; // expected-error{{new expression for type 'auto' cannot use list-initialization}} - new auto ({1,2,3}); // expected-error{{new expression for type 'auto' cannot use list-initialization}} + new auto {}; // expected-error{{new expression for type 'auto' requires a constructor argument}} + new auto {1,2,3}; // expected-error{{new expression for type 'auto' contains multiple constructor arguments}} + new auto ({1,2,3}); // expected-error{{new expression for type 'auto' contains multiple constructor arguments}} } void p2example() { Index: lib/Sema/SemaExprCXX.cpp === --- lib/Sema/SemaExprCXX.cpp +++ lib/Sema/SemaExprCXX.cpp @@ -1748,20 +1748,27 @@ if (AllocType.isNull()) return ExprError(); } else if (Deduced) { +bool Braced = (initStyle == CXXNewExpr::ListInit); +if (NumInits == 1) { + if (auto p = dyn_cast_or_null(Inits[0])) { +Inits = p->getInits(); +NumInits = p->getNumInits(); +Braced = true; + } +} + if (initStyle == CXXNewExpr::NoInit || NumInits == 0) return ExprError(Diag(StartLoc, diag::err_auto_new_requires_ctor_arg) << AllocType << TypeRange); -if (initStyle == CXXNewExpr::ListInit || -(NumInits == 1 && isa(Inits[0]))) -
[PATCH] D41075: [driver][darwin] Infer the 'simuluator
arphaman created this revision. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D41075 Files: lib/Driver/ToolChains/Darwin.cpp test/Driver/darwin-version.c Index: test/Driver/darwin-version.c === --- test/Driver/darwin-version.c +++ test/Driver/darwin-version.c @@ -262,3 +262,13 @@ // RUN: %clang -target uknown-apple-macos10.11.2 -arch=armv7k -c %s -### 2>&1 | \ // RUN: FileCheck --check-prefix=CHECK-VERSION-TIGNORE-ARCH1 %s // CHECK-VERSION-TIGNORE-ARCH1: "unknown-apple-macosx10.11.2" + +// Target can be used to specify the environment: + +// RUN: %clang -target x86_64-apple-ios11-simulator -c %s -### 2>&1 | \ +// RUN: FileCheck --check-prefix=CHECK-VERSION-TENV-SIM1 %s +// CHECK-VERSION-TENV-SIM1: "x86_64-apple-ios11.0.0-simulator" + +// RUN: %clang -target armv7k-apple-ios10.1-simulator -c %s -### 2>&1 | \ +// RUN: FileCheck --check-prefix=CHECK-VERSION-TENV-SIM2 %s +// CHECK-VERSION-TENV-SIM2: "thumbv7k-apple-ios10.1.0-simulator" Index: lib/Driver/ToolChains/Darwin.cpp === --- lib/Driver/ToolChains/Darwin.cpp +++ lib/Driver/ToolChains/Darwin.cpp @@ -1181,9 +1181,12 @@ }; using DarwinPlatformKind = Darwin::DarwinPlatformKind; + using DarwinEnvironmentKind = Darwin::DarwinEnvironmentKind; DarwinPlatformKind getPlatform() const { return Platform; } + DarwinEnvironmentKind getEnvironment() const { return Environment; } + StringRef getOSVersion() const { if (Kind == OSVersionArg) return Argument->getValue(); @@ -1234,8 +1237,17 @@ } static DarwinPlatform createFromTarget(llvm::Triple::OSType OS, - StringRef OSVersion, Arg *A) { -return DarwinPlatform(TargetArg, getPlatformFromOS(OS), OSVersion, A); + StringRef OSVersion, Arg *A, + llvm::Triple::EnvironmentType Env) { +DarwinPlatform Result(TargetArg, getPlatformFromOS(OS), OSVersion, A); +switch (Env) { +case llvm::Triple::Simulator: + Result.Environment = DarwinEnvironmentKind::Simulator; + break; +default: + break; +} +return Result; } static DarwinPlatform createOSVersionArg(DarwinPlatformKind Platform, Arg *A) { @@ -1282,6 +1294,7 @@ SourceKind Kind; DarwinPlatformKind Platform; + DarwinEnvironmentKind Environment = DarwinEnvironmentKind::NativeEnvironment; std::string OSVersion; Arg *Argument; StringRef EnvVarName; @@ -1478,7 +1491,8 @@ return None; std::string OSVersion = getOSVersion(Triple.getOS(), Triple, TheDriver); return DarwinPlatform::createFromTarget(Triple.getOS(), OSVersion, - Args.getLastArg(options::OPT_target)); + Args.getLastArg(options::OPT_target), + Triple.getEnvironment()); } } // namespace @@ -1584,10 +1598,11 @@ } else llvm_unreachable("unknown kind of Darwin platform"); - DarwinEnvironmentKind Environment = NativeEnvironment; + DarwinEnvironmentKind Environment = OSTarget->getEnvironment(); // Recognize iOS targets with an x86 architecture as the iOS simulator. - if (Platform != MacOS && (getTriple().getArch() == llvm::Triple::x86 || -getTriple().getArch() == llvm::Triple::x86_64)) + if (Environment == NativeEnvironment && Platform != MacOS && + (getTriple().getArch() == llvm::Triple::x86 || + getTriple().getArch() == llvm::Triple::x86_64)) Environment = Simulator; setTarget(Platform, Environment, Major, Minor, Micro); Index: test/Driver/darwin-version.c === --- test/Driver/darwin-version.c +++ test/Driver/darwin-version.c @@ -262,3 +262,13 @@ // RUN: %clang -target uknown-apple-macos10.11.2 -arch=armv7k -c %s -### 2>&1 | \ // RUN: FileCheck --check-prefix=CHECK-VERSION-TIGNORE-ARCH1 %s // CHECK-VERSION-TIGNORE-ARCH1: "unknown-apple-macosx10.11.2" + +// Target can be used to specify the environment: + +// RUN: %clang -target x86_64-apple-ios11-simulator -c %s -### 2>&1 | \ +// RUN: FileCheck --check-prefix=CHECK-VERSION-TENV-SIM1 %s +// CHECK-VERSION-TENV-SIM1: "x86_64-apple-ios11.0.0-simulator" + +// RUN: %clang -target armv7k-apple-ios10.1-simulator -c %s -### 2>&1 | \ +// RUN: FileCheck --check-prefix=CHECK-VERSION-TENV-SIM2 %s +// CHECK-VERSION-TENV-SIM2: "thumbv7k-apple-ios10.1.0-simulator" Index: lib/Driver/ToolChains/Darwin.cpp === --- lib/Driver/ToolChains/Darwin.cpp +++ lib/Driver/ToolChains/Darwin.cpp @@ -1181,9 +1181,12 @@ }; using DarwinPlatformKind = Darwin::DarwinPlatformKind; + using DarwinEnvironmentKind = Darwin::Darw
[PATCH] D41075: [driver][darwin] Infer the 'simuluator
arphaman abandoned this revision. arphaman added a comment. Accidental 'return', will reopen Repository: rC Clang https://reviews.llvm.org/D41075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41076: [driver][darwin] Set the 'simulator' environment when it's specified in '-target'
arphaman created this revision. This patch ensures that '-target' can be used to set the 'simulator' environment component in the target triple. Repository: rC Clang https://reviews.llvm.org/D41076 Files: lib/Driver/ToolChains/Darwin.cpp test/Driver/darwin-version.c Index: test/Driver/darwin-version.c === --- test/Driver/darwin-version.c +++ test/Driver/darwin-version.c @@ -262,3 +262,13 @@ // RUN: %clang -target uknown-apple-macos10.11.2 -arch=armv7k -c %s -### 2>&1 | \ // RUN: FileCheck --check-prefix=CHECK-VERSION-TIGNORE-ARCH1 %s // CHECK-VERSION-TIGNORE-ARCH1: "unknown-apple-macosx10.11.2" + +// Target can be used to specify the environment: + +// RUN: %clang -target x86_64-apple-ios11-simulator -c %s -### 2>&1 | \ +// RUN: FileCheck --check-prefix=CHECK-VERSION-TENV-SIM1 %s +// CHECK-VERSION-TENV-SIM1: "x86_64-apple-ios11.0.0-simulator" + +// RUN: %clang -target armv7k-apple-ios10.1-simulator -c %s -### 2>&1 | \ +// RUN: FileCheck --check-prefix=CHECK-VERSION-TENV-SIM2 %s +// CHECK-VERSION-TENV-SIM2: "thumbv7k-apple-ios10.1.0-simulator" Index: lib/Driver/ToolChains/Darwin.cpp === --- lib/Driver/ToolChains/Darwin.cpp +++ lib/Driver/ToolChains/Darwin.cpp @@ -1181,9 +1181,12 @@ }; using DarwinPlatformKind = Darwin::DarwinPlatformKind; + using DarwinEnvironmentKind = Darwin::DarwinEnvironmentKind; DarwinPlatformKind getPlatform() const { return Platform; } + DarwinEnvironmentKind getEnvironment() const { return Environment; } + StringRef getOSVersion() const { if (Kind == OSVersionArg) return Argument->getValue(); @@ -1234,8 +1237,17 @@ } static DarwinPlatform createFromTarget(llvm::Triple::OSType OS, - StringRef OSVersion, Arg *A) { -return DarwinPlatform(TargetArg, getPlatformFromOS(OS), OSVersion, A); + StringRef OSVersion, Arg *A, + llvm::Triple::EnvironmentType Env) { +DarwinPlatform Result(TargetArg, getPlatformFromOS(OS), OSVersion, A); +switch (Env) { +case llvm::Triple::Simulator: + Result.Environment = DarwinEnvironmentKind::Simulator; + break; +default: + break; +} +return Result; } static DarwinPlatform createOSVersionArg(DarwinPlatformKind Platform, Arg *A) { @@ -1282,6 +1294,7 @@ SourceKind Kind; DarwinPlatformKind Platform; + DarwinEnvironmentKind Environment = DarwinEnvironmentKind::NativeEnvironment; std::string OSVersion; Arg *Argument; StringRef EnvVarName; @@ -1478,7 +1491,8 @@ return None; std::string OSVersion = getOSVersion(Triple.getOS(), Triple, TheDriver); return DarwinPlatform::createFromTarget(Triple.getOS(), OSVersion, - Args.getLastArg(options::OPT_target)); + Args.getLastArg(options::OPT_target), + Triple.getEnvironment()); } } // namespace @@ -1584,10 +1598,11 @@ } else llvm_unreachable("unknown kind of Darwin platform"); - DarwinEnvironmentKind Environment = NativeEnvironment; + DarwinEnvironmentKind Environment = OSTarget->getEnvironment(); // Recognize iOS targets with an x86 architecture as the iOS simulator. - if (Platform != MacOS && (getTriple().getArch() == llvm::Triple::x86 || -getTriple().getArch() == llvm::Triple::x86_64)) + if (Environment == NativeEnvironment && Platform != MacOS && + (getTriple().getArch() == llvm::Triple::x86 || + getTriple().getArch() == llvm::Triple::x86_64)) Environment = Simulator; setTarget(Platform, Environment, Major, Minor, Micro); Index: test/Driver/darwin-version.c === --- test/Driver/darwin-version.c +++ test/Driver/darwin-version.c @@ -262,3 +262,13 @@ // RUN: %clang -target uknown-apple-macos10.11.2 -arch=armv7k -c %s -### 2>&1 | \ // RUN: FileCheck --check-prefix=CHECK-VERSION-TIGNORE-ARCH1 %s // CHECK-VERSION-TIGNORE-ARCH1: "unknown-apple-macosx10.11.2" + +// Target can be used to specify the environment: + +// RUN: %clang -target x86_64-apple-ios11-simulator -c %s -### 2>&1 | \ +// RUN: FileCheck --check-prefix=CHECK-VERSION-TENV-SIM1 %s +// CHECK-VERSION-TENV-SIM1: "x86_64-apple-ios11.0.0-simulator" + +// RUN: %clang -target armv7k-apple-ios10.1-simulator -c %s -### 2>&1 | \ +// RUN: FileCheck --check-prefix=CHECK-VERSION-TENV-SIM2 %s +// CHECK-VERSION-TENV-SIM2: "thumbv7k-apple-ios10.1.0-simulator" Index: lib/Driver/ToolChains/Darwin.cpp === --- lib/Driver/ToolChains/Darwin.cpp +++ lib/Driver/ToolChains/Darwin.cpp @@ -1181,9 +1181,12 @@ }; using DarwinPlatformKind
[PATCH] D40707: [libcxx] Fix basic_stringbuf constructor
mclow.lists added a comment. This looks good to me; with a couple of nits. Comment at: test/std/input.output/string.streams/stringbuf/stringbuf.cons/default.pass.cpp:26 +{ +assert(this->eback() == 0); +assert(this->gptr() == 0); Not zero, please. `nullptr` or `NULL` . `nullptr` would be better, but we can't assume c++11 here, so I guess it's `NULL`. https://reviews.llvm.org/D40707 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41077: [analyser] different.CallArgsOrder checker implementation
alexey.knyshev created this revision. alexey.knyshev added a project: clang. Herald added a subscriber: mgorny. CallArgsOrderChecker which looks for accidental swap or skip of arguments in function, methods, constructors and operators calls Repository: rC Clang https://reviews.llvm.org/D41077 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/CallArgsOrderChecker.cpp test/Analysis/call-args-order.c test/Analysis/call-args-order.cpp Index: test/Analysis/call-args-order.cpp === --- test/Analysis/call-args-order.cpp +++ test/Analysis/call-args-order.cpp @@ -0,0 +1,103 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core.CallArgsOrder -verify -x c++ %s + +typedef unsigned char byte; + +class Color { + byte r, g, b, a; + +public: + Color(byte R, byte G, byte B, byte A = 0) + : r(R), g(G), b(B), a(A) {} + + void set(byte R, byte G, byte B, byte A = 0) { +r = R; +g = G; +b = B; +a = A; + } + + void operator()(byte R, byte G, byte B, byte A = 0); + void operator+=(byte Delta); + + byte getR() const { return r; } + byte getG() const { return g; } + byte getB() const { return b; } + byte getA() const { return a; } + + static Color copy1(const Color &Color); + static Color copy2(const Color &Color); + static Color copy3(const Color &Color); + static Color copy4(const Color &Color); + static Color copy5(const Color &Color); + static Color copy6(const Color &Color); +}; + +void Color::operator()(byte R, byte G, byte B, byte A) { + set(R, B, G, A); // expected-warning {{possibly wrong order of args 'B' and 'G' passed as params 'G' and 'B'}} +} + +void Color::operator+=(byte Delta) { + r += Delta; + g += Delta; + b += Delta; + a += Delta; +} + +Color newRed1(byte r, byte b, byte a) { + return Color(r, b, a); // expected-warning {{possibly missing arg for param 'G'}} +} + +Color newRed2(byte r, byte g, byte a) { + return Color(r, g, a); // expected-warning {{possibly missing arg for param 'B'}} +} + +Color Color::copy1(const Color &o) { + return Color(o.r, o.b, o.g, o.a); // expected-warning {{possibly wrong order of args 'o.b' and 'o.g' passed as params 'G' and 'B'}} +} + +Color Color::copy2(const Color &o) { + return Color(o.r, o.g, o.a, o.b); // expected-warning {{possibly wrong order of args 'o.a' and 'o.b' passed as params 'B' and 'A'}} +} + +Color Color::copy3(const Color &o) { + return Color(o.r, o.g, o.g, o.g); // no-warning +} + +Color Color::copy4(const Color &o) { + return Color(o.r, o.r, o.r, o.r); // no-warning +} + +Color Color::copy5(const Color &o) { + return Color(o.getR(), o.getB(), o.getG(), o.getA()); // expected-warning {{possibly wrong order of args 'o.getB()' and 'o.getG()' passed as params 'G' and 'B'}} +} + +Color Color::copy6(const Color &o) { + // totaly messed up (not sure if it's wrong or not) + return Color(o.a, o.r, o.g, o.b); // no-warning +} + +Color newMagicColor() { + const byte r = 200; + const byte g = 150; + const byte b = 100; + const byte a = 0; + + return Color(r, b, g, a); // expected-warning {{possibly wrong order of args 'b' and 'g' passed as params 'G' and 'B'}} +} + +void modify(Color &c) { + int red = 1; + int green = 2; + int blue = 3; + c.set(red, green, blue); // no-warning + c.set(red, blue, green); // expected-warning {{possibly wrong order of args 'blue' and 'green' passed as params 'G' and 'B'}} + c.set(red, green, 0);// no-warning + c.set(red, green, c.getA()); // no-warning + + c(red, green, blue); // no-warning + c(green, red, blue); // expected-warning {{possibly wrong order of args 'green' and 'red' passed as params 'R' and 'G'}} + + c += 1; // no-warning + + c(red, blue, green, 0); // expected-warning {{possibly wrong order of args 'blue' and 'green' passed as params 'G' and 'B'}} +} Index: test/Analysis/call-args-order.c === --- test/Analysis/call-args-order.c +++ test/Analysis/call-args-order.c @@ -0,0 +1,57 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core.CallArgsOrder -verify -std=c99 %s + +struct Viewport { + int width; + int height; + + float aspectRatio; +}; + +void setDimentions(struct Viewport *vp, int width, int height) { + vp->width = width; + vp->height = height; + vp->aspectRatio = width / height; +} + +float getWidth(struct Viewport *vp) { return vp->width; } +float getHeight(struct Viewport *vp) { return vp->height; } + +float getFloat() { + return 128.5; +} + +int main() { + struct Viewport vp; + + int newWidth = 800; + int newHeight = 600; + setDimentions(&vp, newHeight, newWidth);// expected-warning {{possibly wrong order of args 'newHeight' and 'newWidth' passed as params 'width' and 'height'}} + setDimentions(&vp, newWidth, newHeight);// no-warning + setDimentions(&vp, newWidth, newWidth);
[PATCH] D39963: [RISCV] Add initial RISC-V target and driver support
asb updated this revision to Diff 126404. asb marked 3 inline comments as done. asb retitled this revision from "[RISCV][RFC] Add initial RISC-V target and driver support" to "[RISCV] Add initial RISC-V target and driver support". asb edited the summary of this revision. asb added a comment. Herald added a subscriber: sabuasal. Patch updates: - Address review comments - (u)int64_type is `long int` on RV64 rather than `long long int` - Remove driver for bare metal RV32 target (to be posted in a subsequent patch and maybe discussed on cfe-dev). >From my perspective, the only deficiency in this patch is now the tests (which >I'll add to this review ASAP). Let me know if you see other remaining problems. https://reviews.llvm.org/D39963 Files: lib/Basic/CMakeLists.txt lib/Basic/Targets.cpp lib/Basic/Targets/RISCV.cpp lib/Basic/Targets/RISCV.h lib/Driver/CMakeLists.txt lib/Driver/ToolChains/Arch/RISCV.cpp lib/Driver/ToolChains/Arch/RISCV.h lib/Driver/ToolChains/Clang.cpp lib/Driver/ToolChains/Clang.h lib/Driver/ToolChains/Gnu.cpp lib/Driver/ToolChains/Linux.cpp test/Driver/frame-pointer.c test/Driver/riscv-abi.c test/Driver/riscv-features.c test/Driver/riscv32-toolchain.c test/Driver/riscv64-toolchain.c test/Preprocessor/init.c Index: test/Preprocessor/init.c === --- test/Preprocessor/init.c +++ test/Preprocessor/init.c @@ -10003,3 +10003,398 @@ // ARM-DARWIN-BAREMETAL-64: #define __PTRDIFF_TYPE__ long int // ARM-DARWIN-BAREMETAL-64: #define __SIZE_TYPE__ long unsigned int +// RUN: %clang_cc1 -E -dM -ffreestanding -triple=riscv32 < /dev/null \ +// RUN: | FileCheck -match-full-lines -check-prefix=RISCV32 %s +// RISCV32: #define _ILP32 1 +// RISCV32: #define __ATOMIC_ACQUIRE 2 +// RISCV32: #define __ATOMIC_ACQ_REL 4 +// RISCV32: #define __ATOMIC_CONSUME 1 +// RISCV32: #define __ATOMIC_RELAXED 0 +// RISCV32: #define __ATOMIC_RELEASE 3 +// RISCV32: #define __ATOMIC_SEQ_CST 5 +// RISCV32: #define __BIGGEST_ALIGNMENT__ 16 +// RISCV32: #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ +// RISCV32: #define __CHAR16_TYPE__ unsigned short +// RISCV32: #define __CHAR32_TYPE__ unsigned int +// RISCV32: #define __CHAR_BIT__ 8 +// RISCV32: #define __DBL_DECIMAL_DIG__ 17 +// RISCV32: #define __DBL_DENORM_MIN__ 4.9406564584124654e-324 +// RISCV32: #define __DBL_DIG__ 15 +// RISCV32: #define __DBL_EPSILON__ 2.2204460492503131e-16 +// RISCV32: #define __DBL_HAS_DENORM__ 1 +// RISCV32: #define __DBL_HAS_INFINITY__ 1 +// RISCV32: #define __DBL_HAS_QUIET_NAN__ 1 +// RISCV32: #define __DBL_MANT_DIG__ 53 +// RISCV32: #define __DBL_MAX_10_EXP__ 308 +// RISCV32: #define __DBL_MAX_EXP__ 1024 +// RISCV32: #define __DBL_MAX__ 1.7976931348623157e+308 +// RISCV32: #define __DBL_MIN_10_EXP__ (-307) +// RISCV32: #define __DBL_MIN_EXP__ (-1021) +// RISCV32: #define __DBL_MIN__ 2.2250738585072014e-308 +// RISCV32: #define __DECIMAL_DIG__ __LDBL_DECIMAL_DIG__ +// RISCV32: #define __ELF__ 1 +// RISCV32: #define __FINITE_MATH_ONLY__ 0 +// RISCV32: #define __FLT_DECIMAL_DIG__ 9 +// RISCV32: #define __FLT_DENORM_MIN__ 1.40129846e-45F +// RISCV32: #define __FLT_DIG__ 6 +// RISCV32: #define __FLT_EPSILON__ 1.19209290e-7F +// RISCV32: #define __FLT_EVAL_METHOD__ 0 +// RISCV32: #define __FLT_HAS_DENORM__ 1 +// RISCV32: #define __FLT_HAS_INFINITY__ 1 +// RISCV32: #define __FLT_HAS_QUIET_NAN__ 1 +// RISCV32: #define __FLT_MANT_DIG__ 24 +// RISCV32: #define __FLT_MAX_10_EXP__ 38 +// RISCV32: #define __FLT_MAX_EXP__ 128 +// RISCV32: #define __FLT_MAX__ 3.40282347e+38F +// RISCV32: #define __FLT_MIN_10_EXP__ (-37) +// RISCV32: #define __FLT_MIN_EXP__ (-125) +// RISCV32: #define __FLT_MIN__ 1.17549435e-38F +// RISCV32: #define __FLT_RADIX__ 2 +// RISCV32: #define __GCC_ATOMIC_BOOL_LOCK_FREE 1 +// RISCV32: #define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 1 +// RISCV32: #define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 1 +// RISCV32: #define __GCC_ATOMIC_CHAR_LOCK_FREE 1 +// RISCV32: #define __GCC_ATOMIC_INT_LOCK_FREE 1 +// RISCV32: #define __GCC_ATOMIC_LLONG_LOCK_FREE 1 +// RISCV32: #define __GCC_ATOMIC_LONG_LOCK_FREE 1 +// RISCV32: #define __GCC_ATOMIC_POINTER_LOCK_FREE 1 +// RISCV32: #define __GCC_ATOMIC_SHORT_LOCK_FREE 1 +// RISCV32: #define __GCC_ATOMIC_TEST_AND_SET_TRUEVAL 1 +// RISCV32: #define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 1 +// RISCV32: #define __GNUC_MINOR__ {{.*}} +// RISCV32: #define __GNUC_PATCHLEVEL__ {{.*}} +// RISCV32: #define __GNUC_STDC_INLINE__ 1 +// RISCV32: #define __GNUC__ {{.*}} +// RISCV32: #define __GXX_ABI_VERSION {{.*}} +// RISCV32: #define __ILP32__ 1 +// RISCV32: #define __INT16_C_SUFFIX__ +// RISCV32: #define __INT16_MAX__ 32767 +// RISCV32: #define __INT16_TYPE__ short +// RISCV32: #define __INT32_C_SUFFIX__ +// RISCV32: #define __INT32_MAX__ 2147483647 +// RISCV32: #define __INT32_TYPE__ int +// RISCV32: #define __INT64_C_SUFFIX__ LL +// RISCV32: #define __INT64_MAX__ 9223372036854775807LL +// RISCV32: #define __INT64_TYPE__ long long int
r320405 - Fix warn-enum-compare.cpp on Windows
Author: hans Date: Mon Dec 11 10:58:18 2017 New Revision: 320405 URL: http://llvm.org/viewvc/llvm-project?rev=320405&view=rev Log: Fix warn-enum-compare.cpp on Windows It's been failing since r319875. Modified: cfe/trunk/test/SemaCXX/warn-enum-compare.cpp Modified: cfe/trunk/test/SemaCXX/warn-enum-compare.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-enum-compare.cpp?rev=320405&r1=320404&r2=320405&view=diff == --- cfe/trunk/test/SemaCXX/warn-enum-compare.cpp (original) +++ cfe/trunk/test/SemaCXX/warn-enum-compare.cpp Mon Dec 11 10:58:18 2017 @@ -1,4 +1,5 @@ -// RUN: %clang_cc1 %s -fsyntax-only -verify +// RUN: %clang_cc1 %s -fsyntax-only -verify -triple %itanium_abi_triple +// RUN: %clang_cc1 %s -fsyntax-only -verify -triple %ms_abi_triple -DMSABI enum Foo { FooA, FooB, FooC }; enum Bar { BarD, BarE, BarF }; @@ -42,8 +43,10 @@ void test () { while (b == c); while (B1 == name1::B2); while (B2 == name2::B1); +#ifndef MSABI while (x == AnonAA); // expected-warning {{comparison of constant 'AnonAA' (42) with expression of type 'Foo' is always false}} while (AnonBB == y); // expected-warning {{comparison of constant 'AnonBB' (45) with expression of type 'Bar' is always false}} +#endif while (AnonAA == AnonAB); while (AnonAB == AnonBA); while (AnonBB == AnonAA); @@ -69,7 +72,9 @@ void test () { while (B2 == ((name2::B1))); while (td == Anon1); +#ifndef MSABI while (td == AnonAA); // expected-warning {{comparison of constant 'AnonAA' (42) with expression of type 'TD' is always false}} +#endif while (B1 == B2); // expected-warning {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}} while (name1::B2 == name2::B3); // expected-warning {{comparison of two values with different enumeration types ('name1::Baz' and 'name2::Baz')}} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r319875 - Fix a bunch of wrong "tautological unsigned enum compare" diagnostics in C++.
I've committed a fix to pacify the test in r320405. On Wed, Dec 6, 2017 at 12:27 PM, Galina Kistanova via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Hello Richard, > > This commit broke the tests on the builder: > > http://lab.llvm.org:8011/builders/llvm-clang-x86_64- > expensive-checks-win/builds/6598 > > . . . > Failing Tests (1): > Clang :: SemaCXX/warn-enum-compare.cpp > > Please have a look? > > Thanks > > Galina > > On Tue, Dec 5, 2017 at 7:00 PM, Richard Smith via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: rsmith >> Date: Tue Dec 5 19:00:51 2017 >> New Revision: 319875 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=319875&view=rev >> Log: >> Fix a bunch of wrong "tautological unsigned enum compare" diagnostics in >> C++. >> >> An enumeration with a fixed underlying type can have any value in its >> underlying type, not just those spanned by the values of its enumerators. >> >> Modified: >> cfe/trunk/lib/Sema/SemaChecking.cpp >> cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.cpp >> >> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaC >> hecking.cpp?rev=319875&r1=319874&r2=319875&view=diff >> >> == >> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Dec 5 19:00:51 2017 >> @@ -8260,11 +8260,12 @@ struct IntRange { >> } else if (const EnumType *ET = dyn_cast(T)) { >>// For enum types in C++, use the known bit width of the >> enumerators. >>EnumDecl *Enum = ET->getDecl(); >> - // In C++11, enums without definitions can have an explicitly >> specified >> - // underlying type. Use this type to compute the range. >> - if (!Enum->isCompleteDefinition()) >> + // In C++11, enums can have a fixed underlying type. Use this type >> to >> + // compute the range. >> + if (Enum->isFixed()) { >> return IntRange(C.getIntWidth(QualType(T, 0)), >> !ET->isSignedIntegerOrEnumerationType()); >> + } >> >>unsigned NumPositive = Enum->getNumPositiveBits(); >>unsigned NumNegative = Enum->getNumNegativeBits(); >> >> Modified: cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/taut >> ological-unsigned-enum-zero-compare.cpp?rev=319875&r1= >> 319874&r2=319875&view=diff >> >> == >> --- cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.cpp >> (original) >> +++ cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.cpp Tue >> Dec 5 19:00:51 2017 >> @@ -2,11 +2,11 @@ >> // RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only >> -DSIGNED -verify %s >> // RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only >> -DSILENCE -Wno-tautological-unsigned-enum-zero-compare -verify %s >> >> -// Okay, this is where it gets complicated. >> -// Then default enum sigdness is target-specific. >> -// On windows, it is signed by default. We do not want to warn in that >> case. >> - >> int main() { >> + // On Windows, all enumerations have a fixed underlying type, which is >> 'int' >> + // if not otherwise specified, so A is identical to C on Windows. >> Otherwise, >> + // we follow the C++ rules, which say that the only valid values of A >> are 0 >> + // and 1. >>enum A { A_foo = 0, A_bar, }; >>enum A a; >> >> @@ -87,21 +87,23 @@ int main() { >> >>if (c < 0) >> return 0; >> - if (0 >= c) // expected-warning {{comparison 0 >= 'enum C' is always >> true}} >> + if (0 >= c) >> return 0; >> - if (c > 0) // expected-warning {{comparison 'enum C' > 0 is always >> false}} >> + if (c > 0) >> return 0; >>if (0 <= c) >> return 0; >> - if (c <= 0) // expected-warning {{comparison 'enum C' <= 0 is always >> true}} >> + if (c <= 0) >> return 0; >>if (0 > c) >> return 0; >>if (c >= 0) >> return 0; >> - if (0 < c) // expected-warning {{0 < 'enum C' is always false}} >> + if (0 < c) >> return 0; >> >> + // FIXME: These diagnostics are terrible. The issue here is that the >> signed >> + // enumeration value was promoted to an unsigned type. >>if (c < 0U) // expected-warning {{comparison of unsigned enum >> expression < 0 is always false}} >> return 0; >>if (0U >= c) >> @@ -121,21 +123,23 @@ int main() { >> #elif defined(SIGNED) >>if (a < 0) >> return 0; >> - if (0 >= a) // expected-warning {{comparison 0 >= 'enum A' is always >> true}} >> + if (0 >= a) >> return 0; >> - if (a > 0) // expected-warning {{comparison 'enum A' > 0 is always >> false}} >> + if (a > 0) >> return 0; >>if (0 <= a) >> return 0; >> - if (a <= 0) // expected-warning {{comparison 'enum A' <= 0 is always >> true}} >> + if (a <= 0)
[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked
CaseyCarter updated this revision to Diff 126406. CaseyCarter added a comment. Make the workaround unconditional. https://reviews.llvm.org/D41048 Files: include/tuple Index: include/tuple === --- include/tuple +++ include/tuple @@ -1012,10 +1012,15 @@ template struct __find_exactly_one_checked { - static constexpr bool __matches[] = {is_same<_T1, _Args>::value...}; -static constexpr size_t value = __find_detail::__find_idx(0, __matches); -static_assert (value != __not_found, "type not found in type list" ); -static_assert(value != __ambiguous,"type occurs more than once in type list"); +inline _LIBCPP_INLINE_VISIBILITY +static constexpr size_t __index() +{ +constexpr bool __matches[] = {is_same<_T1, _Args>::value...}; +return __find_detail::__find_idx(0, __matches); +} +static constexpr size_t value = __index(); +static_assert(value != __not_found, "type not found in type list" ); +static_assert(value != __ambiguous, "type occurs more than once in type list"); }; template Index: include/tuple === --- include/tuple +++ include/tuple @@ -1012,10 +1012,15 @@ template struct __find_exactly_one_checked { - static constexpr bool __matches[] = {is_same<_T1, _Args>::value...}; -static constexpr size_t value = __find_detail::__find_idx(0, __matches); -static_assert (value != __not_found, "type not found in type list" ); -static_assert(value != __ambiguous,"type occurs more than once in type list"); +inline _LIBCPP_INLINE_VISIBILITY +static constexpr size_t __index() +{ +constexpr bool __matches[] = {is_same<_T1, _Args>::value...}; +return __find_detail::__find_idx(0, __matches); +} +static constexpr size_t value = __index(); +static_assert(value != __not_found, "type not found in type list" ); +static_assert(value != __ambiguous, "type occurs more than once in type list"); }; template ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r320406 - [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming
Author: alexfh Date: Mon Dec 11 11:02:26 2017 New Revision: 320406 URL: http://llvm.org/viewvc/llvm-project?rev=320406&view=rev Log: [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming Summary: They are not locally const qualified so they weren't classified as constants by the readability-identifier-naming check. Reviewers: alexfh Reviewed By: alexfh Subscribers: klimek, cfe-commits, xazax.hun Patch by Beren Minor! Differential Revision: https://reviews.llvm.org/D39363 Modified: clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp Modified: clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp?rev=320406&r1=320405&r2=320406&view=diff == --- clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp Mon Dec 11 11:02:26 2017 @@ -449,13 +449,13 @@ static StyleKind findStyleKind( if (const auto *Decl = dyn_cast(D)) { QualType Type = Decl->getType(); -if (!Type.isNull() && Type.isLocalConstQualified() && -NamingStyles[SK_ConstantMember]) - return SK_ConstantMember; - -if (!Type.isNull() && Type.isLocalConstQualified() && -NamingStyles[SK_Constant]) - return SK_Constant; +if (!Type.isNull() && Type.isConstQualified()) { + if (NamingStyles[SK_ConstantMember]) +return SK_ConstantMember; + + if (NamingStyles[SK_Constant]) +return SK_Constant; +} if (Decl->getAccess() == AS_private && NamingStyles[SK_PrivateMember]) return SK_PrivateMember; @@ -478,13 +478,13 @@ static StyleKind findStyleKind( if (Decl->isConstexpr() && NamingStyles[SK_ConstexprVariable]) return SK_ConstexprVariable; -if (!Type.isNull() && Type.isLocalConstQualified() && -NamingStyles[SK_ConstantParameter]) - return SK_ConstantParameter; - -if (!Type.isNull() && Type.isLocalConstQualified() && -NamingStyles[SK_Constant]) - return SK_Constant; +if (!Type.isNull() && Type.isConstQualified()) { + if (NamingStyles[SK_ConstantParameter]) +return SK_ConstantParameter; + + if (NamingStyles[SK_Constant]) +return SK_Constant; +} if (Decl->isParameterPack() && NamingStyles[SK_ParameterPack]) return SK_ParameterPack; @@ -501,29 +501,25 @@ static StyleKind findStyleKind( if (Decl->isConstexpr() && NamingStyles[SK_ConstexprVariable]) return SK_ConstexprVariable; -if (!Type.isNull() && Type.isLocalConstQualified() && -Decl->isStaticDataMember() && NamingStyles[SK_ClassConstant]) - return SK_ClassConstant; - -if (!Type.isNull() && Type.isLocalConstQualified() && -Decl->isFileVarDecl() && NamingStyles[SK_GlobalConstant]) - return SK_GlobalConstant; - -if (!Type.isNull() && Type.isLocalConstQualified() && -Decl->isStaticLocal() && NamingStyles[SK_StaticConstant]) - return SK_StaticConstant; - -if (!Type.isNull() && Type.isLocalConstQualified() && -Decl->isLocalVarDecl() && NamingStyles[SK_LocalConstant]) - return SK_LocalConstant; - -if (!Type.isNull() && Type.isLocalConstQualified() && -Decl->isFunctionOrMethodVarDecl() && NamingStyles[SK_LocalConstant]) - return SK_LocalConstant; - -if (!Type.isNull() && Type.isLocalConstQualified() && -NamingStyles[SK_Constant]) - return SK_Constant; +if (!Type.isNull() && Type.isConstQualified()) { + if (Decl->isStaticDataMember() && NamingStyles[SK_ClassConstant]) +return SK_ClassConstant; + + if (Decl->isFileVarDecl() && NamingStyles[SK_GlobalConstant]) +return SK_GlobalConstant; + + if (Decl->isStaticLocal() && NamingStyles[SK_StaticConstant]) +return SK_StaticConstant; + + if (Decl->isLocalVarDecl() && NamingStyles[SK_LocalConstant]) +return SK_LocalConstant; + + if (Decl->isFunctionOrMethodVarDecl() && NamingStyles[SK_LocalConstant]) +return SK_LocalConstant; + + if (NamingStyles[SK_Constant]) +return SK_Constant; +} if (Decl->isStaticDataMember() && NamingStyles[SK_ClassMember]) return SK_ClassMember; @@ -681,8 +677,9 @@ static void addUsage(IdentifierNamingChe static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures, const NamedDecl *Decl, SourceRange Range, SourceManager *SourceMgr = nullptr) { - return addUsage(Failures, IdentifierNamingCheck::NamingCheckId( -Decl->getLocation(), Decl->getNameAsString()), + return addUsage(Failures, +
[PATCH] D39363: [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming
This revision was automatically updated to reflect the committed changes. Closed by commit rL320406: [clang-tidy] Correctly classify constant arrays and constant strings as… (authored by alexfh). Repository: rL LLVM https://reviews.llvm.org/D39363 Files: clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp Index: clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp === --- clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp +++ clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp @@ -405,6 +405,38 @@ using ::FOO_NS::InlineNamespace::CE_function; // CHECK-FIXES: {{^}} using ::foo_ns::inline_namespace::ce_function;{{$}} + + unsigned MY_LOCAL_array[] = {1,2,3}; +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: invalid case style for local variable 'MY_LOCAL_array' +// CHECK-FIXES: {{^}} unsigned my_local_array[] = {1,2,3};{{$}} + + unsigned const MyConstLocal_array[] = {1,2,3}; +// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for local constant 'MyConstLocal_array' +// CHECK-FIXES: {{^}} unsigned const kMyConstLocalArray[] = {1,2,3};{{$}} + + static unsigned MY_STATIC_array[] = {1,2,3}; +// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: invalid case style for static variable 'MY_STATIC_array' +// CHECK-FIXES: {{^}} static unsigned s_myStaticArray[] = {1,2,3};{{$}} + + static unsigned const MyConstStatic_array[] = {1,2,3}; +// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: invalid case style for static constant 'MyConstStatic_array' +// CHECK-FIXES: {{^}} static unsigned const MY_CONST_STATIC_ARRAY[] = {1,2,3};{{$}} + + char MY_LOCAL_string[] = "123"; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'MY_LOCAL_string' +// CHECK-FIXES: {{^}} char my_local_string[] = "123";{{$}} + + char const MyConstLocal_string[] = "123"; +// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for local constant 'MyConstLocal_string' +// CHECK-FIXES: {{^}} char const kMyConstLocalString[] = "123";{{$}} + + static char MY_STATIC_string[] = "123"; +// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for static variable 'MY_STATIC_string' +// CHECK-FIXES: {{^}} static char s_myStaticString[] = "123";{{$}} + + static char const MyConstStatic_string[] = "123"; +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: invalid case style for static constant 'MyConstStatic_string' +// CHECK-FIXES: {{^}} static char const MY_CONST_STATIC_STRING[] = "123";{{$}} } #define MY_TEST_Macro(X) X() @@ -418,6 +450,27 @@ template struct a { typename t_t::template b<> c; + + char const MY_ConstMember_string[4] = "123"; +// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for constant member 'MY_ConstMember_string' +// CHECK-FIXES: {{^}} char const my_const_member_string[4] = "123";{{$}} + + static char const MyConstClass_string[]; +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: invalid case style for class constant 'MyConstClass_string' +// CHECK-FIXES: {{^}} static char const kMyConstClassString[];{{$}} }; +template +char const a::MyConstClass_string[] = "123"; +// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for class constant 'MyConstClass_string' +// CHECK-FIXES: {{^}}char const a::kMyConstClassString[] = "123";{{$}} + template class A> struct b { A c; }; + +unsigned MY_GLOBAL_array[] = {1,2,3}; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global variable 'MY_GLOBAL_array' +// CHECK-FIXES: {{^}}unsigned g_my_global_array[] = {1,2,3};{{$}} + +unsigned const MyConstGlobal_array[] = {1,2,3}; +// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global constant 'MyConstGlobal_array' +// CHECK-FIXES: {{^}}unsigned const MY_CONST_GLOBAL_ARRAY[] = {1,2,3};{{$}} Index: clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp === --- clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp @@ -449,13 +449,13 @@ if (const auto *Decl = dyn_cast(D)) { QualType Type = Decl->getType(); -if (!Type.isNull() && Type.isLocalConstQualified() && -NamingStyles[SK_ConstantMember]) - return SK_ConstantMember; +if (!Type.isNull() && Type.isConstQualified()) { + if (NamingStyles[SK_ConstantMember]) +return SK_ConstantMember; -if (!Type.isNull() && Type.isLocalConstQualified() && -NamingStyles[SK_Constant]) - return SK_Constant; + if (NamingStyles[SK_Constant]) +return SK_Constant; +} if (Decl->getAccess() == AS_private && NamingStyles[SK_PrivateMember]) return SK_PrivateMember; @@ -478,13 +478,13 @@ if (Decl->isConstexpr()
[PATCH] D41080: Don't trigger -Wuser-defined-literals for system headers
dim created this revision. In https://reviews.llvm.org/D41064, I proposed adding `#pragma clang diagnostic ignored "-Wuser-defined-literals"` to some of libc++'s headers, since these warnings are now triggered by clang's new `-std=gnu++14` default: $ cat test.cpp #include $ clang -std=c++14 -Wsystem-headers -Wall -Wextra -c test.cpp In file included from test.cpp:1: In file included from /usr/include/c++/v1/string:470: /usr/include/c++/v1/string_view:763:29: warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals] basic_string_view operator "" sv(const char *__str, size_t __len) ^ /usr/include/c++/v1/string_view:769:32: warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals] basic_string_view operator "" sv(const wchar_t *__str, size_t __len) ^ /usr/include/c++/v1/string_view:775:33: warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals] basic_string_view operator "" sv(const char16_t *__str, size_t __len) ^ /usr/include/c++/v1/string_view:781:33: warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals] basic_string_view operator "" sv(const char32_t *__str, size_t __len) ^ In file included from test.cpp:1: /usr/include/c++/v1/string:4012:24: warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals] basic_string operator "" s( const char *__str, size_t __len ) ^ /usr/include/c++/v1/string:4018:27: warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals] basic_string operator "" s( const wchar_t *__str, size_t __len ) ^ /usr/include/c++/v1/string:4024:28: warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals] basic_string operator "" s( const char16_t *__str, size_t __len ) ^ /usr/include/c++/v1/string:4030:28: warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals] basic_string operator "" s( const char32_t *__str, size_t __len ) ^ 8 warnings generated. Both @aaron.ballman and @mclow.lists felt that adding this workaround to the libc++ headers was the wrong way, and it should be fixed in clang instead. Here is a proposal to do just that. I verified that this suppresses the warning, even when -Wsystem-headers is used, and that the warning is still emitted for a declaration outside of system headers. Repository: rC Clang https://reviews.llvm.org/D41080 Files: lib/Sema/SemaDeclCXX.cpp Index: lib/Sema/SemaDeclCXX.cpp === --- lib/Sema/SemaDeclCXX.cpp +++ lib/Sema/SemaDeclCXX.cpp @@ -13076,7 +13076,8 @@ StringRef LiteralName = FnDecl->getDeclName().getCXXLiteralIdentifier()->getName(); - if (LiteralName[0] != '_') { + if (LiteralName[0] != '_' && + !getSourceManager().isInSystemHeader(FnDecl->getLocation())) { // C++11 [usrlit.suffix]p1: // Literal suffix identifiers that do not start with an underscore // are reserved for future standardization. Index: lib/Sema/SemaDeclCXX.cpp === --- lib/Sema/SemaDeclCXX.cpp +++ lib/Sema/SemaDeclCXX.cpp @@ -13076,7 +13076,8 @@ StringRef LiteralName = FnDecl->getDeclName().getCXXLiteralIdentifier()->getName(); - if (LiteralName[0] != '_') { + if (LiteralName[0] != '_' && + !getSourceManager().isInSystemHeader(FnDecl->getLocation())) { // C++11 [usrlit.suffix]p1: // Literal suffix identifiers that do not start with an underscore // are reserved for future standardization. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41064: Suppress -Wuser-defined-literals for and
dim abandoned this revision. dim added a comment. Abandoned in favor of https://reviews.llvm.org/D41080. Repository: rCXX libc++ https://reviews.llvm.org/D41064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits